1547191 - Crash in [@ ThirdPartyUtil::IsThirdPartyWindow]

Status: RESOLVED FIXED

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

Description

[Calixte Denizet (:calixte)]

This bug is for crash report bp-a2e3b45e-1ba8-48d6-ad73-31ae20190426.

Top 10 frames of crashing thread:

0 xul.dll ThirdPartyUtil::IsThirdPartyWindow dom/base/ThirdPartyUtil.cpp:200
1 xul.dll nsContentUtils::IsThirdPartyWindowOrChannel dom/base/nsContentUtils.cpp:8327
2 xul.dll mozilla::dom::Document::MaybeAllowStorageForOpenerAfterUserInteraction dom/base/Document.cpp:12192
3 xul.dll mozilla::dom::Document::SetUserHasInteracted dom/base/Document.cpp:12071
4 xul.dll mozilla::EventStateManager::PreHandleEvent dom/events/EventStateManager.cpp:458
5 xul.dll nsresult mozilla::PresShell::EventHandler::DispatchEvent layout/base/PresShell.cpp:7739
6 xul.dll nsresult mozilla::PresShell::EventHandler::HandleEventWithCurrentEventInfo layout/base/PresShell.cpp:7708
7 xul.dll mozilla::PresShell::EventHandler::HandleEventWithTarget layout/base/PresShell.cpp:7654
8 xul.dll mozilla::PresShell::HandleEventWithTarget layout/base/PresShell.h:129
9 xul.dll mozilla::PointerEventHandler::DispatchPointerFromMouseOrTouch dom/events/PointerEventHandler.cpp:543

There are 12 crashes (from 7 installations) in nightly 68 with buildid 20190425220817. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1545807.

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

Comment 1

[(no longer active)]

https://searchfox.org/mozilla-central/rev/444ee13e14fe30451651c0f62b3979c76766ada4/dom/base/ThirdPartyUtil.h#39 is getting a nullptr here since GetScriptableParent() can return null. Previously we were calling GetURIFromWindow() https://searchfox.org/mozilla-central/rev/444ee13e14fe30451651c0f62b3979c76766ada4/dom/base/ThirdPartyUtil.cpp#114 which (through GetPrincipalFromWindow) performs the null check.

Comment 2

[(no longer active)]

Attachment: Bug 1547191 - Make sure ThirdPartyUtil::GetBaseDomainFromWindow() correctly deals with nullptr arguments;

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b0f8ba4be72
Make sure ThirdPartyUtil::GetBaseDomainFromWindow() correctly deals with nullptr arguments; r=baku

Comment 4

[(no longer active)]

Comment on attachment 9061040 [details]
Bug 1547191 - Make sure ThirdPartyUtil::GetBaseDomainFromWindow() correctly deals with nullptr arguments;

Beta/Release Uplift Approval Request

• User impact if declined: We crash.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: The crash was diagnosed through code inspection.
• List of other uplifts needed: Bug 1545807
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): nullptr check.
• String changes made/needed: None

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/6b0f8ba4be72

Comment 6

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9061040 [details]
Bug 1547191 - Make sure ThirdPartyUtil::GetBaseDomainFromWindow() correctly deals with nullptr arguments;

Crash fix, approved for 67 beta 15 thanks.

Comment 7

[Raul Gurzau (:RaulG)]

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

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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