Bug 429732 - Make Finder.jsm iterate over matches asynchronously in small batches so it does not block the UI thread. r=mikedeboer
authorTomasz Kołodziejski <tkolodziejski@mozilla.com>
Thu, 18 Sep 2014 10:23:00 +0200
changeset 206197 2d22a8a36638dc1fcc1bed513f24b857883a2316
parent 206196 9dbcd7b224919fc8dda74c91683d07a1ee055678
child 206198 f73db3f4348155f32cdac5f4f35f75b2542975dd
push id27516
push userryanvm@gmail.com
push dateFri, 19 Sep 2014 17:54:48 +0000
treeherdermozilla-central@b00bdb144e06 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs429732
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 429732 - Make Finder.jsm iterate over matches asynchronously in small batches so it does not block the UI thread. r=mikedeboer
toolkit/content/tests/chrome/findbar_window.xul
toolkit/content/widgets/findbar.xml
toolkit/modules/Finder.jsm
--- a/toolkit/content/tests/chrome/findbar_window.xul
+++ b/toolkit/content/tests/chrome/findbar_window.xul
@@ -387,34 +387,16 @@
          "testQuickFindText: find field is not focused");
 
       enterStringIntoFindField(SEARCH_TEXT);
       ok(gBrowser.contentWindow.getSelection() == SEARCH_TEXT,
          "testQuickFindText: failed to find '" + SEARCH_TEXT + "'");
       testClipboardSearchString(SEARCH_TEXT);
     }
 
-    // Perform an async function in serial on each of the list items.
-    function asyncForEach(list, async, callback) {
-      let i = 0;
-      let len = list.length;
-
-      if (!len)
-        return callback();
-
-      async(list[i], function handler() {
-          i++;
-          if (i < len) {
-            async(list[i], handler, i);
-          } else {
-            callback();
-          }
-      }, i);
-    }
-
     function testFindCountUI(callback) {
       clearFocus();
       document.getElementById("cmd_find").doCommand();
 
       ok(!gFindBar.hidden, "testFindCountUI: failed to open findbar");
       ok(document.commandDispatcher.focusedElement == gFindBar._findField.inputField,
          "testFindCountUI: find field is not focused");
 
@@ -439,56 +421,55 @@
         text: "texxx",
         current: 0,
         total: 0
       }];
       let regex = /([\d]*)\sof\s([\d]*)/;
       let timeout = gFindBar._matchesCountTimeoutLength + 20;
 
       function assertMatches(aTest, aMatches) {
-        window.opener.wrappedJSObject.SimpleTest.is(aTest.current, aMatches[1],
+        window.opener.wrappedJSObject.SimpleTest.is(aMatches[1], aTest.current,
           "Currently highlighted match should be at " + aTest.current);
-        window.opener.wrappedJSObject.SimpleTest.is(aTest.total, aMatches[2],
+        window.opener.wrappedJSObject.SimpleTest.is(aMatches[2], aTest.total,
           "Total amount of matches should be " + aTest.total);
       }
 
-      function testString(aTest, aNext) {
-        gFindBar.clear();
-        enterStringIntoFindField(aTest.text);
-
-        setTimeout(function() {
+      function* generatorTest() {
+        for (let test of tests) {
+          gFindBar.clear();
+          yield;
+          enterStringIntoFindField(test.text);
+          yield;
           let matches = foundMatches.value.match(regex);
-          if (!aTest.total) {
+          if (!test.total) {
             ok(!matches, "No message should be shown when 0 matches are expected");
-            aNext();
           } else {
-            assertMatches(aTest, matches);
-            let cycleTests = [];
-            let cycles = aTest.total;
-            while (--cycles) {
-              aTest.current++;
-              if (aTest.current > aTest.total)
-                aTest.current = 1;
-              cycleTests.push({
-                current: aTest.current,
-                total: aTest.total
-              });
+            assertMatches(test, matches);
+            for (let i = 1; i < test.total; i++) {
+              gFindBar.onFindAgainCommand();
+              yield;
+              // test.current + 1, test.current + 2, ..., test.total, 1, ..., test.current
+              let current = (test.current + i - 1) % test.total + 1;
+              assertMatches({
+                current: current,
+                total: test.total
+              }, foundMatches.value.match(regex));
             }
-            asyncForEach(cycleTests, function(aCycleTest, aNextCycle) {
-              gFindBar.onFindAgainCommand();
-              setTimeout(function() {
-                assertMatches(aCycleTest, foundMatches.value.match(regex));
-                aNextCycle();
-              }, timeout);
-            }, aNext);
           }
-        }, timeout);
+        }
+        callback();
       }
-
-      asyncForEach(tests, testString, callback);
+      let test = generatorTest();
+      let resultListener = {
+        onMatchesCountResult: function() {
+          test.next();
+        }
+      };
+      gFindBar.browser.finder.addResultListener(resultListener);
+      test.next();
     }
 
     function testClipboardSearchString(aExpected) {
       if (!gHasFindClipboard)
         return;
 
       if (!aExpected)
         aExpected = "";
--- a/toolkit/content/widgets/findbar.xml
+++ b/toolkit/content/widgets/findbar.xml
@@ -443,50 +443,34 @@
           if (!this._pluralForm) {
             this._pluralForm = Components.utils.import(
                                "resource://gre/modules/PluralForm.jsm", {}).PluralForm;
           }
           return this._pluralForm;
         ]]></getter>
       </property>
 
-      <method name="_updateMatchesCountWorker">
-        <parameter name="aRes"/>
-        <body><![CDATA[
-          let word = this._findField.value;
-          if (aRes == this.nsITypeAheadFind.FIND_NOTFOUND || !word) {
-            this._foundMatches.hidden = true;
-            this._foundMatches.value = "";
-          } else {
-            let matchesCount = this.browser.finder.requestMatchesCount(
-              word, this._matchesCountLimit, this._findMode == this.FIND_LINKS);
-            window.clearTimeout(this._updateMatchesCountTimeout);
-            this._updateMatchesCountTimeout = null;
-          }
-        ]]></body>
-      </method>
-
       <!--
         - Updates the search match count after each find operation on a new string.
         - @param aRes
         -        the result of the find operation
         -->
       <method name="_updateMatchesCount">
-        <parameter name="aRes"/>
         <body><![CDATA[
           if (this._matchesCountLimit == 0 || !this._dispatchFindEvent("matchescount"))
             return;
 
           if (this._updateMatchesCountTimeout) {
             window.clearTimeout(this._updateMatchesCountTimeout);
-            this._updateMatchesCountTimeout = null;
           }
           this._updateMatchesCountTimeout =
-            window.setTimeout(() => this._updateMatchesCountWorker(aRes),
-                              this._matchesCountTimeoutLength);
+            window.setTimeout(() => {
+              this.browser.finder.requestMatchesCount(this._findField.value, this._matchesCountLimit,
+                  this._findMode == this.FIND_LINKS);
+            }, this._matchesCountTimeoutLength);
         ]]></body>
       </method>
 
       <!--
         - Turns highlight on or off.
         - @param aHighlight (boolean)
         -        Whether to turn the highlight on or off
         -->
--- a/toolkit/modules/Finder.jsm
+++ b/toolkit/modules/Finder.jsm
@@ -6,27 +6,30 @@ this.EXPORTED_SYMBOLS = ["Finder"];
 
 const Ci = Components.interfaces;
 const Cc = Components.classes;
 const Cu = Components.utils;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Geometry.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/Task.jsm");
 
 XPCOMUtils.defineLazyServiceGetter(this, "TextToSubURIService",
                                          "@mozilla.org/intl/texttosuburi;1",
                                          "nsITextToSubURI");
 XPCOMUtils.defineLazyServiceGetter(this, "Clipboard",
                                          "@mozilla.org/widget/clipboard;1",
                                          "nsIClipboard");
 XPCOMUtils.defineLazyServiceGetter(this, "ClipboardHelper",
                                          "@mozilla.org/widget/clipboardhelper;1",
                                          "nsIClipboardHelper");
 
+const kHighlightIterationSizeMax = 100;
+
 function Finder(docShell) {
   this._fastFind = Cc["@mozilla.org/typeaheadfind;1"].createInstance(Ci.nsITypeAheadFind);
   this._fastFind.init(docShell);
 
   this._docShell = docShell;
   this._listeners = [];
   this._previousLink = null;
   this._searchString = null;
@@ -122,42 +125,44 @@ Finder.prototype = {
                                           Ci.nsIClipboard.kFindClipboard,
                                           this._getWindow().document);
   },
 
   set caseSensitive(aSensitive) {
     this._fastFind.caseSensitive = aSensitive;
   },
 
+  _lastFindResult: null,
+
   /**
    * Used for normal search operations, highlights the first match.
    *
    * @param aSearchString String to search for.
    * @param aLinksOnly Only consider nodes that are links for the search.
    * @param aDrawOutline Puts an outline around matched links.
    */
   fastFind: function (aSearchString, aLinksOnly, aDrawOutline) {
-    let result = this._fastFind.find(aSearchString, aLinksOnly);
+    this._lastFindResult = this._fastFind.find(aSearchString, aLinksOnly);
     let searchString = this._fastFind.searchString;
-    this._notify(searchString, result, false, aDrawOutline);
+    this._notify(searchString, this._lastFindResult, false, aDrawOutline);
   },
 
   /**
    * Repeat the previous search. Should only be called after a previous
    * call to Finder.fastFind.
    *
    * @param aFindBackwards Controls the search direction:
    *    true: before current match, false: after current match.
    * @param aLinksOnly Only consider nodes that are links for the search.
    * @param aDrawOutline Puts an outline around matched links.
    */
   findAgain: function (aFindBackwards, aLinksOnly, aDrawOutline) {
-    let result = this._fastFind.findAgain(aFindBackwards, aLinksOnly);
+    this._lastFindResult = this._fastFind.findAgain(aFindBackwards, aLinksOnly);
     let searchString = this._fastFind.searchString;
-    this._notify(searchString, result, aFindBackwards, aDrawOutline);
+    this._notify(searchString, this._lastFindResult, aFindBackwards, aDrawOutline);
   },
 
   /**
    * Forcibly set the search string of the find clipboard to the currently
    * selected text in the window, on supported platforms (i.e. OSX).
    */
   setSearchStringToSelection: function() {
     // Find the selected text.
@@ -169,24 +174,28 @@ Finder.prototype = {
     // Empty strings are rather useless to search for.
     if (!searchString.length)
       return null;
 
     this.clipboardSearchString = searchString;
     return searchString;
   },
 
-  highlight: function (aHighlight, aWord) {
-    let found = this._highlight(aHighlight, aWord, null);
+  highlight: Task.async(function* (aHighlight, aWord) {
+    if (this._abortHighlight) {
+      this._abortHighlight();
+    }
+
+    let found = yield this._highlight(aHighlight, aWord, null);
     if (aHighlight) {
       let result = found ? Ci.nsITypeAheadFind.FIND_FOUND
                          : Ci.nsITypeAheadFind.FIND_NOTFOUND;
       this._notify(aWord, result, false, false, false);
     }
-  },
+  }),
 
   enableSelection: function() {
     this._fastFind.setSelectionModeAndRepaint(Ci.nsISelectionController.SELECTION_ON);
     this._restoreOriginalOutline();
   },
 
   removeSelection: function() {
     this._fastFind.collapseSelection();
@@ -257,38 +266,49 @@ Finder.prototype = {
         controller.scrollLine(false);
         break;
       case Ci.nsIDOMKeyEvent.DOM_VK_DOWN:
         controller.scrollLine(true);
         break;
     }
   },
 
+  _notifyMatchesCount: function(result) {
+    for (let l of this._listeners) {
+      try {
+        l.onMatchesCountResult(result);
+      } catch (ex) {}
+    }
+  },
+
   requestMatchesCount: function(aWord, aMatchLimit, aLinksOnly) {
+    if (this._lastFindResult == Ci.nsITypeAheadFind.FIND_NOTFOUND ||
+        this.searchString == "") {
+      return this._notifyMatchesCount({
+        total: 0,
+        current: 0
+      });
+    }
     let window = this._getWindow();
     let result = this._countMatchesInWindow(aWord, aMatchLimit, aLinksOnly, window);
 
     // Count matches in (i)frames AFTER searching through the main window.
     for (let frame of result._framesToCount) {
       // We've reached our limit; no need to do more work.
       if (result.total == -1 || result.total == aMatchLimit)
         break;
       this._countMatchesInWindow(aWord, aMatchLimit, aLinksOnly, frame, result);
     }
 
     // The `_currentFound` and `_framesToCount` properties are only used for
     // internal bookkeeping between recursive calls.
     delete result._currentFound;
     delete result._framesToCount;
 
-    for (let l of this._listeners) {
-      try {
-        l.onMatchesCountResult(result);
-      } catch (ex) {}
-    }
+    this._notifyMatchesCount(result);
   },
 
   /**
    * Counts the number of matches for the searched word in the passed window's
    * content.
    * @param aWord
    *        the word to search for.
    * @param aMatchLimit
@@ -317,51 +337,47 @@ Finder.prototype = {
       aStats.total = -1;
       return aStats;
     }
 
     this._collectFrames(aWindow, aStats);
 
     let foundRange = this._fastFind.getFoundRange();
 
-    this._findIterator(aWord, aWindow, aRange => {
-      if (!aLinksOnly || this._rangeStartsInLink(aRange)) {
+    for(let range of this._findIterator(aWord, aWindow)) {
+      if (!aLinksOnly || this._rangeStartsInLink(range)) {
         ++aStats.total;
         if (!aStats._currentFound) {
           ++aStats.current;
           aStats._currentFound = (foundRange &&
-            aRange.startContainer == foundRange.startContainer &&
-            aRange.startOffset == foundRange.startOffset &&
-            aRange.endContainer == foundRange.endContainer &&
-            aRange.endOffset == foundRange.endOffset);
+            range.startContainer == foundRange.startContainer &&
+            range.startOffset == foundRange.startOffset &&
+            range.endContainer == foundRange.endContainer &&
+            range.endOffset == foundRange.endOffset);
         }
       }
       if (aStats.total == aMatchLimit) {
         aStats.total = -1;
-        return false;
+        break;
       }
-    });
+    };
 
     return aStats;
   },
 
   /**
-   * Basic wrapper around nsIFind that provides invoking a callback `aOnFind`
-   * each time an occurence of `aWord` string is found.
+   * Basic wrapper around nsIFind that provides a generator yielding
+   * a range each time an occurence of `aWord` string is found.
    *
    * @param aWord
    *        the word to search for.
    * @param aWindow
    *        the window to search in.
-   * @param aOnFind
-   *        the Function to invoke when a word is found. if Boolean `false` is
-   *        returned, the find operation will be stopped and the Function will
-   *        not be invoked again.
    */
-  _findIterator: function(aWord, aWindow, aOnFind) {
+  _findIterator: function* (aWord, aWindow) {
     let doc = aWindow.document;
     let body = (doc instanceof Ci.nsIDOMHTMLDocument && doc.body) ?
                doc.body : doc.documentElement;
 
     if (!body)
       return;
 
     let searchRange = doc.createRange();
@@ -376,23 +392,44 @@ Finder.prototype = {
     let retRange = null;
 
     let finder = Cc["@mozilla.org/embedcomp/rangefind;1"]
                    .createInstance()
                    .QueryInterface(Ci.nsIFind);
     finder.caseSensitive = this._fastFind.caseSensitive;
 
     while ((retRange = finder.Find(aWord, searchRange, startPt, endPt))) {
-      if (aOnFind(retRange) === false)
-        break;
+      yield retRange;
       startPt = retRange.cloneRange();
       startPt.collapse(false);
     }
   },
 
+  _highlightIterator: Task.async(function* (aWord, aWindow, aOnFind) {
+    let count = 0;
+    for (let range of this._findIterator(aWord, aWindow)) {
+      aOnFind(range);
+      if (++count >= kHighlightIterationSizeMax) {
+          count = 0;
+          yield this._highlightSleep(0);
+      }
+    }
+  }),
+
+  _abortHighlight: null,
+  _highlightSleep: function(delay) {
+    return new Promise((resolve, reject) => {
+      this._abortHighlight = () => {
+        this._abortHighlight = null;
+        reject();
+      };
+      this._getWindow().setTimeout(resolve, delay);
+    });
+  },
+
   /**
    * Helper method for `_countMatchesInWindow` that recursively collects all
    * visible (i)frames inside a window.
    *
    * @param aWindow
    *        the window to extract the (i)frames from.
    * @param aStats
    *        Object that contains a Set called '_framesToCount'
@@ -516,35 +553,35 @@ Finder.prototype = {
     // Removes the outline around the last found link.
     if (this._previousLink) {
       this._previousLink.style.outline = this._tmpOutline;
       this._previousLink.style.outlineOffset = this._tmpOutlineOffset;
       this._previousLink = null;
     }
   },
 
-  _highlight: function (aHighlight, aWord, aWindow) {
+  _highlight: Task.async(function* (aHighlight, aWord, aWindow) {
     let win = aWindow || this._getWindow();
 
     let found = false;
     for (let i = 0; win.frames && i < win.frames.length; i++) {
-      if (this._highlight(aHighlight, aWord, win.frames[i]))
+      if (yield this._highlight(aHighlight, aWord, win.frames[i]))
         found = true;
     }
 
     let controller = this._getSelectionController(win);
     let doc = win.document;
     if (!controller || !doc || !doc.documentElement) {
       // Without the selection controller,
       // we are unable to (un)highlight any matches
       return found;
     }
 
     if (aHighlight) {
-      this._findIterator(aWord, win, aRange => {
+      yield this._highlightIterator(aWord, win, aRange => {
         this._highlightRange(aRange, controller);
         found = true;
       });
     } else {
       // First, attempt to remove highlighting from main document
       let sel = controller.getSelection(Ci.nsISelectionController.SELECTION_FIND);
       sel.removeAllRanges();
 
@@ -562,17 +599,17 @@ Finder.prototype = {
         }
       }
 
       // Removing the highlighting always succeeds, so return true.
       found = true;
     }
 
     return found;
-  },
+  }),
 
   _highlightRange: function(aRange, aController) {
     let node = aRange.startContainer;
     let controller = aController;
 
     let editableNode = this._getEditableNode(node);
     if (editableNode)
       controller = editableNode.editor.selectionController;