Closed
Bug 1282671
Opened 8 years ago
Closed 8 years ago
"Assertion failure: !win->IsDying()" with long-script interrupt and window.close()
Categories
(Core :: DOM: Core & HTML, defect, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla52
People
(Reporter: jruderman, Assigned: bhsu)
Details
(Keywords: assertion, testcase)
Attachments
(4 files)
1. Set the prefs mentioned in the testcase 2. Load the testcase 3. Click two buttons Result: Assertion failure: !win->IsDying(), at /Users/jruderman/trees/mozilla-central/js/xpconnect/src/XPCJSRuntime.cpp:1456 Expected: script terminated by timeout
Reporter | ||
Comment 1•8 years ago
|
||
Reporter | ||
Comment 2•8 years ago
|
||
I can't reproduce when running Firefox under lldb. Does running under lldb disable the script-timeout feature??
Reporter | ||
Comment 3•8 years ago
|
||
This callback runs arbitrary code, so asserting that the window is not dying seems bogus. Bobby?
Flags: needinfo?(bobbyholley)
Comment 5•8 years ago
|
||
Yeah, we should probably just check and bail out like we do for the non-window case, though probably in this case we should return false and kill the script. r=me on a patch that replaces the assert with: if (win->IsDying()) { // The window is being torn down. When that happens we try to prevent the dispatch of new runnables, // so it also makes sense to kill any long-running script. The user is primarily interested in this // page going away. return false; }
Flags: needinfo?(bobbyholley)
Updated•8 years ago
|
Priority: -- → P2
Assignee | ||
Comment 7•8 years ago
|
||
Sure, but I am new to this field, it would take me a while, and thus I will working on this without taking it :)
Flags: needinfo?(bhsu)
Assignee | ||
Comment 8•8 years ago
|
||
Hi Jesse, and sorry for such a late response After diving into the code a little bit, I found that the case is still reproducible by executing firefox in non-e10s mode, which you can do that by the following command. ``` ./mach run --disable-e10s ``` With e10s enabled, it seems that Gecko doesn't immediately close the nsGlobalWindow after invoking `window.close()`, and thus the nsGlobalWindow is not marked as dying. As a result, we don't get hit by the assertion. Please refer to http://searchfox.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#8607-8609 for more detailed information :)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bhsu
Assignee | ||
Comment 9•8 years ago
|
||
Assignee | ||
Comment 10•8 years ago
|
||
Comment on attachment 8800501 [details] [diff] [review] Loose the assertion checking win->IsDying() Per comment 5, I set the flag to r+
Attachment #8800501 -
Flags: review+
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8800501 [details] [diff] [review] Loose the assertion checking win->IsDying() Per comment 5, I set this patch to r+
Assignee | ||
Comment 12•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=aec25af40696
Keywords: checkin-needed
Comment 13•8 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ba66a8bb5b3d Loose the assertion checking win->IsDying(). r=bholly
Keywords: checkin-needed
Comment 14•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ba66a8bb5b3d
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•