1638389 - Crash in [@ nsGlobalWindowInner::ObserveStorageNotification]

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, defect, P1)

Description

[Andrew McCreight [:mccr8]]

This bug is for crash report bp-d672dc9b-c324-4c15-8038-d5d320200423.

Top 10 frames of crashing thread:

0 xul.dll nsGlobalWindowInner::ObserveStorageNotification dom/base/nsGlobalWindowInner.cpp:5053
1 xul.dll mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/dom/storage/StorageNotifierService.cpp:85:9'>::Run xpcom/threads/nsThreadUtils.h:557
2 xul.dll mozilla::SchedulerGroup::Runnable::Run xpcom/threads/SchedulerGroup.cpp:146
3 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1200
4 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:481
5 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:87
6 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
7 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290
8 xul.dll nsBaseAppShell::Run widget/nsBaseAppShell.cpp:137
9 xul.dll nsAppShell::Run widget/windows/nsAppShell.cpp:406

The crash reason is: MOZ_DIAGNOSTIC_ASSERT(StorageUtils::PrincipalsEqual(aEvent->GetPrincipal(), storagePrincipal)).

It looks like this first showed up in the 20200422214848 build. The linked crash report is from the first day this happened. There were 2 crashes on that build, then 3 crashes on the 20200424114754 build, then no crashes until the 20200510092917 build.

It looks like the volume really picked up on the 20200514094044 build.

Comment 1

[Julien Cristau [:jcristau]]

Steven can you triage this please?

Comment 2

[Steven Englehardt [:englehardt]]

Baku, would you mind to take a look? Looking like this might be related to your changes in Bug 1551055.

Comment 3

[Andrea Marchesini [:baku]]

(In reply to Steven Englehardt [:englehardt] from comment #2)

Baku, would you mind to take a look? Looking like this might be related to your changes in Bug 1551055.

That change landed 1 year ago. This a 'recent' crash. Wondering if it's related to dFPI, somehow.

Comment 4

[Gary Chen [:xeonchen]]

b94981d467ea was landed in the 20200422214848 build, and the modification to localStorage matches the case of the assertion. But I can't find other strong relevance why this would make it hit the assertion.

On the other hand, I think the assertion here is not always correct when dFPI enabled, so I'd suggest to remove the assertion or update to satisfy dFPI environment.

Comment 5

[Andrea Marchesini [:baku]]

I'm OK with changing the assertion. Is it something you can take?

Comment 6

[Gary Chen [:xeonchen]]

(In reply to Andrea Marchesini [:baku] from comment #5)

I'm OK with changing the assertion. Is it something you can take?

Sure, no problem.

Comment 7

[Gary Chen [:xeonchen]]

Attachment: Bug 1638389 - make assertion conditional;

Comment 8

[Asif Youssuff]

Seeing this crash on https://www.amazon.com/s?url=search-alias%3Daps&field-keywords=pixel+2+spigen

Crash ids:

bp-003df0d6-6ce8-4c62-ab8a-37acc0200601
bp-d915fc77-3274-4bc5-82db-1133f0200601

Comment 9

[Gary Chen [:xeonchen]]

(In reply to Asif Youssuff from comment #8)

Seeing this crash on https://www.amazon.com/s?url=search-alias%3Daps&field-keywords=pixel+2+spigen

Crash ids:

bp-003df0d6-6ce8-4c62-ab8a-37acc0200601
bp-d915fc77-3274-4bc5-82db-1133f0200601

Can you confirm that your network.cookie.cookieBehavior is set to 5?

Comment 10

[Asif Youssuff]

(In reply to Gary Chen [:xeonchen] from comment #9)

Can you confirm that your network.cookie.cookieBehavior is set to 5?

Yes, it is.

Comment 11

[Gary Chen [:xeonchen]]

Attachment: Bug 1638389 - skip events that principals are not matched;

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e9ba31526222
skip events that principals are not matched; r=timhuang

Comment 13

[Andreea Pavel [:apavel]]

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

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:xeonchen, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 15

[Gary Chen [:xeonchen]]

Set firefox78 to wontfix because:

1. The crash is caused by MOZ_DIAGNOSTIC_ASSERT and it will not affect beta but aurora channel.
2. The prerequisite of the crash is network.cookie.cookieBehavior equals to 5, this is an hidden config in all channels except in Nightly.