830948 - Crash [@ nsGlobalWindow::GetInnerHeight(int*)] while clicking View -> Zoom -> Zoom In on Firefox Aurora 20

Status: VERIFIED FIXED

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

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Crash signature: bp-9e55cde4-113e-4441-b673-3bcbf2130115

I was using Aurora 20 build ID 20130115042018. In a new private browsing window, I was doing a Google search, opening 4 tabs, and closing 3 after I finished reading them. Using a mouse, I then clicked on View -> Zoom -> Zoom In and it immediately crashed.

The crashing line is at http://hg.mozilla.org/releases/mozilla-aurora/annotate/57fc7a3fcd2b/dom/base/nsGlobalWindow.cpp#l3704

Jesse suggests that something might have messed up mDocShell in line 3704, but we are not entirely sure if EnsureSizeUpToDate() might have been the culprit.

Comment 1

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

Indeed, EnsureSizeUpToDate flushes, which may cause scripts to run, which may end up
closing the window so mDocShell may be null.

Comment 2

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

Attachment: patch

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Nominating for tracking, this is easy to hit from the end-user side. I'm not sure how far back this affects though.

Comment 4

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

(In reply to Olli Pettay [:smaug] from comment #2)
> Created attachment 702500 [details] [diff] [review]
> patch
Note, as far as I see, EnsureSizeUpToDate doesn't actually do anything if there isn't mDocshell.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 702500 [details] [diff] [review]
patch

r=me

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/43135307225f

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I poked at the source and it seems that this bug is present across all branches.

Comment 8

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

Comment on attachment 702500 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): old stuff
User impact if declined: crashes
Testing completed (on m-c, etc.): landed to m-c yesterday 
Risk to taking this patch (and alternatives if risky): null check
String or UUID changes made by this patch: NA

The patch seems to apply cleanly everywhere.

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

This doesn't qualify for esr landing, but will track for 19 and approve for branches.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

This is also in my queue to land on Aurora once it reopens.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/595310c37d98

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 702500 [details] [diff] [review]
patch

This doesn't seem to fix any obvious problems on b2g18, so minus for landing there. If that's not a correct assessment, please re-nominate.

Comment 13

[Paul Silaghi, QA [:pauly]]

I couldn't reproduce the issue in comment 0. Checking the crash stats, this looks good in general, but I see 1 crash on FF 19b3 - https://crash-stats.mozilla.com/report/index/5338111a-d778-47d1-a2a4-7e7dc2130129. Any thoughts?

Comment 14

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Paul Silaghi [QA] from comment #13)
> I couldn't reproduce the issue in comment 0. Checking the crash stats, this
> looks good in general, but I see 1 crash on FF 19b3 -
> https://crash-stats.mozilla.com/report/index/5338111a-d778-47d1-a2a4-
> 7e7dc2130129. Any thoughts?

That crash crashes at a different line, at http://hg.mozilla.org/releases/mozilla-beta/annotate/8848df2565b6/dom/base/nsGlobalWindow.cpp#l3634 - I've filed bug 838806 for this.

Since crash-stats does not seem to show anymore related crashes -> VERIFIED

Comment 15

[Paul Silaghi, QA [:pauly]]

Based on comment 14