1672873 - Assertion failure: aParam.mStateData->PortIdentifiers().IsEmpty() && aParam.mStateData->BlobImpls().IsEmpty() && aParam.mStateData->InputStreams().IsEmpty(), at src/docshell/shistory/SessionHistoryEntry.cpp:1343

Status: VERIFIED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Assertion failure: aParam.mStateData->PortIdentifiers().IsEmpty() && aParam.mStateData->BlobImpls().IsEmpty() && aParam.mStateData->InputStreams().IsEmpty(), at src/docshell/shistory/SessionHistoryEntry.cpp:1343

#0 0x7f42556fe7dc in mozilla::ipc::IPDLParamTraits<mozilla::dom::SessionHistoryInfo>::Write(IPC::Message*, mozilla::ipc::IProtocol*, mozilla::dom::SessionHistoryInfo const&) src/docshell/shistory/SessionHistoryEntry.cpp:1341:5
#1 0x7f42508dc57a in mozilla::dom::PContentChild::SendSetActiveSessionHistoryEntry(mozilla::dom::MaybeDiscarded<mozilla::dom::BrowsingContext> const&, mozilla::Maybe<nsPoint> const&, mozilla::dom::SessionHistoryInfo const&, unsigned int const&, int const&, unsigned int const&, nsID const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentChild.cpp:7210:5
#2 0x7f42556783cb in mozilla::dom::BrowsingContext::SetActiveSessionHistoryEntry(mozilla::Maybe<nsPoint> const&, mozilla::dom::SessionHistoryInfo*, unsigned int, int, unsigned int) src/docshell/base/BrowsingContext.cpp:2743:35
#3 0x7f42556cbcac in nsDocShell::UpdateActiveEntry(bool, mozilla::Maybe<nsPoint> const&, nsIURI*, nsIURI*, nsIPrincipal*, nsIContentSecurityPolicy*, nsTSubstring<char16_t> const&, mozilla::Maybe<bool> const&, nsIStructuredCloneContainer*, bool) src/docshell/base/nsDocShell.cpp:11628:23
#4 0x7f42556cb0f7 in nsDocShell::UpdateURLAndHistory(mozilla::dom::Document*, nsIURI*, nsIStructuredCloneContainer*, nsTSubstring<char16_t> const&, bool, nsIURI*, bool) src/docshell/base/nsDocShell.cpp:11117:7
#5 0x7f42556ca3ea in nsDocShell::AddState(JS::Handle<JS::Value>, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, bool, JSContext*) src/docshell/base/nsDocShell.cpp:11047:8
#6 0x7f4251ba3f22 in nsHistory::PushOrReplaceState(JSContext*, JS::Handle<JS::Value>, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::dom::CallerType, mozilla::ErrorResult&, bool) src/dom/base/nsHistory.cpp:265:19
#7 0x7f4251ba3d5d in nsHistory::PushState(JSContext*, JS::Handle<JS::Value>, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::dom::CallerType, mozilla::ErrorResult&) src/dom/base/nsHistory.cpp:216:3
#8 0x7f4252d35442 in mozilla::dom::History_Binding::pushState(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/HistoryBinding.cpp:377:24
#9 0x7f4252d691fa in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3229:13
#10 0x7f4255ce95a1 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) src/js/src/vm/Interpreter.cpp:506:13
#11 0x7f4255ce8cb8 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:598:12
#12 0x7f4255cea883 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:663:10
#13 0x7f4255cde623 in CallFromStack src/js/src/vm/Interpreter.cpp:667:10
#14 0x7f4255cde623 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3336:16
#15 0x7f4255cd5794 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:476:13
#16 0x7f4255ce8c89 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:635:13
#17 0x7f4255cea883 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:663:10
#18 0x7f4255ceaabf in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) src/js/src/vm/Interpreter.cpp:680:8
#19 0x7f42562d4f0b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/jsapi.cpp:2829:10
#20 0x7f4252a9cbbc in mozilla::dom::EventListener::HandleEvent(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/EventListenerBinding.cpp:57:8
#21 0x7f42530fd6a6 in void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/EventListenerBinding.h:66:12
#22 0x7f42530fd3ed in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) src/dom/events/EventListenerManager.cpp:1072:43
#23 0x7f42530fe092 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) src/dom/events/EventListenerManager.cpp:1269:17
#24 0x7f42530f3362 in HandleEvent /builds/worker/workspace/obj-build/dist/include/mozilla/EventListenerManager.h:354:5
#25 0x7f42530f3362 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:352:17
#26 0x7f42530f2913 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:554:16
#27 0x7f42530f5420 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) src/dom/events/EventDispatcher.cpp:1092:11
#28 0x7f42546fe9e7 in nsDocumentViewer::LoadComplete(nsresult) src/layout/base/nsDocumentViewer.cpp:1095:7
#29 0x7f42556b28b1 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) src/docshell/base/nsDocShell.cpp:6450:20
#30 0x7f42556b229a in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp:5803:7
#31 0x7f42556b318f in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp
#32 0x7f42510708dc in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) src/uriloader/base/nsDocLoader.cpp:1348:3
#33 0x7f425106fe8a in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:954:14
#34 0x7f425106e21c in nsDocLoader::DocLoaderIsEmpty(bool, mozilla::Maybe<nsresult> const&) src/uriloader/base/nsDocLoader.cpp:757:9
#35 0x7f425106f2cd in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:640:5
#36 0x7f425106faac in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp
#37 0x7f424ffea776 in mozilla::net::nsLoadGroup::NotifyRemovalObservers(nsIRequest*, nsresult) src/netwerk/base/nsLoadGroup.cpp:615:22
#38 0x7f424ffebc83 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:522:10
#39 0x7f4251a343c1 in mozilla::dom::Document::DoUnblockOnload() src/dom/base/Document.cpp:10924:18
#40 0x7f4251a13110 in mozilla::dom::Document::UnblockOnload(bool) src/dom/base/Document.cpp:10854:9
#41 0x7f4251a238ed in mozilla::dom::Document::DispatchContentLoadedEvents() src/dom/base/Document.cpp:7413:3
#42 0x7f4251a94956 in applyImpl<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1188:12
#43 0x7f4251a94956 in apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1194:12
#44 0x7f4251a94956 in mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1240:13
#45 0x7f424fe56912 in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:146:20
#46 0x7f424fe5bdbf in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:245:16
#47 0x7f424fe5a43a in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:515:26
#48 0x7f424fe594e4 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:374:15
#49 0x7f424fe59697 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:171:36
#50 0x7f424fe5f2e6 in operator() src/xpcom/threads/TaskController.cpp:85:37
#51 0x7f424fe5f2e6 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_3>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:577:5
#52 0x7f424fe70537 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1197:14
#53 0x7f424fe760da in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:513:10
#54 0x7f4250766086 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:87:21
#55 0x7f42506d8813 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:334:10
#56 0x7f42506d872d in RunHandler src/ipc/chromium/src/base/message_loop.cc:327:3
#57 0x7f42506d872d in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:309:3
#58 0x7f42543b6358 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#59 0x7f4255baf413 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:913:20
#60 0x7f4250766e49 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:237:9
#61 0x7f42506d8813 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:334:10
#62 0x7f42506d872d in RunHandler src/ipc/chromium/src/base/message_loop.cc:327:3
#63 0x7f42506d872d in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:309:3
#64 0x7f4255baeff8 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:744:34
#65 0x55a5f5c6c657 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#66 0x55a5f5c6c657 in main src/browser/app/nsBrowserApp.cpp:304:18

Comment 1

[Tyson Smith [:tsmith]]

Attachment: prefs.js

Comment 2

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Unable to reproduce bug using the following builds:

mozilla-central 20201027044126-46a0e993f8bb
mozilla-central 20201022093646-03de9a8a6f7c
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Comment 3

[Kris Maglione [:kmag]]

It would be nice if these were three separate assertions, so we could tell which of them was failing. It is pretty surprising that any of them would fail, though... I can't see any plausible way for it to happen if we're passing in undefined.

Comment 4

[Neha Kochar [:neha]]

Tyson, can you please re-test and confirm if this still reproduces, since bugmon couldn't reproduce (comment 2)?

Comment 5

[Tyson Smith [:tsmith]]

I cannot reproduce the issues with attached test case but the fuzzers are still hitting it. I found a new test case and I am reducing it now. I will attach it once reduction complete.

Comment 6

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Comment 7

[Tyson Smith [:tsmith]]

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

Comment 8

[Kris Maglione [:kmag]]

OK, that testcase is entirely unsurprising. It actually does try to store a Blob in the pushState data, which we don't actually guard against, and then assert can't happen.

And that probably actually does work without SHiP. So the question is, do we want to disallow it everywhere, or try to support it in SHiP?

Comment 9

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20201106041434-81a3ef82469b.
Failed to bisect testcase (Start build crashes!):

Start: 478c5bf5ccb37d3a38bda833a8751fd43db58ff1 (20191108171434)
End: 03de9a8a6f7c949b046b5a1197988391ede9e84f (20201022093646)
BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=False, coverage=False, valgrind=False)

Comment 10

[Chris Peterson [:cpeterson]]

(In reply to Jason Kratzer [:jkratzer] from comment #9)

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20201106041434-81a3ef82469b.
Failed to bisect testcase (Start build crashes!):

Start: 478c5bf5ccb37d3a38bda833a8751fd43db58ff1 (20191108171434)
End: 03de9a8a6f7c949b046b5a1197988391ede9e84f (20201022093646)
BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=False, coverage=False, valgrind=False)

Tyson, does this Bugmon analysis mean 20201106041434-81a3ef82469b is the regressing changeset? Or that the regression range is between End: 03de9a8a6f7c949b046b5a1197988391ede9e84f (20201022093646) and 20201106041434-81a3ef82469b?

Tracking for Fission M7 Beta for now.

Comment 11

[Tyson Smith [:tsmith]]

(In reply to Chris Peterson [:cpeterson] from comment #10)

Tyson, does this Bugmon analysis mean 20201106041434-81a3ef82469b is the regressing changeset? Or that the regression range is between End: 03de9a8a6f7c949b046b5a1197988391ede9e84f (20201022093646) and 20201106041434-81a3ef82469b?

We can repro the bug: Verified bug as reproducible on mozilla-central 20201106041434-81a3ef82469b.
We can't bisect: Failed to bisect testcase (Start build crashes!):

So the bug goes back further than the oldest build available to us on TC.

:jkratzer, perhaps you can make the message a bit more descriptive?

Comment 12

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

This bug has a fairly clear cause. It seems that we don't specify the correct restrictions on what data should be supported by the structured clone container created for storing the shared state, meaning that we support serializing some types which we can't actually meaningfully serialize, including Messageports, DOM Blobs, and input streams. When Chrome serializes data in their pushState implementation they serialize it with their kForStorage flag (https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/core/frame/history.cc;l=241;drc=24842514ee052f9b2e785bf1a06f33ac90a710a4), which presumably prohibits data which couldn't be persisted to disk and read on a different run of the browser, and we likely want to do the same thing.

We already use our rough analogue of this type (the DifferentProcess scope) for serializing this data, but we don't also disable cloning and transferring with the StructuredCloneHolder constructor, which we likely need to expose through the nsStructuredCloneHolder (and ipc::StructuredCloneData) constructor. https://searchfox.org/mozilla-central/rev/69babd862de70cabfa1d0a369d38e4881bd41e4d/dom/base/StructuredCloneHolder.h#173-187

It my also be necessary to audit the specific things disabled by setting aSupportsCloning to CloningNotSupported, as I don't currently see any callers which set that flag, and there may be something which we want to support here (though I think it's unlikely) https://searchfox.org/mozilla-central/rev/69babd862de70cabfa1d0a369d38e4881bd41e4d/dom/base/StructuredCloneHolder.cpp#1014-1016

Finally, we may need to tweak the assertion a bit to check HasClonedDOMObjects() instead of the getters for each type, as they assert mSupportsCloning is true: https://searchfox.org/mozilla-central/rev/69babd862de70cabfa1d0a369d38e4881bd41e4d/dom/base/StructuredCloneHolder.h#215-232

Comment 13

[Andrew McCreight [:mccr8]]

Neha asked me to take a look. Hopefully with Nika's detailed comment I can make some headway.

Comment 14

[Andrew McCreight [:mccr8]]

Attachment: Bug 1672873 - Use BuildClonedMessageDataForChild to send cloneable data in SessionHistoryInfo.

The serialization method for SessionHistoryInfo uses some low-level
functions to pack up some of the clone data, but this method
actually has a PContent actor available, so it can use one of the
nicer BuildClonedMessageDataFor methods to send this, which
should improve support for pushMessage with blobs.

The read method already uses the StealFromClonedMessageDataFor
methods so no changes are required there.

Comment 15

[Peter Van der Beken [:peterv]]

Attachment: Bug 1672873 - Add test for pushState with serializable objects in the state. r?smaug!

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68c64e421a98
Use BuildClonedMessageDataForChild to send cloneable data in SessionHistoryInfo. r=peterv

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a050ee554da8
Add test for pushState with serializable objects in the state. r=smaug

Comment 18

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/68c64e421a98
https://hg.mozilla.org/mozilla-central/rev/a050ee554da8

Comment 19

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

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 20

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20210216031051-00b18dc4bfac.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Comment 22

[Andrew McCreight [:mccr8]]

This fix is specific to Fission, so there's no need to backport.