1680418 - Assertion failure: contentType.Equals(aResult->mSharedState.Get()->mContentType) (We don't expect this to change!), at src/docshell/shistory/SessionHistoryEntry.cpp:1472

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Assertion failure: contentType.Equals(aResult->mSharedState.Get()->mContentType) (We don't expect this to change!), at src/docshell/shistory/SessionHistoryEntry.cpp:1472

#0 0x7f1ebd721c1f in mozilla::ipc::IPDLParamTraits<mozilla::dom::SessionHistoryInfo>::Read(IPC::Message const*, PickleIterator*, mozilla::ipc::IProtocol*, mozilla::dom::SessionHistoryInfo*) src/docshell/shistory/SessionHistoryEntry.cpp:1471:5
#1 0x7f1eb88ec4ab in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentParent.cpp:13271:20
#2 0x7f1eb872bb8e in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2153:25
#3 0x7f1eb87282ad in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2077:9
#4 0x7f1eb8729756 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1925:3
#5 0x7f1eb872a37b in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1956:13
#6 0x7f1eb7e1811f in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:452:16
#7 0x7f1eb7e1678a in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:722:26
#8 0x7f1eb7e15834 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:581:15
#9 0x7f1eb7e159e7 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:375:36
#10 0x7f1eb7e1b9f6 in operator() src/xpcom/threads/TaskController.cpp:122:37
#11 0x7f1eb7e1b9f6 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_3>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:485:5
#12 0x7f1eb7e2cf77 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1196:14
#13 0x7f1eb7e3301a in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:513:10
#14 0x7f1eb8731426 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:87:21
#15 0x7f1eb869e793 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:334:10
#16 0x7f1eb869e6ad in RunHandler src/ipc/chromium/src/base/message_loop.cc:327:3
#17 0x7f1eb869e6ad in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:309:3
#18 0x7f1ebc3cfef8 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#19 0x7f1ebdabbf0e in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:271:30
#20 0x7f1ebdbc5926 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:5105:22
#21 0x7f1ebdbc6ad5 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:5297:8
#22 0x7f1ebdbc7380 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:5353:21
#23 0x55bd9309c991 in do_main src/browser/app/nsBrowserApp.cpp:219:22
#24 0x55bd9309c991 in main src/browser/app/nsBrowserApp.cpp:337:16

Comment 1

[Tyson Smith [:tsmith]]

Attachment: prefs.js

Comment 2

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/_CO1wyYCLxsm8FNTxS7yvQ/index.html

Comment 3

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Comment 4

[Chris Peterson [:cpeterson]]

Olli, this is a fuzz bug related to session history, so it's seems important.

Tracking for Fission M6c.

Comment 5

[Peter Van der Beken [:peterv]]

The new content type is text/html, but the existing shared state in the parent has an empty content type.

Comment 6

[Nika Layzell [:nika] (ni? for response)]

The new content type is coming from the content process as when the same-document navigation entry is created, it reads the content type from nsDocShell::mContentTypeHint here: https://searchfox.org/mozilla-central/rev/71621bfa47a371f2b1ccfd33c704913124afb933/docshell/base/nsDocShell.cpp#8923. This seems like perhaps the wrong behaviour, as that mContentTypeHint is only a hint, set by DoLoadURI when starting a load as a hint as to what the load's content type should be. (https://searchfox.org/mozilla-central/rev/71621bfa47a371f2b1ccfd33c704913124afb933/docshell/base/nsDocShell.cpp#10230-10234)

I haven't had enough time to look into the purpose of mContentType yet on SHEntryShared, but I'm guessing the content type isn't being configured correctly for session history entries if it's exclusively based on the content-provided mContentTypeHint.

Comment 7

[Nika Layzell [:nika] (ni? for response)]

I've looked into this a bit more. It seems like in the old case, we'd create a brand new session history entry using mContentHint as we do today from HandleSameDocumentNavigation -> OnNewURI -> AddToSessionHistory. This new entry would have a brand new SHEntryShared object, with these new attributes configured on it.

However, this is a same-document navigation, so it can't use a different SHEntryShared object than the currently active entry. This is fixed almost immediately after the call to OnNewURI by calling AdoptBFCacheEntry. This completely clobbered the previous shared state from the call in AddToSessionHistory, replacing it with the existing shared state.

I think that this means we need to adjust our behaviour for same-document navigations. Specifically, I think we'll want to switch the call in HandleSameDocumentNavigation to use the 2-argument form of the constructor if mActiveEntry is non-null, and the many-argument version if it's null, like we do inUpdateActiveEntry. We can then also probably drop the now-unused aSharedStateFrom first parameter to that many-argument constructor.

I think this should have the correct behaviour of ignoring the shared state configuration which is used when creating the entry as it will be replaced with the existing shared state, and probably will fix the crash.

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1680418 - avoid clobbering shared state in same-document navigations,

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb7a2b31aebb
avoid clobbering shared state in same-document navigations, r=peterv

Comment 10

[Sandor Molnar[:smolnar]]

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