1717497 - Crash in [@ mozilla::dom::SessionStoreDataCollector::CollectSessionStoreData]

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Andrew McCreight [:mccr8]]

Maybe Fission related. (DOMFissionEnabled=1)

Looks like this showed up in the 20210616214200 build. All of the non-beta crashes have Fission enabled.

Crash report: https://crash-stats.mozilla.org/report/index/008e88c8-f422-4820-a53c-4c6470210620

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(listener->mTimer)

Top 10 frames of crashing thread:

0 libxul.so mozilla::dom::SessionStoreDataCollector::CollectSessionStoreData toolkit/components/sessionstore/SessionStoreDataCollector.cpp:51
1 libxul.so mozilla::dom::SessionStoreChangeListener::HandleEvent toolkit/components/sessionstore/SessionStoreChangeListener.cpp:92
2 libxul.so {virtual override thunk} 
3 libxul.so mozilla::EventListenerManager::HandleEventInternal dom/events/EventListenerManager.cpp:1306
4 libxul.so mozilla::EventTargetChainItem::HandleEventTargetChain dom/events/EventDispatcher.cpp:593
5 libxul.so mozilla::EventTargetChainItem::HandleEventTargetChain dom/events/EventDispatcher.cpp:637
6 libxul.so mozilla::EventDispatcher::Dispatch dom/events/EventDispatcher.cpp:1116
7 libxul.so mozilla::dom::VisualViewport::FireScrollEvent dom/base/VisualViewport.cpp:299
8 libxul.so mozilla::dom::VisualViewport::VisualViewportScrollEvent::Run dom/base/VisualViewport.cpp:280
9 libxul.so nsRefreshDriver::Tick layout/base/nsRefreshDriver.cpp:2274

Comment 1

[Andrew McCreight [:mccr8]]

This is the set of patches added in that build: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=593c9e377f44615d071e2e31a726aa2db31eee27&tochange=36647ef6f014ee7199a0bad93851750ead132473

Kashav, any ideas?

Comment 2

[u608768]

This might be bug 1701337, which made it so we no longer create mTimer if browser.sessionstore.debug.no_auto_updates is true... but that's a debug-only pref. Is there any way to find out if it's enabled in these crashes?

Comment 3

[Andrew McCreight [:mccr8]]

No, prefs are not recorded unless you add a crash annotation for them. There is a ExperimentalFeatures section under ProtectedData that I can see which does contain a bunch of pref, but that pref isn't in there. I don't know what determines if a pref is there or not. Not all of them look like things we have experiments for.

Comment 4

[Andrew McCreight [:mccr8]]

I guess at a glance I did see a few of the prefs in that section in this file, so maybe that's what decides whether they are included or not.

Comment 5

[u608768]

I guess it's a bug regardless since we shouldn't be asserting we have a timer if there's now a path where we don't create one.

Comment 6

[u608768]

Attachment: Bug 1717497 - Only assert we have a timer if we expect to have created one, r?farre

Comment 7

[u608768]

Attachment: Bug 1717497 - Wait for the _restoreHistory() promise before firing SSTabRestored, r?annyG

Depends on D118101

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3de4da02449
Only assert we have a timer if we expect to have created one, r=farre

Comment 9

[Phabricator Automation]

Comment on attachment 9228370 [details]
Bug 1717497 - Wait for the _restoreHistory() promise before firing SSTabRestored, r?annyG

Revision D118501 was moved to bug 1696815. Setting attachment 9228370 [details] to obsolete.

Comment 10

[Alexandru Michis [:malexandru]]

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

Comment 11

[Chris Peterson [:cpeterson]]

Retroactively assigning this fixed bug to Fission Milestone M7a.