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

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: 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.

Comment 1

[Jesse Ruderman]

Attachment: stack trace: assertion

Comment 2

[Jesse Ruderman]

Attachment: stack trace: crash

Comment 3

[Neil Deakin]

The crash is a regression from bug 743975. The assertion is a regression from bug 391834.

Comment 4

[Neil Deakin]

Attachment: 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.

Comment 5

[Neil Deakin]

Attachment: Patch with test

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

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

Have to make sure FF17 gets the fix too.

Comment 7

[Neil Deakin]

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.

Comment 8

[Neil Deakin]

Attachment: Patch with fixed test

Need to select the tab after opening it.

Comment 9

[Neil Deakin]

Checked in to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/1d89bc229215

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1d89bc229215

Comment 11

[Ed Morley [:emorley]]

Backed out for bug 787744:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e92b62f3f21

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0e92b62f3f21

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3c3b8440c478

Comment 14

[Neil Deakin]

I checked this in without the test for now.

Comment 15

[Scoobidiver (away)]

It's #2 top browser crasher on Mac OS X in 17.0a2.

Comment 16

[Jesse Ruderman]

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

[Scoobidiver (away)]

(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

[Scoobidiver (away)]

It would be fine to land the patch in Aurora before Branch 17.0 switches to Beta (#3 in 17.0a2 on Mac).

Comment 19

[Alex Keybl [:akeybl]]

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

[Scoobidiver (away)]

(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 21

[:Gavin Sharp [email: [e-mail]]]

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

Comment 22

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4a5424a18ac7

Comment 23

[Federico Padua [:fedepad]]

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.

Comment 24

[Virgil Dicu [:virgil] [QA]]

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.

Comment 25

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old