Changeset 122428 in webkit

Timestamp:

Jul 12, 2012 1:38:16 AM (12 years ago)

Author:

Carlos Garcia Campos

Message:

[WK2] Performance issue in FindController::findString
​https://bugs.webkit.org/show_bug.cgi?id=78132

Patch by Sergio Villar Senin <​svillar@igalia.com> on 2012-07-12
Reviewed by Anders Carlsson.

FindController should not unmark all text matches by default. It
will be done only if the string is not found or if
markAllTextMatches() is called. This will allow clients to look
for the next/previous without having to unmark() + mark() all the
text matches for every single search operation.

• UIProcess/API/gtk/WebKitFindController.cpp:

(webKitFindControllerPerform):
(webkit_find_controller_search_next):
(webkit_find_controller_search_previous):

• WebProcess/WebPage/FindController.cpp:

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

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/gtk/WebKitFindController.cpp (modified) (4 diffs)
• WebProcess/WebPage/FindController.cpp (modified) (3 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2012-07-12  Sergio Villar Senin  <svillar@igalia.com>
+
+        [WK2] Performance issue in FindController::findString
+        https://bugs.webkit.org/show_bug.cgi?id=78132
+
+        Reviewed by Anders Carlsson.
+
+        FindController should not unmark all text matches by default. It
+        will be done only if the string is not found or if
+        markAllTextMatches() is called. This will allow clients to look
+        for the next/previous without having to unmark() + mark() all the
+        text matches for every single search operation.
+
+        * UIProcess/API/gtk/WebKitFindController.cpp:
+        (webKitFindControllerPerform):
+        (webkit_find_controller_search_next):
+        (webkit_find_controller_search_previous):
+        * WebProcess/WebPage/FindController.cpp:
+        (WebKit::FindController::updateFindUIAfterPageScroll):
+        (WebKit::FindController::findString):
+
 2012-07-12  Christophe Dumez  <christophe.dumez@intel.com>
 

trunk/Source/WebKit2/UIProcess/API/gtk/WebKitFindController.cpp

@@ -49 +49 @@
 typedef enum {
     FindOperation,
+    FindNextPrevOperation,
     CountOperation
 } WebKitFindControllerOperation;
@@ -346 +347 @@
     WKPageRef wkPage = getWKPageFromWebKitWebView(findController->priv->webView);
 
-    if (operation == FindOperation) {
-        // Unconditionally highlight text matches. WK1 API was forcing
-        // clients to enable/disable highlighting. Since most of them
-        // (all?) where using highlighting we decided to simplify the
-        // WK2 API and unconditionally show highlights.
-        wkFindOptions = static_cast<WKFindOptions>(findController->priv->findOptions | kWKFindOptionsShowHighlight);
-        WKPageFindString(wkPage, wkSearchText.get(), wkFindOptions, findController->priv->maxMatchCount);
+    if (operation == CountOperation) {
+        WKPageCountStringMatches(wkPage, wkSearchText.get(), wkFindOptions, findController->priv->maxMatchCount);
         return;
     }
 
-    WKPageCountStringMatches(wkPage, wkSearchText.get(), wkFindOptions, findController->priv->maxMatchCount);
+    if (operation == FindOperation)
+        // Unconditionally highlight text matches when the search
+        // starts. WK1 API was forcing clients to enable/disable
+        // highlighting. Since most of them (all?) where using that
+        // feature we decided to simplify the WK2 API and
+        // unconditionally show highlights. Both search_next() and
+        // search_prev() should not enable highlighting to avoid an
+        // extra unmarkAllTextMatches() + markAllTextMatches()
+        wkFindOptions = static_cast<WKFindOptions>(findController->priv->findOptions | kWKFindOptionsShowHighlight);
+
+    WKPageFindString(wkPage, wkSearchText.get(), wkFindOptions, findController->priv->maxMatchCount);
 }
 
@@ -416 +422 @@
 
     findController->priv->findOptions = findController->priv->findOptions & ~WEBKIT_FIND_OPTIONS_BACKWARDS;
-    webKitFindControllerPerform(findController, FindOperation);
+    findController->priv->findOptions = findController->priv->findOptions & ~kWKFindOptionsShowHighlight;
+    webKitFindControllerPerform(findController, FindNextPrevOperation);
 }
 
@@ -433 +440 @@
 
     findController->priv->findOptions = findController->priv->findOptions | WEBKIT_FIND_OPTIONS_BACKWARDS;
-    webKitFindControllerPerform(findController, FindOperation);
+    findController->priv->findOptions = findController->priv->findOptions & ~kWKFindOptionsShowHighlight;
+    webKitFindControllerPerform(findController, FindNextPrevOperation);
 }
 

trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp

@@ -98 +98 @@
 
     if (!found) {
+        m_webPage->corePage()->unmarkAllTextMatches();
+
         // Clear the selection.
         if (selectedFrame)
@@ -115 +117 @@
                 --maxMatchCount;
 
+            m_webPage->corePage()->unmarkAllTextMatches();
             matchCount = m_webPage->corePage()->markAllMatchesForText(string, core(options), shouldShowHighlight, maxMatchCount + 1);
 
@@ -153 +156 @@
 void FindController::findString(const String& string, FindOptions options, unsigned maxMatchCount)
 {
-    m_webPage->corePage()->unmarkAllTextMatches();
-
     bool found = m_webPage->corePage()->findString(string, core(options));