Bug 1290914 - move the matches count and highlight-all request from the findbar binding to the JS module in the content process, so it's closer to the metal. r?jaws draft
authorMike de Boer <mdeboer@mozilla.com>
Wed, 07 Sep 2016 12:03:45 +0200
changeset 410968 0d56ae5f3cdda0ad88735139c35ebdc321ebc0ae
parent 410967 1d7be95d8cbe5bf7868adf31281774adf8cc161a
child 410969 04873d7e224d9c76b78915a14cab02b425db5a4e
push id28805
push usermdeboer@mozilla.com
push dateWed, 07 Sep 2016 10:05:39 +0000
reviewersjaws
bugs1290914
milestone51.0a1
Bug 1290914 - move the matches count and highlight-all request from the findbar binding to the JS module in the content process, so it's closer to the metal. r?jaws MozReview-Commit-ID: IgPlFro1bg9
dom/browser-element/BrowserElementChildPreload.js
toolkit/content/widgets/findbar.xml
toolkit/modules/Finder.jsm
toolkit/modules/FinderHighlighter.jsm
toolkit/modules/RemoteFinder.jsm
--- a/dom/browser-element/BrowserElementChildPreload.js
+++ b/dom/browser-element/BrowserElementChildPreload.js
@@ -1463,65 +1463,62 @@ BrowserElementChild.prototype = {
         return;
       }
     }
     sendAsyncMsg('got-web-manifest', {
       id: data.json.id,
       successRv: manifest
     });
   }),
+
   _initFinder: function() {
     if (!this._finder) {
-      try {
-        this._findLimit = Services.prefs.getIntPref("accessibility.typeaheadfind.matchesCountLimit");
-      } catch (e) {
-        // Pref not available, assume 0, no match counting.
-        this._findLimit = 0;
-      }
-
       let {Finder} = Components.utils.import("resource://gre/modules/Finder.jsm", {});
       this._finder = new Finder(docShell);
-      this._finder.addResultListener({
-        onMatchesCountResult: (data) => {
-          sendAsyncMsg('findchange', {
-            active: true,
-            searchString: this._finder.searchString,
-            searchLimit: this._findLimit,
-            activeMatchOrdinal: data.current,
-            numberOfMatches: data.total
-          });
-        }
-      });
     }
+    let listener = {
+      onMatchesCountResult: (data) => {
+        sendAsyncMsg("findchange", {
+          active: true,
+          searchString: this._finder.searchString,
+          searchLimit: this._finder.matchesCountLimit,
+          activeMatchOrdinal: data.current,
+          numberOfMatches: data.total
+        });
+        this._finder.removeResultListener(listener);
+      }
+    };
+    this._finder.addResultListener(listener);
   },
 
   _recvFindAll: function(data) {
     this._initFinder();
     let searchString = data.json.searchString;
     this._finder.caseSensitive = data.json.caseSensitive;
     this._finder.fastFind(searchString, false, false);
-    this._finder.requestMatchesCount(searchString, this._findLimit, false);
+    this._finder.requestMatchesCount(searchString, this._finder.matchesCountLimit, false);
   },
 
   _recvFindNext: function(data) {
     if (!this._finder) {
       debug("findNext() called before findAll()");
       return;
     }
+    this._initFinder();
     this._finder.findAgain(data.json.backward, false, false);
-    this._finder.requestMatchesCount(this._finder.searchString, this._findLimit, false);
+    this._finder.requestMatchesCount(this._finder.searchString, this._finder.matchesCountLimit, false);
   },
 
   _recvClearMatch: function(data) {
     if (!this._finder) {
       debug("clearMach() called before findAll()");
       return;
     }
     this._finder.removeSelection();
-    sendAsyncMsg('findchange', {active: false});
+    sendAsyncMsg("findchange", {active: false});
   },
 
   _recvSetInputMethodActive: function(data) {
     let msgData = { id: data.json.id };
     if (!this._isContentWindowCreated) {
       if (data.json.args.isActive) {
         // To activate the input method, we should wait before the content
         // window is ready.
--- a/toolkit/content/widgets/findbar.xml
+++ b/toolkit/content/widgets/findbar.xml
@@ -339,16 +339,18 @@
               this._self._typeAheadLinksOnly = prefsvc.getBoolPref(aPrefName);
               break;
             case "accessibility.typeaheadfind.casesensitive":
               this._self._setCaseSensitivity(prefsvc.getIntPref(aPrefName));
               break;
             case "findbar.entireword":
               this._self._entireWord = prefsvc.getBoolPref(aPrefName);
               this._self._updateEntireWord();
+              // Update the matches count.
+              this._updateMatchesCount(this.nsITypeAheadFind.FIND_FOUND);
               break;
             case "findbar.highlightAll":
               this._self.toggleHighlight(prefsvc.getBoolPref(aPrefName), true);
               break;
             case "findbar.modalHighlight":
               this._self._useModalHighlight = prefsvc.getBoolPref(aPrefName);
               if (this._self.browser.finder)
                 this._self.browser.finder.onModalHighlightChange(this._self._useModalHighlight);
@@ -494,21 +496,21 @@
 
       <!--
         - Updates the search match count after each find operation on a new string.
         - @param aRes
         -        the result of the find operation
         -->
       <method name="_updateMatchesCount">
         <body><![CDATA[
-          if (this._matchesCountLimit == 0 || !this._dispatchFindEvent("matchescount"))
+          if (!this._dispatchFindEvent("matchescount"))
             return;
 
           this.browser.finder.requestMatchesCount(this._findField.value,
-            this._matchesCountLimit, this._findMode == this.FIND_LINKS);
+            this._findMode == this.FIND_LINKS);
         ]]></body>
       </method>
 
       <!--
         - Turns highlight on or off.
         - @param aHighlight (boolean)
         -        Whether to turn the highlight on or off
         - @param aFromPrefObserver (boolean)
@@ -623,17 +625,16 @@
         -->
       <method name="_setCaseSensitivity">
         <parameter name="aCaseSensitivity"/>
         <body><![CDATA[
           this._typeAheadCaseSensitive = aCaseSensitivity;
           this._updateCaseSensitivity();
           this._findFailedString = null;
           this._find();
-          this._maybeHighlightAll();
 
           this._dispatchFindEvent("casesensitivitychange");
         ]]></body>
       </method>
 
       <!--
         - Updates the entire-word mode of the findbar and its UI.
         -->
@@ -648,19 +649,16 @@
 
           // Show the checkbox on the full Find bar in non-auto mode.
           // Show the label in all other cases.
           let hideCheckbox = this._findMode != this.FIND_NORMAL;
           checkbox.hidden = hideCheckbox;
           statusLabel.hidden = !hideCheckbox;
 
           this.browser.finder.entireWord = entireWord;
-
-          // Update the matches count
-          this._updateMatchesCount(this.nsITypeAheadFind.FIND_FOUND);
         ]]></body>
       </method>
 
       <!--
         - Sets the findbar entire-word mode
         - @param aEntireWord (boolean)
         - Whether or not entire-word mode should be turned on.
         -->
@@ -1015,18 +1013,16 @@
             // Getting here means the user commanded a find op. Make sure any
             // initial prefilling is ignored if it hasn't happened yet.
             if (this._startFindDeferred) {
               this._startFindDeferred.resolve();
               this._startFindDeferred = null;
             }
 
             this._enableFindButtons(val);
-            this._maybeHighlightAll();
-
             this._updateCaseSensitivity(val);
             this._updateEntireWord();
 
             this.browser.finder.fastFind(val, this._findMode == this.FIND_LINKS,
                                          this._findMode != this.FIND_NORMAL);
           }
 
           if (this._findMode != this.FIND_NORMAL)
@@ -1095,17 +1091,16 @@
               break;
             case this.nsITypeAheadFind.FIND_FOUND:
             default:
               this._findStatusIcon.removeAttribute("status");
               this._findStatusDesc.textContent = "";
               this._findField.removeAttribute("status");
               break;
           }
-          this._updateMatchesCount(res);
         ]]></body>
       </method>
 
       <method name="updateControlState">
         <parameter name="aResult"/>
         <parameter name="aFindPrevious"/>
         <body><![CDATA[
           this._updateStatusUI(aResult, aFindPrevious);
--- a/toolkit/modules/Finder.jsm
+++ b/toolkit/modules/Finder.jsm
@@ -21,16 +21,17 @@ XPCOMUtils.defineLazyServiceGetter(this,
 XPCOMUtils.defineLazyServiceGetter(this, "Clipboard",
                                          "@mozilla.org/widget/clipboard;1",
                                          "nsIClipboard");
 XPCOMUtils.defineLazyServiceGetter(this, "ClipboardHelper",
                                          "@mozilla.org/widget/clipboardhelper;1",
                                          "nsIClipboardHelper");
 
 const kSelectionMaxLen = 150;
+const kMatchesCountLimitPref = "accessibility.typeaheadfind.matchesCountLimit";
 
 function Finder(docShell) {
   this._fastFind = Cc["@mozilla.org/typeaheadfind;1"].createInstance(Ci.nsITypeAheadFind);
   this._fastFind.init(docShell);
 
   this._currentFoundRange = null;
   this._docShell = docShell;
   this._listeners = [];
@@ -108,17 +109,19 @@ Finder.prototype = {
     if (!this.iterator.continueRunning({
       caseSensitive: this._fastFind.caseSensitive,
       entireWord: this._fastFind.entireWord,
       linksOnly: options.linksOnly,
       word: options.searchString
     })) {
       this.iterator.stop();
     }
+
     this.highlighter.update(options);
+    this.requestMatchesCount(options.searchString, options.linksOnly);
 
     this._outlineLink(options.drawOutline);
 
     for (let l of this._listeners) {
       try {
         l.onFindResult(options);
       } catch (ex) {}
     }
@@ -162,16 +165,24 @@ Finder.prototype = {
   get highlighter() {
     if (this._highlighter)
       return this._highlighter;
 
     const {FinderHighlighter} = Cu.import("resource://gre/modules/FinderHighlighter.jsm", {});
     return this._highlighter = new FinderHighlighter(this);
   },
 
+  get matchesCountLimit() {
+    if (typeof this._matchesCountLimit == "number")
+      return this._matchesCountLimit;
+
+    this._matchesCountLimit = Services.prefs.getIntPref(kMatchesCountLimitPref) || 0;
+    return this._matchesCountLimit;
+  },
+
   _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.
@@ -316,16 +327,17 @@ Finder.prototype = {
         this._getWindow().focus()
       }
     } catch (e) {}
   },
 
   onFindbarClose: function() {
     this.enableSelection();
     this.highlighter.highlight(false);
+    this.iterator.reset();
     BrowserUtils.trackToolbarVisibility(this._docShell, "findbar", false);
   },
 
   onFindbarOpen: function() {
     BrowserUtils.trackToolbarVisibility(this._docShell, "findbar", true);
   },
 
   onModalHighlightChange(useModalHighlight) {
@@ -378,54 +390,53 @@ Finder.prototype = {
         controller.scrollLine(true);
         break;
     }
   },
 
   _notifyMatchesCount: function(result = this._currentMatchesCountResult) {
     // The `_currentFound` property is only used for internal bookkeeping.
     delete result._currentFound;
-    if (result.total == this._currentMatchLimit)
+    if (result.total == this.matchesCountLimit)
       result.total = -1;
 
     for (let l of this._listeners) {
       try {
         l.onMatchesCountResult(result);
       } catch (ex) {}
     }
 
     this._currentMatchesCountResult = null;
   },
 
-  requestMatchesCount: function(aWord, aMatchLimit, aLinksOnly) {
+  requestMatchesCount: function(aWord, aLinksOnly) {
     if (this._lastFindResult == Ci.nsITypeAheadFind.FIND_NOTFOUND ||
-        this.searchString == "" || !aWord) {
+        this.searchString == "" || !aWord || !this.matchesCountLimit) {
       this._notifyMatchesCount({
         total: 0,
         current: 0
       });
       return;
     }
 
     let window = this._getWindow();
     this._currentFoundRange = this._fastFind.getFoundRange();
-    this._currentMatchLimit = aMatchLimit;
 
     let params = {
       caseSensitive: this._fastFind.caseSensitive,
       entireWord: this._fastFind.entireWord,
       linksOnly: aLinksOnly,
       word: aWord
     };
     if (!this.iterator.continueRunning(params))
       this.iterator.stop();
 
     this.iterator.start(Object.assign(params, {
       finder: this,
-      limit: aMatchLimit,
+      limit: this.matchesCountLimit,
       listener: this,
       useCache: true,
     })).then(() => {
       // Without a valid result, there's nothing to notify about. This happens
       // when the iterator was started before and won the race.
       if (!this._currentMatchesCountResult || !this._currentMatchesCountResult.total)
         return;
       this._notifyMatchesCount();
@@ -448,17 +459,17 @@ Finder.prototype = {
         range.endContainer == this._currentFoundRange.endContainer &&
         range.endOffset == this._currentFoundRange.endOffset);
     }
   },
 
   onIteratorReset() {},
 
   onIteratorRestart({ word, linksOnly }) {
-    this.requestMatchesCount(word, this._currentMatchLimit, linksOnly);
+    this.requestMatchesCount(word, linksOnly);
   },
 
   onIteratorStart() {
     this._currentMatchesCountResult = {
       total: 0,
       current: 0,
       _currentFound: false
     };
--- a/toolkit/modules/FinderHighlighter.jsm
+++ b/toolkit/modules/FinderHighlighter.jsm
@@ -25,17 +25,17 @@ const kHighlightAllPref = "findbar.highl
 const kModalHighlightPref = "findbar.modalHighlight";
 const kFontPropsCSS = ["color", "font-family", "font-kerning", "font-size",
   "font-size-adjust", "font-stretch", "font-variant", "font-weight", "line-height",
   "letter-spacing", "text-emphasis", "text-orientation", "text-transform", "word-spacing"];
 const kFontPropsCamelCase = kFontPropsCSS.map(prop => {
   let parts = prop.split("-");
   return parts.shift() + parts.map(part => part.charAt(0).toUpperCase() + part.slice(1)).join("");
 });
-const kRGBRE = /^rgba?\s*\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*/i
+const kRGBRE = /^rgba?\s*\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*/i;
 // This uuid is used to prefix HTML element IDs and classNames in order to make
 // them unique and hard to clash with IDs and classNames content authors come up
 // with, since the stylesheet for modal highlighting is inserted as an agent-sheet
 // in the active HTML document.
 const kModalIdPrefix = "cedee4d0-74c5-4f2d-ab43-4d37c0f9d463";
 const kModalOutlineId = kModalIdPrefix + "-findbar-modalHighlight-outline";
 const kModalStyle = `
 .findbar-modalHighlight-outline {
@@ -475,16 +475,19 @@ FinderHighlighter.prototype = {
 
     outlineNode = dict.modalHighlightOutline;
     try {
       outlineNode.removeAttributeForElement(kModalOutlineId, "grow");
     } catch (ex) {}
     window.requestAnimationFrame(() => {
       outlineNode.setAttributeForElement(kModalOutlineId, "grow", true);
     });
+
+    if (this._highlightAll && data.searchString)
+      this.highlight(true, data.searchString, data.linksOnly);
   },
 
   /**
    * Invalidates the list by clearing the map of highglighted ranges that we
    * keep to build the mask for.
    */
   clear(window = null) {
     if (!window) {
@@ -954,18 +957,17 @@ FinderHighlighter.prototype = {
     window = window.top;
     let dict = this.getForWindow(window);
     let document = window.document;
 
     const kMaskId = kModalIdPrefix + "-findbar-modalHighlight-outlineMask";
     let maskNode = document.createElement("div");
 
     // Make sure the dimmed mask node takes the full width and height that's available.
-    let {width, height} = this._getWindowDimensions(window);
-    dict.lastWindowDimensions = { width, height };
+    let {width, height} = dict.lastWindowDimensions = this._getWindowDimensions(window);
     maskNode.setAttribute("id", kMaskId);
     maskNode.setAttribute("class", kMaskId + (kDebug ? ` ${kModalIdPrefix}-findbar-debug` : ""));
     maskNode.setAttribute("style", `width: ${width}px; height: ${height}px;`);
     if (dict.brightText)
       maskNode.setAttribute("brighttext", "true");
 
     if (paintContent || dict.modalHighlightAllMask) {
       this._updateRangeOutline(dict);
--- a/toolkit/modules/RemoteFinder.jsm
+++ b/toolkit/modules/RemoteFinder.jsm
@@ -195,20 +195,19 @@ RemoteFinder.prototype = {
     this._browser.messageManager.sendAsyncMessage("Finder:KeyPress",
                                                   { keyCode: aEvent.keyCode,
                                                     ctrlKey: aEvent.ctrlKey,
                                                     metaKey: aEvent.metaKey,
                                                     altKey: aEvent.altKey,
                                                     shiftKey: aEvent.shiftKey });
   },
 
-  requestMatchesCount: function (aSearchString, aMatchLimit, aLinksOnly) {
+  requestMatchesCount: function (aSearchString, aLinksOnly) {
     this._browser.messageManager.sendAsyncMessage("Finder:MatchesCount",
                                                   { searchString: aSearchString,
-                                                    matchLimit: aMatchLimit,
                                                     linksOnly: aLinksOnly });
   }
 }
 
 function RemoteFinderListener(global) {
   let {Finder} = Cu.import("resource://gre/modules/Finder.jsm", {});
   this._finder = new Finder(global.docShell);
   this._finder.addResultListener(this);
@@ -317,17 +316,17 @@ RemoteFinderListener.prototype = {
         this._finder.onFindbarOpen();
         break;
 
       case "Finder:KeyPress":
         this._finder.keyPress(data);
         break;
 
       case "Finder:MatchesCount":
-        this._finder.requestMatchesCount(data.searchString, data.matchLimit, data.linksOnly);
+        this._finder.requestMatchesCount(data.searchString, data.linksOnly);
         break;
 
       case "Finder:ModalHighlightChange":
         this._finder.onModalHighlightChange(data.useModalHighlight);
         break;
     }
   }
 };