Bug 1303874 - make the active window object part of the iterator params to make sure that similar iterator runs for different runs are not treated as the same, thus potentially yielding incorrect results. r=jaws
authorMike de Boer <mdeboer@mozilla.com>
Tue, 11 Oct 2016 13:08:00 +0200
changeset 360363 67e3ee9b41edc9d70863704fad44a50d283bb886
parent 360362 6e861a1843cb30d678bb4aa88c29cb9fbea61d1b
child 360364 6996ca3c1ef6cb928825a91642972a2375fb1910
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1303874
milestone52.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 1303874 - make the active window object part of the iterator params to make sure that similar iterator runs for different runs are not treated as the same, thus potentially yielding incorrect results. r=jaws MozReview-Commit-ID: H6gB2IMndM8
toolkit/modules/FinderHighlighter.jsm
toolkit/modules/FinderIterator.jsm
--- a/toolkit/modules/FinderHighlighter.jsm
+++ b/toolkit/modules/FinderHighlighter.jsm
@@ -208,17 +208,18 @@ FinderHighlighter.prototype = {
     if (highlight) {
       let params = {
         allowDistance: 1,
         caseSensitive: this.finder._fastFind.caseSensitive,
         entireWord: this.finder._fastFind.entireWord,
         linksOnly, word,
         finder: this.finder,
         listener: this,
-        useCache: true
+        useCache: true,
+        window
       };
       if (this.iterator._areParamsEqual(params, dict.lastIteratorParams))
         return this._found;
       if (params) {
         yield this.iterator.start(params);
         if (this._found)
           this.finder._outlineLink(true);
       }
--- a/toolkit/modules/FinderIterator.jsm
+++ b/toolkit/modules/FinderIterator.jsm
@@ -113,17 +113,17 @@ this.FinderIterator = {
       let { onEnd } = this._listeners.get(listener);
       if (onEnd)
         onEnd();
     }
 
     let window = finder._getWindow();
     let resolver;
     let promise = new Promise(resolve => resolver = resolve);
-    let iterParams = { caseSensitive, entireWord, linksOnly, useCache, word };
+    let iterParams = { caseSensitive, entireWord, linksOnly, useCache, window, word };
 
     this._listeners.set(listener, { limit, onEnd: resolver });
 
     // If we're not running anymore and we're requesting the previous result, use it.
     if (!this.running && this._previousResultAvailable(iterParams)) {
       this._yieldPreviousResult(listener, window);
       return promise;
     }
@@ -145,17 +145,17 @@ this.FinderIterator = {
       this._yieldIntermediateResult(listener, window);
 
       return promise;
     }
 
     // Start!
     this.running = true;
     this._currentParams = iterParams;
-    this._findAllRanges(finder, window, ++this._spawnId);
+    this._findAllRanges(finder, ++this._spawnId);
 
     return promise;
   },
 
   /**
    * Stop the currently running iterator as soon as possible and optionally cache
    * the result for later.
    *
@@ -200,17 +200,17 @@ this.FinderIterator = {
     if (!iterParams)
       return;
     this.stop();
 
     // Restart manually.
     this.running = true;
     this._currentParams = iterParams;
 
-    this._findAllRanges(finder, finder._getWindow(), ++this._spawnId);
+    this._findAllRanges(finder, ++this._spawnId);
     this._notifyListeners("restart", iterParams);
   },
 
   /**
    * Reset the internal state of the iterator. Typically this would be called
    * when the docShell is not active anymore, which makes the current and cached
    * previous result invalid.
    * If the iterator is running, it will be stopped as soon as possible.
@@ -304,16 +304,17 @@ this.FinderIterator = {
    *                                  distance'.
    * @return {Boolean}
    */
   _areParamsEqual(paramSet1, paramSet2, allowDistance = 0) {
     return (!!paramSet1 && !!paramSet2 &&
       paramSet1.caseSensitive === paramSet2.caseSensitive &&
       paramSet1.entireWord === paramSet2.entireWord &&
       paramSet1.linksOnly === paramSet2.linksOnly &&
+      paramSet1.window === paramSet2.window &&
       NLP.levenshtein(paramSet1.word, paramSet2.word) <= allowDistance);
   },
 
   /**
    * Internal; iterate over a predefined set of ranges that have been collected
    * before.
    * Also here, we make sure to pause every `kIterationSizeMax` iterations to
    * make sure we don't block the host process too long. In the case of a break
@@ -406,40 +407,39 @@ this.FinderIterator = {
     yield* this._yieldResult(listener, this.ranges, window, false);
     this._catchingUp.delete(listener);
   }),
 
   /**
    * Internal; see the documentation of the start() method above.
    *
    * @param {Finder}       finder  Currently active Finder instance
-   * @param {nsIDOMWindow} window  The window to search in
    * @param {Number}       spawnId Since `stop()` is synchronous and this method
    *                               is not, this identifier is used to learn if
    *                               it's supposed to still continue after a pause.
    * @yield {nsIDOMRange}
    */
-  _findAllRanges: Task.async(function* (finder, window, spawnId) {
+  _findAllRanges: Task.async(function* (finder, spawnId) {
     if (this._timeout) {
       if (this._timer)
         clearTimeout(this._timer);
       yield new Promise(resolve => this._timer = setTimeout(resolve, this._timeout));
       this._timer = null;
       // During the timeout, we could have gotten the signal to stop iterating.
       // Make sure we do here.
       if (!this.running || spawnId !== this._spawnId)
         return;
     }
 
     this._notifyListeners("start", this.params);
 
+    let { linksOnly, window, word } = this._currentParams;
     // First we collect all frames we need to search through, whilst making sure
     // that the parent window gets dibs.
     let frames = [window].concat(this._collectFrames(window, finder));
-    let { linksOnly, word } = this._currentParams;
     let iterCount = 0;
     for (let frame of frames) {
       for (let range of this._iterateDocument(this._currentParams, frame)) {
         // Between iterations, for example after a sleep of one cycle, we could
         // have gotten the signal to stop iterating. Make sure we do here.
         if (!this.running || spawnId !== this._spawnId)
           return;