1333371 - Crash in nsPIDOMWindow<T>::GetDocGroup

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Flyweb, defect)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is 
report bp-a1b34bc7-505c-4dfc-8d3f-d41b72170123.
=============================================================

There is one crash on nightly with build-id 20170123163559 and in analyzing the backtrace it could be a regression introduced by patch [1] to fix bug 1314833.

[1] https://hg.mozilla.org/mozilla-central/rev?node=d3cfaafedb4339897fcede34a67125a4d16a7b98

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

The aOwner could be null from the caller at:
http://searchfox.org/mozilla-central/source/dom/flyweb/FlyWebPublishedServer.cpp#482
I'll provide a patch to fix this asap.

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch: Fallback to AbstractThread::MainThread() if the inner window is not provided.

Hi Bill,

Sorry for not capturing this problem in bug 1314833.
May I have your review for this quick fix?

Thanks!

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

treeherder builds look fine:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=64b111c6e5a064a957d6aa8d7164bedfd681b647

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4477cdb38b11
Fallback to AbstractThread::MainThread() if the inner window is not provided. r=billm

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4477cdb38b11

Comment 6

[Justin D'Arcangelo [:justindarc]]

btseng: We're still crashing with this patch. We were not crashing until Bug 1314833 landed. See the new log of the crash here:

https://bugzilla.mozilla.org/show_bug.cgi?id=1333909#c7

Comment 7

[Justin D'Arcangelo [:justindarc]]

Removing ni? flag since this issue is now resolved.

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

change status to resolved according to comment 7.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comments 6 & 7 paint a rather muddy picture of things, but OK. Please request Aurora approval on this patch when you get a chance.

Comment 10

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8830169 [details] [diff] [review]
Patch: Fallback to AbstractThread::MainThread() if the inner window is not provided.

Approval Request Comment
[Feature/Bug causing the regression]:Bug 1314833.
[User impact if declined]:Crash when accessing FlyWeb API.
[Is this code covered by automated tests?]:There is no test case in dom/flyweb but was manually tested accorrding to https://bugzilla.mozilla.org/show_bug.cgi?id=1333909#c8
[Has the fix been verified in Nightly?]:Yes, it was manually tested in https://bugzilla.mozilla.org/show_bug.cgi?id=1333909#c8
[Needs manual test from QE? If yes, steps to reproduce]: No. It's been tested according to https://bugzilla.mozilla.org/show_bug.cgi?id=1333909#c8
[List of other uplifts needed for the feature/fix]:No.
[Is the change risky?]:No.
[Why is the change risky/not risky?]:It's not risky because we have add a nullptr check to prevent the same crash happens again.
[String changes made/needed]:No.

Comment 11

[Gerry Chang [:gchang]]

Comment on attachment 8830169 [details] [diff] [review]
Patch: Fallback to AbstractThread::MainThread() if the inner window is not provided.

Fix a crash and was verified in nightly. Aurora53+.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/514a5268030f