Bug 1339782 - use a WeakMap to keep track of windows with active findbar highlighters and make sure no JS error occurs when a window is closed. r?jaws
MozReview-Commit-ID: KGdvuWJZzMV
--- a/toolkit/modules/Finder.jsm
+++ b/toolkit/modules/Finder.jsm
@@ -52,22 +52,23 @@ Finder.prototype = {
return this._iterator;
this._iterator = Cu.import("resource://gre/modules/FinderIterator.jsm", null).FinderIterator;
return this._iterator;
},
destroy() {
if (this._iterator)
this._iterator.reset();
- if (this._highlighter) {
+ let window = this._getWindow();
+ if (this._highlighter && window) {
// if we clear all the references before we hide the highlights (in both
// highlighting modes), we simply can't use them to find the ranges we
// need to clear from the selection.
- this._highlighter.hide();
- this._highlighter.clear();
+ this._highlighter.hide(window);
+ this._highlighter.clear(window);
}
this.listeners = [];
this._docShell.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIWebProgress)
.removeProgressListener(this, Ci.nsIWebProgress.NOTIFY_LOCATION);
this._listeners = [];
this._currentFoundRange = this._fastFind = this._docShell = this._previousLink =
this._highlighter = null;
@@ -291,17 +292,17 @@ Finder.prototype = {
enableSelection() {
this._fastFind.setSelectionModeAndRepaint(Ci.nsISelectionController.SELECTION_ON);
this._restoreOriginalOutline();
},
removeSelection() {
this._fastFind.collapseSelection();
this.enableSelection();
- this.highlighter.clear();
+ this.highlighter.clear(this._getWindow());
},
focusContent() {
// Allow Finder listeners to cancel focusing the content.
for (let l of this._listeners) {
try {
if ("shouldFocusContent" in l &&
!l.shouldFocusContent())
@@ -470,16 +471,18 @@ Finder.prototype = {
this._currentMatchesCountResult = {
total: 0,
current: 0,
_currentFound: false
};
},
_getWindow() {
+ if (!this._docShell)
+ return null;
return this._docShell.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindow);
},
/**
* Get the bounding selection rect in CSS px relative to the origin of the
* top-level content document.
*/
_getResultRect() {
--- a/toolkit/modules/FinderHighlighter.jsm
+++ b/toolkit/modules/FinderHighlighter.jsm
@@ -126,17 +126,17 @@ function mockAnonymousContentNode(domNod
return (domNode.querySelector("#" + id) || domNode).animate(keyframes, duration);
},
setCutoutRectsForElement(id, rects) {
// no-op for now.
}
};
}
-let gWindows = new Map();
+let gWindows = new WeakMap();
/**
* FinderHighlighter class that is used by Finder.jsm to take care of the
* 'Highlight All' feature, which can highlight all find occurrences in a page.
*
* @param {Finder} finder Finder.jsm instance
*/
function FinderHighlighter(finder) {
@@ -332,19 +332,19 @@ FinderHighlighter.prototype = {
*
* @param {nsIDOMWindow} window The dimmed background will overlay this window.
* Optional, defaults to the finder window.
* @param {nsIDOMRange} skipRange A range that should not be removed from the
* find selection.
* @param {nsIDOMEvent} event When called from an event handler, this will
* be the triggering event.
*/
- hide(window = null, skipRange = null, event = null) {
+ hide(window, skipRange = null, event = null) {
try {
- window = (window || this.finder._getWindow()).top;
+ window = window.top;
} catch (ex) {
Cu.reportError(ex);
return;
}
let dict = this.getForWindow(window);
let isBusySelecting = dict.busySelecting;
dict.busySelecting = false;
@@ -407,17 +407,17 @@ FinderHighlighter.prototype = {
* by the consumer of the Finder.
*/
update(data) {
let window = this.finder._getWindow();
let dict = this.getForWindow(window);
let foundRange = this.finder._fastFind.getFoundRange();
if (data.result == Ci.nsITypeAheadFind.FIND_NOTFOUND || !data.searchString || !foundRange) {
- this.hide();
+ this.hide(window);
return;
}
if (!this._modal) {
if (this._highlightAll) {
dict.previousFoundRange = dict.currentFoundRange;
dict.currentFoundRange = foundRange;
let params = this.iterator.params;
@@ -449,24 +449,18 @@ FinderHighlighter.prototype = {
this.highlight(true, data.searchString, data.linksOnly);
},
/**
* Invalidates the list by clearing the map of highlighted ranges that we
* keep to build the mask for.
*/
clear(window = null) {
- if (!window) {
- // Since we're clearing _all the things_, make sure we hide 'em all as well.
- for (let win of gWindows.keys())
- this.hide(win);
- // Reset the Map, because no range references a node anymore.
- gWindows.clear();
+ if (!window || !window.top)
return;
- }
let dict = this.getForWindow(window.top);
this._finishOutlineAnimations(dict);
dict.dynamicRangesSet.clear();
dict.frames.clear();
dict.modalHighlightRectsMap.clear();
dict.brightText = null;
},
@@ -474,34 +468,37 @@ FinderHighlighter.prototype = {
/**
* When the current page is refreshed or navigated away from, the CanvasFrame
* contents is not valid anymore, i.e. all anonymous content is destroyed.
* We need to clear the references we keep, which'll make sure we redraw
* everything when the user starts to find in page again.
*/
onLocationChange() {
let window = this.finder._getWindow();
+ if (!window || !window.top)
+ return;
this.hide(window);
this.clear(window);
this._removeRangeOutline(window);
gWindows.delete(window.top);
},
/**
* When `kModalHighlightPref` pref changed during a session, this callback is
* invoked. When modal highlighting is turned off, we hide the CanvasFrame
* contents.
*
* @param {Boolean} useModalHighlight
*/
onModalHighlightChange(useModalHighlight) {
- if (this._modal && !useModalHighlight) {
- this.hide();
- this.clear();
+ let window = this.finder._getWindow();
+ if (window && this._modal && !useModalHighlight) {
+ this.hide(window);
+ this.clear(window);
}
this._modal = useModalHighlight;
},
/**
* When 'Highlight All' is toggled during a session, this callback is invoked
* and when it's turned off, the found occurrences will be removed from the mask.
*