Changeset 214893 in webkit for trunk/Source/WebCore/page/Page.cpp

Timestamp:

Apr 4, 2017, 12:59:52 PM (8 years ago)

Author:

[email protected]

Message:

[Mac] -[WKWebView findMatchesForString:relativeToMatch:findOptions:maxResults:resultCollector:] invokes the resultCollector with didWrap = NO even when it wraps
​https://bugs.webkit.org/show_bug.cgi?id=165801
<rdar://problem/29649535>

Reviewed by Wenson Hsieh.

New API tests: WebKit2.FindInPageWrapping*

Previously, when doing an incremental find that wrapped, we would
say that it did not, leading NSTextFinder to not provide its usual
wrapping UI, and other clients of the NSTextFinderClient protocol to
get confused by the lack of wrapping.

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::didFindString):

• UIProcess/WebPageProxy.h:
• UIProcess/WebPageProxy.messages.in:
• UIProcess/API/APIFindClient.h:

(API::FindClient::didFindString):

• UIProcess/API/C/WKPage.cpp:

(WKPageSetPageFindClient):

• UIProcess/Cocoa/FindClient.h:
• UIProcess/Cocoa/FindClient.mm:

(WebKit::FindClient::didFindString):

• WebProcess/WebPage/FindController.cpp:

(WebKit::FindController::updateFindUIAfterPageScroll):
(WebKit::FindController::findString):

• WebProcess/WebPage/FindController.h:

Plumb DidWrap from FindController's call to findString back through
the DidFindString message.

• UIProcess/mac/WKTextFinderClient.mm:

(-[WKTextFinderClient didFindStringMatchesWithRects:didWrapAround:]):
(-[WKTextFinderClient didFindStringMatchesWithRects:]): Deleted.
Make use of the new DidWrap information to stop lying to NSTextFinder
about whether a wrap actually occurred.

• page/FrameTree.cpp:

(WebCore::FrameTree::traverseNextWithWrap):
(WebCore::FrameTree::traversePreviousWithWrap):
(WebCore::FrameTree::traverseNextInPostOrderWithWrap):

• page/FrameTree.h:

Add CanWrap and DidWrap boolean enums, and add an optional out argument
to traverse*WithWrap indicating whether a wrap actually occurred.

• history/CachedPage.cpp:

(WebCore::firePageShowAndPopStateEvents):

• history/PageCache.cpp:

(WebCore::destroyRenderTree):
Adjust to the new CanWrap enum.

• page/Page.cpp:

(WebCore::incrementFrame):
(WebCore::Page::findString):
(WebCore::Page::findStringMatchingRanges):
(WebCore::Page::rangeOfString):
(WebCore::Page::findMatchesForText):
(WebCore::Page::unmarkAllTextMatches):

• page/Page.h:

Adjust to the new CanWrap enum, and optionally plumb DidWrap through
to callers of findString().

• WebView/WebView.mm:

(incrementFrame):
Adjust to the new CanWrap enum.

• TestWebKitAPI/Tests/WebKit2Cocoa/FindInPage.mm:

(TEST):
Add some tests for wrapping finds.

File:

• trunk/Source/WebCore/page/Page.cpp (modified) (7 diffs)

trunk/Source/WebCore/page/Page.cpp

@@ -648 +648 @@
 }
 
-static Frame* incrementFrame(Frame* curr, bool forward, bool wrapFlag)
+static Frame* incrementFrame(Frame* curr, bool forward, CanWrap canWrap, DidWrap* didWrap = nullptr)
 {
     return forward
-        ? curr->tree().traverseNextWithWrap(wrapFlag)
-        : curr->tree().traversePreviousWithWrap(wrapFlag);
-}
-
-bool Page::findString(const String& target, FindOptions options)
+        ? curr->tree().traverseNext(canWrap, didWrap)
+        : curr->tree().traversePrevious(canWrap, didWrap);
+}
+
+bool Page::findString(const String& target, FindOptions options, DidWrap* didWrap)
 {
     if (target.isEmpty())
         return false;
 
-    bool shouldWrap = options & WrapAround;
+    CanWrap canWrap = options & WrapAround ? CanWrap::Yes : CanWrap::No;
     Frame* frame = &focusController().focusedOrMainFrame();
     Frame* startFrame = frame;
@@ -670 +670 @@
             return true;
         }
-        frame = incrementFrame(frame, !(options & Backwards), shouldWrap);
+        frame = incrementFrame(frame, !(options & Backwards), canWrap, didWrap);
     } while (frame && frame != startFrame);
 
     // Search contents of startFrame, on the other side of the selection that we did earlier.
     // We cheat a bit and just research with wrap on
-    if (shouldWrap && !startFrame->selection().isNone()) {
+    if (canWrap == CanWrap::Yes && !startFrame->selection().isNone()) {
+        if (didWrap)
+            *didWrap = DidWrap::Yes;
         bool found = startFrame->editor().findString(target, options | WrapAround | StartInSelection);
         focusController().setFocusedFrame(frame);
@@ -694 +696 @@
         if (frame->selection().isRange())
             frameWithSelection = frame;
-        frame = incrementFrame(frame, true, false);
+        frame = incrementFrame(frame, true, CanWrap::No);
     } while (frame);
 
@@ -736 +738 @@
         return nullptr;
 
-    bool shouldWrap = options & WrapAround;
+    CanWrap canWrap = options & WrapAround ? CanWrap::Yes : CanWrap::No;
     Frame* frame = referenceRange ? referenceRange->ownerDocument().frame() : &mainFrame();
     Frame* startFrame = frame;
@@ -743 +745 @@
             return resultRange;
 
-        frame = incrementFrame(frame, !(options & Backwards), shouldWrap);
+        frame = incrementFrame(frame, !(options & Backwards), canWrap);
     } while (frame && frame != startFrame);
 
     // Search contents of startFrame, on the other side of the reference range that we did earlier.
     // We cheat a bit and just search again with wrap on.
-    if (shouldWrap && referenceRange) {
+    if (canWrap == CanWrap::Yes && referenceRange) {
         if (RefPtr<Range> resultRange = startFrame->editor().rangeOfString(target, referenceRange, options | WrapAround | StartInSelection))
             return resultRange;
@@ -768 +770 @@
             frame->editor().setMarkedTextMatchesAreHighlighted(shouldHighlightMatches == HighlightMatches);
         matchCount += frame->editor().countMatchesForText(target, 0, options, maxMatchCount ? (maxMatchCount - matchCount) : 0, shouldMarkMatches == MarkMatches, 0);
-        frame = incrementFrame(frame, true, false);
+        frame = incrementFrame(frame, true, CanWrap::No);
     } while (frame);
 
@@ -789 +791 @@
     do {
         frame->document()->markers().removeMarkers(DocumentMarker::TextMatch);
-        frame = incrementFrame(frame, true, false);
+        frame = incrementFrame(frame, true, CanWrap::No);
     } while (frame);
 }