1641153 - Crash in [@ mozilla::dom::ClientSource::WindowExecutionReady]

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Calixte Denizet (:calixte)]

This bug is for crash report bp-99206295-3ab7-43a2-89b6-bcf280200527.

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::ClientSource::WindowExecutionReady dom/clients/manager/ClientSource.cpp:265
1 xul.dll nsGlobalWindowInner::ExecutionReady dom/base/nsGlobalWindowInner.cpp:1799
2 xul.dll nsGlobalWindowOuter::SetNewDocument dom/base/nsGlobalWindowOuter.cpp:2391
3 xul.dll nsDocumentViewer::InitInternal layout/base/nsDocumentViewer.cpp:960
4 xul.dll nsDocumentViewer::Init layout/base/nsDocumentViewer.cpp:749
5 xul.dll nsDocShell::SetupNewViewer docshell/base/nsDocShell.cpp:7892
6 xul.dll nsDocShell::Embed docshell/base/nsDocShell.cpp:5494
7 xul.dll nsDocShell::CreateContentViewer docshell/base/nsDocShell.cpp:7702
8 xul.dll nsDSURIContentListener::DoContent docshell/base/nsDSURIContentListener.cpp:181
9 xul.dll nsDocumentOpenInfo::TryContentListener uriloader/base/nsURILoader.cpp:630

There are 26 crashes (from 4 installations) in nightly 78 with buildid 20200526213752. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1637226.

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

Comment 1

[Tim Huang[:timhuang]]

It seems there are still some cases that could trigger this crash. I need to check this. Set the assertion back to MOZ_ASSERT() first to prevent further crashes.

Comment 2

[Tim Huang[:timhuang]]

Attachment: Bug 1641153 - Change the MOZ_DIAGNOSTIC_ASSERT to MOZ_ASSERT in ClientSource.cpp r?dimi

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/abfad4bc648f
Change the MOZ_DIAGNOSTIC_ASSERT to MOZ_ASSERT in ClientSource.cpp r=dimi

Comment 4

[Tim Huang[:timhuang]]

The crash happens when we load a nested iframe who has registered a service worker and has the storage access. It's because we don't check if the iframe is nested in the channel check but we do it in the window check. So, the nested iframe would get access from the channel check but get blocked by the window check, which causes this crash. We should check if the iframe is nested for the channel check as well.

Comment 5

[Tim Huang[:timhuang]]

Attachment: Bug 1641153 - Part 1: Add a test to ensure we won't hit the assertion for nested iframes. r?dimi!

Comment 6

[Tim Huang[:timhuang]]

Attachment: Bug 1641153 - Part 2: Add checking nested iframe in the ContentBlocking::ShouldAllowAccessFor(channel). r?dimi!

We should check the nested iframe in the
ContentBlocking::ShouldAllowAccessFor(channel) as well. This patch
implements this.

Depends on D77074

Comment 7

[Tim Huang[:timhuang]]

Attachment: Bug 1641153 - Part 3: Revert the assertion back to diagnostic assertion in ClientSource.cpp r?dimi!

Depends on D77075

Comment 8

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/abfad4bc648f

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a7e4520064c
Part 1: Add a test to ensure we won't hit the assertion for nested iframes. r=dimi
https://hg.mozilla.org/integration/autoland/rev/b0959ede7962
Part 2: Add checking nested iframe in the ContentBlocking::ShouldAllowAccessFor(channel). r=dimi
https://hg.mozilla.org/integration/autoland/rev/df38944a433e
Part 3: Revert the assertion back to diagnostic assertion in ClientSource.cpp r=dimi

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/5a7e4520064c
https://hg.mozilla.org/mozilla-central/rev/b0959ede7962
https://hg.mozilla.org/mozilla-central/rev/df38944a433e