Closed
Bug 1391218
Opened 7 years ago
Closed 7 years ago
Browser console error thrown when closing the last tab from a FF window in a multi FF window enviroment
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | wontfix |
firefox56 | --- | verified |
firefox57 | --- | verified |
People
(Reporter: aflorinescu, Assigned: rpl)
References
Details
(Keywords: regression)
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
ochameau
:
review+
gchang
:
approval-mozilla-beta+
|
Details |
[Enviroment:] Windows 10 x64, Ubuntu 16.04, Mac OSX 10.12 55.0a1 20170602192203 57.0a1 20170816100153 [Description:] I see the error in browser console when having multiple windows opened, each containing several tabs: then you close all the tabs from that window, and after the last tab from the window, the error will pop. [Steps:] 1. Open FF and browser console. 2. Open several tabs. 3. Drag one tab out -> new window created. 4. In the new window, open several tabs. 5. Open several tabs in the new window (the window from step 3) 6. Close all the tabs from the new window. [Actual Result:] TypeError: parentDocShell.getDocShellEnumerator is not a function. (view-source:resource://devtools/server/actors/tab.js) [Note:] Initially I thought this is caused by photon work, in particular bug 1355426, but I reproduced this bug on Nightly 55, so i assume there is other cause.
Comment 1•7 years ago
|
||
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 Firefox: 57.0a1, Build ID 20170817100132 I have tested this issue on latest Firefox release (55.0.2) and latest Nightly (57.0a1) build. I have managed to reproduce it using the steps provided in the description. Considering this, using the Mozregression tool, I have managed to find a regression window. From the pushlog, it seems that the bug 1302702 has caused this. Here are the results: Last good revision: af3b8957b02d027ecf69b0a946d414ff0a91b96c First bad revision: f0df4a7f6551e886388c876f0da8b2c1de86f64f Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=af3b8957b02d027ecf69b0a946d414ff0a91b96c&tochange=f0df4a7f6551e886388c876f0da8b2c1de86f64f Luca, can you please take a look at this?
status-firefox55:
--- → affected
status-firefox56:
--- → affected
Flags: needinfo?(lgreco)
Keywords: regressionwindow-wanted → regression
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Flags: needinfo?(lgreco)
Assignee | ||
Comment 3•7 years ago
|
||
Thanks for identifying the actual STR, I digged into it and the helper function which uses the `getDocShellEnumerator` function in the tab actor: - http://searchfox.org/mozilla-central/rev/b258e6864ee3e809d40982bc5d0d5aff66a20780/devtools/server/actors/tab.js#62 it is called for a docShell instance that is coming from the "chrome-webnavigation-destroy" observer registered by the chrome actor: - http://searchfox.org/mozilla-central/rev/b258e6864ee3e809d40982bc5d0d5aff66a20780/devtools/server/actors/chrome.js#111 it looks that while in the "chrome-webnavigation-create" branch of that if statement a `subject.QueryInterface(Ci.nsIDocShell)` is called before passing it to `this._docShellCreated(subject)`, in the "chrome-webnavigation-destroy" branch (which calls `this._docShellDestroyed(subject)`) is not. I've reproduced the issue locally and then I changed it to also call `subject.QueryInterface(Ci.nsIDocShell)` before passing the subject to the `_docShellDestroyed` method and this seems to be enough to fix the issue (the `getDocShellEnumerator` function is defined as expected). I've attached a patch which applies the changes described above and pushed it to try (to be sure that it doesn't actually introduce any issue in any of the existent tests): - https://treeherder.mozilla.org/#/jobs?repo=try&revision=1486d45854a0f49a4fee82bffad2a43d0269e53f
Assignee | ||
Updated•7 years ago
|
Attachment #8899556 -
Flags: review?(poirot.alex)
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8899556 [details] Bug 1391218 - Fix 'getDocShellEnumerator is not a function' exception when closing the last tab in a window. https://reviewboard.mozilla.org/r/170864/#review176202 Thanks! ::: devtools/server/actors/chrome.js:111 (Diff revision 1) > } > if (topic == "chrome-webnavigation-create") { > subject.QueryInterface(Ci.nsIDocShell); > this._onDocShellCreated(subject); > } else if (topic == "chrome-webnavigation-destroy") { > + subject.QueryInterface(Ci.nsIDocShell); I imagine you could factorize that call by moving it to line 106/107. (same for tab.js)
Attachment #8899556 -
Flags: review?(poirot.alex) → review+
Comment hidden (mozreview-request) |
Pushed by luca.greco@alcacoop.it: https://hg.mozilla.org/integration/autoland/rev/2be1fd327e63 Fix 'getDocShellEnumerator is not a function' exception when closing the last tab in a window. r=ochameau
Updated•7 years ago
|
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2be1fd327e63
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Comment 8•7 years ago
|
||
Given that 56 is the current DevEdition, please nominate this for Beta approval when you get a chance.
status-firefox-esr52:
--- → unaffected
Flags: needinfo?(lgreco)
Assignee | ||
Comment 9•7 years ago
|
||
Comment on attachment 8899556 [details] Bug 1391218 - Fix 'getDocShellEnumerator is not a function' exception when closing the last tab in a window. Approval Request Comment [Feature/Bug causing the regression]: Bug 1302702 (changes related to "oop extension" on chrome and tabs remove debugging actors) [User impact if declined]: a "TypeError: parentDocShell.getDocShellEnumerator is not a function" exception will be logged in the browser console every time the user closes the last tab in a window, it is not currently clear if there are other side-effects that would impact on the user. [Is this code covered by automated tests?]: No (there are multiple tests that test the chrome and tabs actor, but there is no explicit tests that checks that no unexpected exception are raised when closing a tab) [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: Yes, it can be verified using the STR in Comment 0. [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: Low [Why is the change risky/not risky?]: The change is small and restricted to the observer that the DevTools tab and chrome actors subscribes to watch for the created and destroyed docShells, and ensures that `subject.QueryInterface(Ci.nsIDocShell);` is also called for a destroyed docShell, while currently it is called only for a newly created docShell. [String changes made/needed]: None
Flags: needinfo?(lgreco)
Attachment #8899556 -
Flags: approval-mozilla-beta?
Comment 10•7 years ago
|
||
Hi Adrian, Can you help check if the issue was fixed in the latest nightly?
Flags: qe-verify+
Flags: needinfo?(adrian.florinescu)
Reporter | ||
Comment 11•7 years ago
|
||
Verified as fixed: Ubuntu 16.04, OS X : 57.0a1 20170828100127
Flags: needinfo?(adrian.florinescu)
Comment 12•7 years ago
|
||
Comment on attachment 8899556 [details] Bug 1391218 - Fix 'getDocShellEnumerator is not a function' exception when closing the last tab in a window. Fix a regression and was verified. Beta56+.
Attachment #8899556 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 13•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/e8f74618acb2
Comment 14•7 years ago
|
||
I reproduced this issue using Fx 55.0a1, build ID: 20170817100132, on Windows 10 x64. I can confirm this issue is fixed, I verified using Fx 56.0b10, on Windows 10 x64, Ubuntu 14.04 LTS and mac OS X 10.12.6. Cheers!
You need to log in
before you can comment on or make changes to this bug.
Description
•