1863046 - Remove redundant nsContentUtils::IsInPrivateBrowsing (it's also slow)

Status: NEW

(Core :: DOM: Core & HTML, enhancement)

Description

[Tom Ritter [:tjr]]

In Bug 1851816 I'm landing a new bool on Document to say whether its in PBM or not. I have a patch that removes a redundant function in nsContentUtils, but it's triggering an assertion I added to make sure things seem consistent:

[task 2023-11-03T16:19:23.310Z] 16:19:23     INFO - GECKO(14596) | Assertion failure: !mIsInPrivateBrowsing || mIsInPrivateBrowsing == loadContext->UsePrivateBrowsing(), at /builds/worker/checkouts/gecko/dom/base/Document.cpp:2953
[task 2023-11-03T16:19:23.321Z] 16:19:23     INFO -  Initializing stack-fixing for the first stack frame, this may take a while...
[task 2023-11-03T16:19:51.160Z] 16:19:51     INFO - GECKO(14596) | #01: mozilla::dom::Document::ResetToURI(nsIURI*, nsILoadGroup*, nsIPrincipal*, nsIPrincipal*) [dom/base/Document.cpp:2952]
[task 2023-11-03T16:19:51.162Z] 16:19:51     INFO - GECKO(14596) | #02: nsHTMLDocument::ResetToURI(nsIURI*, nsILoadGroup*, nsIPrincipal*, nsIPrincipal*) [dom/html/nsHTMLDocument.cpp:169]
[task 2023-11-03T16:19:51.163Z] 16:19:51     INFO - GECKO(14596) | #03: nsContentDLF::CreateBlankDocument(nsILoadGroup*, nsIPrincipal*, nsIPrincipal*, nsDocShell*) [layout/build/nsContentDLF.cpp:234]
[task 2023-11-03T16:19:51.164Z] 16:19:51     INFO - GECKO(14596) | #04: nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIPrincipal*, nsIContentSecurityPolicy*, nsIURI*, bool, mozilla::Maybe<nsILoadInfo::CrossOriginEmbedderPolicy> const&, bool, bool, mozilla::dom::WindowGlobalChild*) [docshell/base/nsDocShell.cpp:6650]
[task 2023-11-03T16:19:51.165Z] 16:19:51     INFO - GECKO(14596) | #05: nsDocShell::EnsureContentViewer() [docshell/base/nsDocShell.cpp:6488]
[task 2023-11-03T16:19:51.166Z] 16:19:51     INFO - GECKO(14596) | #06: nsDocShell::GetDocument() [docshell/base/nsDocShell.cpp:3111]
[task 2023-11-03T16:19:51.167Z] 16:19:51     INFO - GECKO(14596) | #07: nsDocShell::InternalLoad(nsDocShellLoadState*, mozilla::Maybe<unsigned int>) [docshell/base/nsDocShell.cpp:9485]
[task 2023-11-03T16:19:51.168Z] 16:19:51     INFO - GECKO(14596) | #08: nsDocShell::LoadURI(nsDocShellLoadState*, bool, bool) [docshell/base/nsDocShell.cpp:850]
[task 2023-11-03T16:19:51.168Z] 16:19:51     INFO - GECKO(14596) | #09: nsFrameLoader::ReallyStartLoadingInternal() [dom/base/nsFrameLoader.cpp:783]
[task 2023-11-03T16:19:51.169Z] 16:19:51     INFO - GECKO(14596) | #10: nsFrameLoader::ReallyStartLoading() [dom/base/nsFrameLoader.cpp:650]
[task 2023-11-03T16:19:51.170Z] 16:19:51     INFO - GECKO(14596) | #11: mozilla::dom::Document::MaybeInitializeFinalizeFrameLoaders() [dom/base/Document.cpp:9411]
[task 2023-11-03T16:19:51.172Z] 16:19:51     INFO - GECKO(14596) | #12: mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0, >::Run() [xpcom/threads/nsThreadUtils.h:1213]
[task 2023-11-03T16:19:51.173Z] 16:19:51     INFO - GECKO(14596) | #13: nsContentUtils::RemoveScriptBlocker() [dom/base/nsContentUtils.cpp:5988]
[task 2023-11-03T16:19:51.173Z] 16:19:51     INFO - GECKO(14596) | #14: mozilla::dom::Document::EndUpdate() [dom/base/Document.cpp:7976]
[task 2023-11-03T16:19:51.174Z] 16:19:51     INFO - GECKO(14596) | #15: nsHtml5AutoFlush::~nsHtml5AutoFlush() [parser/html/nsHtml5TreeOpExecutor.cpp:109]
[task 2023-11-03T16:19:51.175Z] 16:19:51     INFO - GECKO(14596) | #16: nsHtml5TreeOpExecutor::RunFlushLoop() [parser/html/nsHtml5TreeOpExecutor.cpp:723]
[task 2023-11-03T16:19:51.176Z] 16:19:51     INFO - GECKO(14596) | #17: nsHtml5ExecutorFlusher::Run() [parser/html/nsHtml5StreamParser.cpp:176]
[task 2023-11-03T16:19:51.177Z] 16:19:51     INFO - GECKO(14596) | #18: mozilla::RunnableTask::Run() [xpcom/threads/TaskController.cpp:550]
[task 2023-11-03T16:19:51.177Z] 16:19:51     INFO - GECKO(14596) | #19: mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) [xpcom/threads/TaskController.cpp:876]
[task 2023-11-03T16:19:51.178Z] 16:19:51     INFO - GECKO(14596) | #20: mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) [xpcom/threads/TaskController.cpp:0]
[task 2023-11-03T16:19:51.180Z] 16:19:51     INFO - GECKO(14596) | #21: mozilla::TaskController::ProcessPendingMTTask(bool) [xpcom/threads/TaskController.cpp:485]
[task 2023-11-03T16:19:51.181Z] 16:19:51     INFO - GECKO(14596) | #22: mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() [xpcom/threads/nsThreadUtils.h:549]
[task 2023-11-03T16:19:51.182Z] 16:19:51     INFO - GECKO(14596) | #23: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:1202]
[task 2023-11-03T16:19:51.184Z] 16:19:51     INFO - GECKO(14596) | #24: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/threads/nsThreadUtils.cpp:480]
[task 2023-11-03T16:19:51.185Z] 16:19:51     INFO - GECKO(14596) | #25: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:85]
[task 2023-11-03T16:19:51.185Z] 16:19:51     INFO - GECKO(14596) | #26: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:346]
[task 2023-11-03T16:19:51.186Z] 16:19:51     INFO - GECKO(14596) | #27: nsBaseAppShell::Run() [widget/nsBaseAppShell.cpp:150]
[task 2023-11-03T16:19:51.188Z] 16:19:51     INFO - GECKO(14596) | #28: XRE_RunAppShell() [toolkit/xre/nsEmbedFunctions.cpp:721]
[task 2023-11-03T16:19:51.189Z] 16:19:51     INFO - GECKO(14596) | #29: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:235]
[task 2023-11-03T16:19:51.190Z] 16:19:51     INFO - GECKO(14596) | #30: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:346]
[task 2023-11-03T16:19:51.192Z] 16:19:51     INFO - GECKO(14596) | #31: XRE_InitChildProcess(int, char**, XREChildData const*) [toolkit/xre/nsEmbedFunctions.cpp:660]
[task 2023-11-03T16:19:51.368Z] 16:19:51     INFO - GECKO(14596) | #32: main [browser/app/nsBrowserApp.cpp:375]
[task 2023-11-03T16:19:51.371Z] 16:19:51     INFO - GECKO(14596) | #33: __libc_start_main [/lib/x86_64-linux-gnu/libc.so.6 + 0x21b97]
[task 2023-11-03T16:19:51.379Z] 16:19:51     INFO - GECKO(14596) | #34: ??? [/builds/worker/workspace/build/application/firefox/firefox-bin + 0x3d8a9]
[task 2023-11-03T16:19:51.380Z] 16:19:51     INFO - GECKO(14596) | #35: ??? (???:???)

This is happening on the test docshell/test/browser/browser_onbeforeunload_frame.js (on Linux 64 debug if that matters.)

Comment 1

[Tom Ritter [:tjr]]

Attachment: Bug 1863046. Remove redundant nsContentUtils::IsInPrivateBrowsing (it's also slow). r?#dom-core,timhuang,tjr

Depends on D192501

Comment 2

[Tom Ritter [:tjr]]

I'm not if my prior patch will have any real bearing on this, but I rebased and put in a try run: https://treeherder.mozilla.org/jobs?repo=try&revision=26e52939eeae962ab0d92276229e9f8eefa5559a

Comment 3

[Timothy Nikkel (:tnikkel)]

I came across this because I noticed the queryinterfaces in nsContentUtils::IsInPrivateBrowsing in profiles involving loading images and implemented the same fix as you before finding this bug and realizing you had done it.

I pushed to try with the assert in question uncommented and active and we don't seem to hit it on try anymore (including in the mentioned test).

Do you want to revive this and finish it up? If you prefer, I can push this over the finish line.

Comment 4

[Tom Ritter [:tjr]]

I would be happy to let you push it over the finish line =)

Comment 5

[Timothy Nikkel (:tnikkel)]

Thanks!