1298323 - Javascript error 'this._docShell is null' after closing [n+1]th window

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect, P1)

Description

[Mike de Boer [:mikedeboer]]

In bug 384458 I implemented a garbage collection mechanism, but that's doing too much: it destroys the Finder instance for the entire content process when only one window is closed (this firing the 'unload' event).
This is an unfortunate mistake, but easy to fix.

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1298323 - do not destroy the Finder instance when only a single window closes in e10s mode, because that render the findbar useless.

Review commit: https://reviewboard.mozilla.org/r/74484/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/74484/

Comment 2

[Mike de Boer [:mikedeboer]]

[Tracking Requested - why for this release]:
This was introduced in bug 384458, which we disabled, but didn't back out. Therefore we need to uplift this patch.

Comment 3

[Mike de Boer [:mikedeboer]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=316def26dbc4

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8785190 [details]
Bug 1298323 - do not destroy the Finder instance when only a single window closes in e10s mode, because that render the findbar useless.

https://reviewboard.mozilla.org/r/74484/#review72430

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2c5773f47093
do not destroy the Finder instance when only a single window closes in e10s mode, because that render the findbar useless. r=jaws

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2c5773f47093

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Mike, should we uplift this fix to Beta50?

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked since it's a recent regression.

Comment 9

[Mike de Boer [:mikedeboer]]

Comment on attachment 8785190 [details]
Bug 1298323 - do not destroy the Finder instance when only a single window closes in e10s mode, because that render the findbar useless.

Approval Request Comment
[Feature/regressing bug #]: bug 384458
[User impact if declined]: Findbar may become unusable after a while.
[Describe test coverage new/current, TreeHerder]: landed on m-c, now on m-a, passing all tests.
[Risks and why]: minor, reverts back to the old mechanism of findbar lifetime management.
[String/UUID change made/needed]: n/a

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8785190 [details]
Bug 1298323 - do not destroy the Finder instance when only a single window closes in e10s mode, because that render the findbar useless.

Fixes a recent regression, Beta50+

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for Beta.

Comment 12

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e613221b1959

Comment 13

[Mike de Boer [:mikedeboer]]

To verify this bug, you need to perform the following steps:

1. Start Firefox and open two windows
2. In each window, use the findbar at least once by typing in its input box
3. Close one of the windows
4. In the window that's still open, close & re-open the findbar and use it again by typing in its input box
5. No error should appear in the Browser Console.

Comment 14

[Bogdan Maris, Desktop QA]

Reproduced the error from old Nightly (2016-09-26)
> TypeError: this._docShell is nullFinder.jsm:64:5

Verified that using Firefox 50 beta 3 and latest Developer Edition 51.0a2 across platforms (Ubuntu 16.04 32-bit, Mac OS X 10.10.5 and Windows 10 64-bit).

Comment 15

[Bogdan Maris, Desktop QA]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #14)
> Reproduced the error from old Nightly (2016-09-26)

Ups, actually I tested on Nightly from 2016-_08_-26, sorry for this.