Bug 1553384, remove old find/findAgain methods in typeaheadfind in favour of a single find method, r=mikedeboer
authorNeil Deakin <neil@mozilla.com>
Wed, 18 Sep 2019 09:31:43 +0000
changeset 493764 21aa598103758f173b1f0e0a65731089567234d1
parent 493763 3977d67dc67ae062a7a4982153339fccac4a50a8
child 493765 656e05faba49669bba146c73e0db98d4cacb6caf
push id36589
push usernerli@mozilla.com
push dateWed, 18 Sep 2019 21:49:27 +0000
treeherdermozilla-central@21aff209f5a9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs1553384
milestone71.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 1553384, remove old find/findAgain methods in typeaheadfind in favour of a single find method, r=mikedeboer Differential Revision: https://phabricator.services.mozilla.com/D41232
toolkit/actors/FinderChild.jsm
toolkit/components/typeaheadfind/nsITypeAheadFind.idl
toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
toolkit/modules/ActorManagerParent.jsm
toolkit/modules/Finder.jsm
toolkit/modules/FinderParent.jsm
--- a/toolkit/actors/FinderChild.jsm
+++ b/toolkit/actors/FinderChild.jsm
@@ -39,40 +39,18 @@ class FinderChild extends JSWindowActorC
       }
 
       case "Finder:GetInitialSelection": {
         return new Promise(resolve => {
           resolve(this.finder.getActiveSelectionText());
         });
       }
 
-      case "Finder:FastFind":
-        return new Promise(resolve => {
-          let result = this.finder.fastFind(
-            data.searchString,
-            data.linksOnly,
-            data.drawOutline
-          );
-          resolve(result);
-        });
-
-      case "Finder:FindAgain":
-        return new Promise(resolve => {
-          resolve(
-            this.finder.findAgain(
-              data.searchString,
-              data.findBackwards,
-              data.linksOnly,
-              data.drawOutline
-            )
-          );
-        });
-
-      case "Finder:FindInFrame":
-        return this.finder.findInFrame(data);
+      case "Finder:Find":
+        return this.finder.find(data);
 
       case "Finder:Highlight":
         return this.finder
           .highlight(
             data.highlight,
             data.searchString,
             data.linksOnly,
             data.useSubFrames
--- a/toolkit/components/typeaheadfind/nsITypeAheadFind.idl
+++ b/toolkit/components/typeaheadfind/nsITypeAheadFind.idl
@@ -30,27 +30,20 @@ interface nsITypeAheadFind : nsISupports
    * required. */
   void init(in nsIDocShell aDocShell);
 
 
   /***************************** Core functions ****************************/
 
   /* Find aSearchString in page.  If aLinksOnly is true, only search the page's
    * hyperlinks for the string. */
-  unsigned short find(in AString aSearchString, in boolean aLinksOnly);
-
-  /* Find another match in the page. */
-  unsigned short findAgain(in boolean findBackwards, in boolean aLinksOnly);
-
-  /* Find a result only within the current frame. This is a bit misnamed,
-   * but will be removed and combined with 'find()' by a later patch. */
-  unsigned short findInFrame(in AString aSearchString,
-                             in boolean aLinksOnly,
-                             in unsigned long aMode,
-                             in boolean aDontIterateFrames);
+  unsigned short find(in AString aSearchString,
+                      in boolean aLinksOnly,
+                      in unsigned long aMode,
+                      in boolean aDontIterateFrames);
 
   /* Return the range of the most recent match. */
   Range getFoundRange();
 
 
   /**************************** Helper functions ***************************/
 
   /* Change searched docShell.  This happens when e.g. we use the same
@@ -83,17 +76,17 @@ interface nsITypeAheadFind : nsISupports
   readonly attribute Element foundEditable;
                                         // Most recent elem found, if editable
   readonly attribute mozIDOMWindow currentWindow;
                                         // Window of most recent match
 
 
   /******************************* Constants *******************************/
 
-  /* Modes for FindInFrame */
+  /* Modes for Find() */
   const unsigned long FIND_INITIAL = 0;
   const unsigned long FIND_NEXT = 1;
   const unsigned long FIND_PREVIOUS = 2;
   const unsigned long FIND_FIRST = 3;
   const unsigned long FIND_LAST = 4;
 
   /* Find return codes */
   const unsigned short FIND_FOUND    = 0;
--- a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
+++ b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
@@ -946,44 +946,20 @@ void nsTypeAheadFind::RangeStartsInsideL
     }
 
     startContent = parent;
   }
 
   *aIsStartingLink = false;
 }
 
-/* Find another match in the page. */
-NS_IMETHODIMP
-nsTypeAheadFind::FindAgain(bool aFindBackwards, bool aLinksOnly,
-                           uint16_t* aResult)
-
-{
-  *aResult = FIND_NOTFOUND;
-
-  if (!mTypeAheadBuffer.IsEmpty())
-    // Beware! This may flush notifications via synchronous
-    // ScrollSelectionIntoView.
-    FindItNow(aFindBackwards ? nsITypeAheadFind::FIND_PREVIOUS
-                             : nsITypeAheadFind::FIND_NEXT,
-              aLinksOnly, false, false, aResult);
-
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 nsTypeAheadFind::Find(const nsAString& aSearchString, bool aLinksOnly,
+                      uint32_t aMode, bool aDontIterateFrames,
                       uint16_t* aResult) {
-  return FindInternal(FIND_INITIAL, aSearchString, aLinksOnly, false, aResult);
-}
-
-NS_IMETHODIMP
-nsTypeAheadFind::FindInFrame(const nsAString& aSearchString, bool aLinksOnly,
-                             uint32_t aMode, bool aDontIterateFrames,
-                             uint16_t* aResult) {
   if (aMode == nsITypeAheadFind::FIND_PREVIOUS ||
       aMode == nsITypeAheadFind::FIND_NEXT) {
     if (mTypeAheadBuffer.IsEmpty()) {
       *aResult = FIND_NOTFOUND;
     } else {
       FindItNow(aMode, aLinksOnly, false, aDontIterateFrames, aResult);
     }
 
--- a/toolkit/modules/ActorManagerParent.jsm
+++ b/toolkit/modules/ActorManagerParent.jsm
@@ -194,19 +194,17 @@ let ACTORS = {
   // This is the actor that responds to requests from the find toolbar and
   // searches for matches and highlights them.
   Finder: {
     child: {
       moduleURI: "resource://gre/actors/FinderChild.jsm",
       messages: [
         "Finder:CaseSensitive",
         "Finder:EntireWord",
-        "Finder:FastFind",
-        "Finder:FindAgain",
-        "Finder:FindInFrame",
+        "Finder:Find",
         "Finder:SetSearchStringToSelection",
         "Finder:GetInitialSelection",
         "Finder:Highlight",
         "Finder:UpdateHighlightAndMatchCount",
         "Finder:HighlightAllChange",
         "Finder:EnableSelection",
         "Finder:RemoveSelection",
         "Finder:FocusContent",
--- a/toolkit/modules/Finder.jsm
+++ b/toolkit/modules/Finder.jsm
@@ -198,23 +198,29 @@ Finder.prototype = {
       Services.prefs.getIntPref(kMatchesCountLimitPref) || 0;
     return this._matchesCountLimit;
   },
 
   _lastFindResult: null,
 
   /**
    * Used for normal search operations, highlights the first match.
+   * This method is used only for compatibility with non-remote browsers.
    *
    * @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(aSearchString, aLinksOnly, aDrawOutline) {
-    this._lastFindResult = this._fastFind.find(aSearchString, aLinksOnly);
+    this._lastFindResult = this._fastFind.find(
+      aSearchString,
+      aLinksOnly,
+      Ci.nsITypeAheadFind.FIND_INITIAL,
+      false
+    );
     let searchString = this._fastFind.searchString;
 
     let results = {
       searchString,
       result: this._lastFindResult,
       findBackwards: false,
       findAgain: false,
       drawOutline: aDrawOutline,
@@ -226,25 +232,34 @@ Finder.prototype = {
     this.updateHighlightAndMatchCount(results);
 
     return this._lastFindResult;
   },
 
   /**
    * Repeat the previous search. Should only be called after a previous
    * call to Finder.fastFind.
+   * This method is used only for compatibility with non-remote browsers.
    *
    * @param aSearchString String to search for.
    * @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(aSearchString, aFindBackwards, aLinksOnly, aDrawOutline) {
-    this._lastFindResult = this._fastFind.findAgain(aFindBackwards, aLinksOnly);
+    let mode = aFindBackwards
+      ? Ci.nsITypeAheadFind.FIND_PREVIOUS
+      : Ci.nsITypeAheadFind.FIND_NEXT;
+    this._lastFindResult = this._fastFind.find(
+      aFindBackwards,
+      aLinksOnly,
+      mode,
+      false
+    );
     let searchString = this._fastFind.searchString;
 
     let results = {
       searchString,
       result: this._lastFindResult,
       findBackwards: aFindBackwards,
       findAgain: true,
       drawOutline: aDrawOutline,
@@ -252,18 +267,30 @@ Finder.prototype = {
       useSubFrames: true,
     };
     this._setResults(results);
     this.updateHighlightAndMatchCount(results);
 
     return this._lastFindResult;
   },
 
-  findInFrame(options) {
-    this._lastFindResult = this._fastFind.findInFrame(
+  /**
+   * Used for normal search operations, highlights the first or
+   * subsequent match depending on the mode.
+   *
+   * Options are:
+   *  searchString String to search for.
+   *  findAgain True if this a find again operation.
+   *  mode Search mode from nsITypeAheadFind.
+   *  linksOnly Only consider nodes that are links for the search.
+   *  drawOutline Puts an outline around matched links.
+   *  useSubFrames True to iterate over subframes.
+   */
+  find(options) {
+    this._lastFindResult = this._fastFind.find(
       options.searchString,
       options.linksOnly,
       options.mode,
       !options.useSubFrames
     );
     let searchString = this._fastFind.searchString;
     let results = {
       searchString,
--- a/toolkit/modules/FinderParent.jsm
+++ b/toolkit/modules/FinderParent.jsm
@@ -292,21 +292,17 @@ FinderParent.prototype = {
       aArgs.mode = mode;
 
       // A search has started for a different string, so
       // ignore further searches of the old string.
       if (this._searchString != aArgs.searchString) {
         return;
       }
 
-      response = await this.sendQueryToContext(
-        "Finder:FindInFrame",
-        aArgs,
-        currentBC
-      );
+      response = await this.sendQueryToContext("Finder:Find", aArgs, currentBC);
 
       // This can happen if the tab is closed while the find is in progress.
       if (!response) {
         break;
       }
 
       // If the search term was found, stop iterating.
       if (response.result != Ci.nsITypeAheadFind.FIND_NOTFOUND) {