Assertion/crash when calling modal alert() from a closing window

RESOLVED FIXED in Firefox 17

Status

()

--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jruderman, Assigned: enndeakin)

Tracking

(5 keywords)

17 Branch
mozilla18
x86_64
Mac OS X
assertion, crash, regression, testcase, topcrash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17+ verified)

Details

(crash signature)

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 653494 [details]
testcase

1. Set 
     user_pref("prompts.tab_modal.enabled", false);
2. Load the testcase
3. Click "Open evil tab" button
4. Click red button

Result: 
###!!! ASSERTION: DialogsAreBlocked() called without a top window?: 'Error', file ../../../dom/base/nsGlobalWindow.cpp, line 2534

Or, if you click "open evil window" instead of "open evil tab",
Crash [@ nsGlobalWindow::ActivateOrDeactivate]

I suspect this is a regression from bug 391834.  Might be related to bug 692553 and/or bug 715398.
(Reporter)

Comment 1

6 years ago
Created attachment 653495 [details]
stack trace: assertion
(Reporter)

Comment 2

6 years ago
Created attachment 653496 [details]
stack trace: crash

Updated

6 years ago
Crash Signature: [@ nsGlobalWindow::ActivateOrDeactivate]
(Assignee)

Comment 3

6 years ago
The crash is a regression from bug 743975. The assertion is a regression from bug 391834.
Blocks: 743975
(Assignee)

Comment 4

6 years ago
Created attachment 653798 [details] [diff] [review]
Patch

The old code used to null-check what GetWidgetListener now returns. I'm not sure why we should be calling ActivateOrDeactivate is this case but I'll investigate that separately.

Is this a Mac specific crash?

This patch restores the null-check and the old version of the assertion which checked the docshell instead of the window.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
(Assignee)

Comment 5

6 years ago
Created attachment 655973 [details] [diff] [review]
Patch with test
Attachment #653798 - Attachment is obsolete: true
Attachment #655973 - Flags: review?(bugs)

Comment 6

6 years ago
Comment on attachment 655973 [details] [diff] [review]
Patch with test

Have to make sure FF17 gets the fix too.
Attachment #655973 - Flags: review?(bugs) → review+
tracking-firefox17: --- → +
Attachment #655973 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 7

6 years ago
There's a strange test failure when I run this, but I cannot reproduce locally.

https://tbpl.mozilla.org/php/getParsedLog.php?id=14777373&tree=Try

Continuing to investigate.
Attachment #655973 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 8

6 years ago
Created attachment 656992 [details] [diff] [review]
Patch with fixed test

Need to select the tab after opening it.
Attachment #655973 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/1d89bc229215
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Depends on: 787744
Backed out for bug 787744:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e92b62f3f21
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla18 → ---

Updated

6 years ago
Version: Trunk → 17 Branch
https://hg.mozilla.org/mozilla-central/rev/3c3b8440c478
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
(Assignee)

Comment 14

6 years ago
I checked this in without the test for now.

Updated

6 years ago
status-firefox17: --- → affected

Comment 15

6 years ago
It's #2 top browser crasher on Mac OS X in 17.0a2.
Keywords: topcrash
(Reporter)

Comment 16

6 years ago
Are those crashes all from users with prompts.tab_modal.enabled set to false?  Or is there another way to trigger this bug?  Or is the topcrash a different bug?

It's not a topcrash on 18, which is a good sign.

Comment 17

6 years ago
(In reply to Jesse Ruderman from comment #16)
> Or is the topcrash a different bug?
The topcrash keyword is for Aurora where this patch hasn't landed. See https://crash-stats.mozilla.com/report/list?signature=nsGlobalWindow%3A%3AActivateOrDeactivate

Comment 18

6 years ago
It would be fine to land the patch in Aurora before Branch 17.0 switches to Beta (#3 in 17.0a2 on Mac).
Neil - please nominate for Aurora 17 uplift as soon as possible. As Scoobidiver notes, we'd like to land there before uplifting to Beta.

Comment 20

6 years ago
(In reply to Alex Keybl [:akeybl] from comment #19)
> Neil - please nominate for Aurora 17 uplift as soon as possible.
#1 top crasher in Aurora on Mac!
Comment on attachment 656992 [details] [diff] [review]
Patch with fixed test

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 743975
User impact if declined: lots of crashes
Testing completed (on m-c, etc.): on m-c since 2012-09-05
Risk to taking this patch (and alternatives if risky): none, it's a null check
String or UUID changes made by this patch: none
Attachment #656992 - Flags: approval-mozilla-aurora?
Attachment #656992 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I tested on :

Firefox 17.0a2
User Agent : Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID : 20121005042010

There are no problems, no crashes, it appears to work fine.
Keywords: verifyme
Tested on:
Firefox 17.0 Beta Debug
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20121020072533

I opened Firefox from Terminal and ran the testcase from comment 0. After running step 4, no assertion message was found. Also by clicking the "Open evil window" and then "Close & alert" no crash occurred and Firefox was still running.
status-firefox17: fixed → verified
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.