790856 - Window resize accessed a dangling DocumentViewerImpl

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Jesse Ruderman]

Attachment: stack traces from ASan

I wasn't able to reproduce this.  Can you figure it out from the stacks, and knowing that the fuzzer was opening and closing windows?

Comment 1

[Olli Pettay [:smaug]]

Probably.

Comment 2

[Olli Pettay [:smaug]]

  if (mPreviousViewer)
    mPreviousViewer->SetBounds(aBounds);
looks suspicious. We call set bounds on the previous viewer, and at least in theory that could
end up doing stuff which sets mPreviousViewer null.

Comment 3

[Olli Pettay [:smaug]]

Attachment: patch

Jesse, want to try this? Though, if reproducing is hard, verifying the
fix can be difficult.

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

Do we know what versions are affected by this bug?

Comment 5

[Olli Pettay [:smaug]]

If my patch fixes the problem, then everything after 2.0 or something.

Comment 6

[Boris Zbarsky [:bzbarsky]]

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

r=me

Comment 7

[Olli Pettay [:smaug]]

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

[Approval Request Comment]
User impact if declined: possible crash
Fix Landed on Version: NA
Risk to taking this patch (and alternatives if risky): Should be super-safe
String or UUID changes made by this patch: NA

[Approval Request Comment]
Regression caused by (bug #): Bug 290991
User impact if declined: Possible crashes
Testing completed (on m-c, etc.): NA

Comment 8

[Alex Keybl [:akeybl]]

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

[Triage Comment]
Approving for all unreleased branches given this is sec-critical.

Comment 9

[Olli Pettay [:smaug]]

Oh, did I accidentally ask a? for release. Wasn't going to.

Comment 11

[Abhishek Arya]

(In reply to Olli Pettay [:smaug] from comment #3)
> Created attachment 660748 [details] [diff] [review]
> patch
> 
> Jesse, want to try this? Though, if reproducing is hard, verifying the
> fix can be difficult.

Tested your patch, it indeed fixes the crash.

Comment 12

[Alex Keybl [:akeybl]]

(In reply to Olli Pettay [:smaug] from comment #9)
> Oh, did I accidentally ask a? for release. Wasn't going to.

Given the verification, please land asap on all branches. Thanks!

Comment 13

[Olli Pettay [:smaug]]

https://hg.mozilla.org/mozilla-central/rev/48c4938eaf57
https://hg.mozilla.org/releases/mozilla-aurora/rev/c039da3793f7
https://hg.mozilla.org/releases/mozilla-beta/rev/5013b57e5c40
https://hg.mozilla.org/releases/mozilla-esr10/rev/62ad5b34715d