1926456 - Crash in [@ mozilla::dom::CanonicalBrowsingContext::ClearRestoreState] with NS_SUCCEEDED(SetHasRestoreData(false))

Status: RESOLVED FIXED

(Core :: Session Restore, defect)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/0a5d4873-709d-484b-a62c-e34690241022

MOZ_CRASH Reason:

MOZ_DIAGNOSTIC_ASSERT(false) (NS_SUCCEEDED(SetHasRestoreData(false)))

Top 10 frames:

0  libxul.so  mozilla::dom::CanonicalBrowsingContext::ClearRestoreState()  docshell/base/CanonicalBrowsingContext.cpp:2622
1  libxul.so  mozilla::dom::PWindowGlobalParent::OnMessageReceived(IPC::Message const&)  ipc/ipdl/PWindowGlobalParent.cpp:0
2  libxul.so  mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&)  ipc/ipdl/PContentParent.cpp:6435
3  libxul.so  mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecyc...  ipc/glue/MessageChannel.cpp:1785
3  libxul.so  mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecyclePro...  ipc/glue/MessageChannel.cpp:1712
3  libxul.so  mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, ...  ipc/glue/MessageChannel.cpp:1503
3  libxul.so  mozilla::ipc::MessageChannel::MessageTask::Run()  ipc/glue/MessageChannel.cpp:1603
4  libxul.so  mozilla::RunnableTask::Run()  xpcom/threads/TaskController.cpp:618
4  libxul.so  mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::det...  xpcom/threads/TaskController.cpp:945
5  libxul.so  mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detai...  xpcom/threads/TaskController.cpp:768

This showed up on the Nightly crash spike report. It is on Android. Maybe related to the Android SHIP rollout?

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

That is part of session restore, implemented in bug 1702055 and elsewhere.

Comment 2

[Peter Van der Beken [:peterv]]

I looked into these crashes a bit, but I'm clearly missing something. I think we always set HasRestoreData and mRestoreState together. https://searchfox.org/mozilla-central/rev/360b702e1ec793c49dc66122e50fe580d43350b3/docshell/base/CanonicalBrowsingContext.cpp#2657-2658 is a bit weird, since it sets HasRestoreData to false but doesn’t clear mRestoreState, but that should not trigger the assertion.

Comment 3

[Andreas Farre [:farre]]

I think that the problem is that this can call CanonicalBrowsingContext::ClearRestoreState() async, which means that the context could've been discarded while we were restoring.

Comment 4

[Andreas Farre [:farre]]

Attachment: Bug 1926456 - Skip discarded contexts when setting synced field. r=peterv!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a521dd562a5
Skip discarded contexts when setting synced field. r=peterv

Comment 6

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/2a521dd562a5