1814314 - Assertion failure: mPresContext->mLayoutPhaseCount[nsLayoutPhase::Paint] == 0 (constructing frames in the middle of a paint), at /builds/worker/checkouts/gecko/layout/base/nsAutoLayoutPhase.cpp:65

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.zip

Found while fuzzing m-c 20221212-e6f125e02b6f (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.zip

Assertion failure: mPresContext->mLayoutPhaseCount[nsLayoutPhase::Paint] == 0 (constructing frames in the middle of a paint), at /builds/worker/checkouts/gecko/layout/base/nsAutoLayoutPhase.cpp:65

#0 0x7f5f1b371a04 in nsAutoLayoutPhase::Enter() /builds/worker/checkouts/gecko/layout/base/nsAutoLayoutPhase.cpp:64:7
#1 0x7f5f1b395271 in nsAutoLayoutPhase /builds/worker/checkouts/gecko/layout/base/nsAutoLayoutPhase.cpp:18:3
#2 0x7f5f1b395271 in nsCSSFrameConstructor::ContentAppended(nsIContent*, nsCSSFrameConstructor::InsertionKind) /builds/worker/checkouts/gecko/layout/base/nsCSSFrameConstructor.cpp:6541:3
#3 0x7f5f1b33686e in mozilla::PresShell::ContentAppended(nsIContent*) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:4553:22
#4 0x7f5f179c2b44 in operator() /builds/worker/checkouts/gecko/dom/base/MutationObservers.cpp:162:3
#5 0x7f5f179c2b44 in Notify<(IsRemoval)0, (ShouldAssert)1, (lambda at /builds/worker/checkouts/gecko/dom/base/MutationObservers.cpp:162:3), (lambda at /builds/worker/checkouts/gecko/dom/base/MutationObservers.cpp:162:3)> /builds/worker/checkouts/gecko/dom/base/MutationObservers.cpp:97:5
#6 0x7f5f179c2b44 in mozilla::dom::MutationObservers::NotifyContentAppended(nsIContent*, nsIContent*) /builds/worker/checkouts/gecko/dom/base/MutationObservers.cpp:163:3
#7 0x7f5f17b429e0 in nsINode::InsertChildBefore(nsIContent*, nsIContent*, bool, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/base/nsINode.cpp:1608:7
#8 0x7f5f17b4a423 in nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/base/nsINode.cpp:2825:5
#9 0x7f5f180fbf5b in InsertBefore /builds/worker/checkouts/gecko/dom/base/nsINode.h:2065:12
#10 0x7f5f180fbf5b in AppendChild /builds/worker/checkouts/gecko/dom/base/nsINode.h:2072:12
#11 0x7f5f180fbf5b in mozilla::dom::Node_Binding::appendChild(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/NodeBinding.cpp:997:60
#12 0x7f5f190011d2 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/dom/bindings/BindingUtils.cpp:3308:13
#13 0x7f5f1d42d336 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:459:13
#14 0x7f5f1d42cc5f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:547:12
#15 0x7f5f1d41e89f in CallFromStack /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:619:10
#16 0x7f5f1d41e89f in Interpret(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:3362:16
#17 0x7f5f1d411f5e in js::RunScript(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:431:13
#18 0x7f5f1d42cb5b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:579:13
#19 0x7f5f1d42e08c in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:646:8
#20 0x7f5f1d4e91dc in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/CallAndConstruct.cpp:53:10
#21 0x7f5f16c34250 in nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) /builds/worker/checkouts/gecko/js/xpconnect/src/XPCWrappedJSClass.cpp:918:17
#22 0x7f5f15e79e52 in PrepareAndDispatch /builds/worker/checkouts/gecko/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:115:37
#23 0x7f5f15e7915a in SharedStub xptcstubs_x86_64_linux.cpp
#24 0x7f5f198de238 in mozilla::dom::TextTrackManager::UpdateCueDisplay() /builds/worker/checkouts/gecko/dom/html/TextTrackManager.cpp:276:19
#25 0x7f5f198e01d3 in mozilla::dom::TextTrackManager::TimeMarchesOn() /builds/worker/checkouts/gecko/dom/html/TextTrackManager.cpp:825:3
#26 0x7f5f19874276 in mozilla::dom::HTMLMediaElement::RunAutoplay() /builds/worker/checkouts/gecko/dom/html/HTMLMediaElement.cpp:6112:26
#27 0x7f5f1b5b64b9 in nsVideoFrame::OnVisibilityChange(mozilla::Visibility, mozilla::Maybe<mozilla::OnNonvisible> const&) /builds/worker/checkouts/gecko/layout/generic/nsVideoFrame.cpp:543:11
#28 0x7f5f1b4f8468 in nsIFrame::IncApproximateVisibleCount() /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:2342:3
#29 0x7f5f1b33e9b2 in mozilla::PresShell::EnsureFrameInApproximatelyVisibleList(nsIFrame*) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:6287:13
#30 0x7f5f1b4bc733 in nsIFrame::BuildDisplayListForChild(mozilla::nsDisplayListBuilder*, nsIFrame*, mozilla::nsDisplayListSet const&, mozilla::EnumSet<nsIFrame::DisplayChildFlag, unsigned int>) /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:4208:25
#31 0x7f5f1b457e0d in DisplayLine(mozilla::nsDisplayListBuilder*, nsLineList_iterator&, bool, mozilla::nsDisplayListSet const&, nsBlockFrame*, mozilla::css::TextOverflow*, unsigned int, int, int&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:7059:13
#32 0x7f5f1b456788 in nsBlockFrame::BuildDisplayList(mozilla::nsDisplayListBuilder*, mozilla::nsDisplayListSet const&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:7214:9
#33 0x7f5f1b504ec4 in nsIFrame::BuildDisplayListForSimpleChild(mozilla::nsDisplayListBuilder*, nsIFrame*, mozilla::nsDisplayListSet const&) /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:4009:11
#34 0x7f5f1b4bc05d in nsIFrame::BuildDisplayListForChild(mozilla::nsDisplayListBuilder*, nsIFrame*, mozilla::nsDisplayListSet const&, mozilla::EnumSet<nsIFrame::DisplayChildFlag, unsigned int>) /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:4113:5
#35 0x7f5f1b45cf7c in nsCanvasFrame::BuildDisplayList(mozilla::nsDisplayListBuilder*, mozilla::nsDisplayListSet const&) /builds/worker/checkouts/gecko/layout/generic/nsCanvasFrame.cpp:584:5
#36 0x7f5f1b504ec4 in nsIFrame::BuildDisplayListForSimpleChild(mozilla::nsDisplayListBuilder*, nsIFrame*, mozilla::nsDisplayListSet const&) /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:4009:11
#37 0x7f5f1b4bc05d in nsIFrame::BuildDisplayListForChild(mozilla::nsDisplayListBuilder*, nsIFrame*, mozilla::nsDisplayListSet const&, mozilla::EnumSet<nsIFrame::DisplayChildFlag, unsigned int>) /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:4113:5
#38 0x7f5f1b4bfa3a in mozilla::ScrollFrameHelper::BuildDisplayList(mozilla::nsDisplayListBuilder*, mozilla::nsDisplayListSet const&) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:4332:15
#39 0x7f5f1b504ec4 in nsIFrame::BuildDisplayListForSimpleChild(mozilla::nsDisplayListBuilder*, nsIFrame*, mozilla::nsDisplayListSet const&) /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:4009:11
#40 0x7f5f1b4bc05d in nsIFrame::BuildDisplayListForChild(mozilla::nsDisplayListBuilder*, nsIFrame*, mozilla::nsDisplayListSet const&, mozilla::EnumSet<nsIFrame::DisplayChildFlag, unsigned int>) /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:4113:5
#41 0x7f5f1b42c94d in mozilla::ViewportFrame::BuildDisplayList(mozilla::nsDisplayListBuilder*, mozilla::nsDisplayListSet const&) /builds/worker/checkouts/gecko/layout/generic/ViewportFrame.cpp:66:3
#42 0x7f5f1b5027a1 in nsIFrame::BuildDisplayListForStackingContext(mozilla::nsDisplayListBuilder*, mozilla::nsDisplayList*, bool*) /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:3463:5
#43 0x7f5f1b3cc766 in nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, mozilla::nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) /builds/worker/checkouts/gecko/layout/base/nsLayoutUtils.cpp:3349:15
#44 0x7f5f1b33f321 in mozilla::PresShell::PaintInternal(nsView*, mozilla::PaintInternalFlags) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:6463:5
#45 0x7f5f1af29282 in nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) /builds/worker/checkouts/gecko/view/nsViewManager.cpp:433:18
#46 0x7f5f1af28d7f in nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) /builds/worker/checkouts/gecko/view/nsViewManager.cpp:368:22
#47 0x7f5f1af2a21c in nsViewManager::ProcessPendingUpdates() /builds/worker/checkouts/gecko/view/nsViewManager.cpp:941:5
#48 0x7f5f1b2f95d4 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsRefreshDriver::IsExtraTick) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2804:11
#49 0x7f5f1b3080f2 in operator() /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:1784:25
#50 0x7f5f1b3080f2 in mozilla::detail::RunnableFunction<nsRefreshDriver::EnsureTimerStarted(nsRefreshDriver::EnsureTimerStartedFlags)::$_1>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:546:5
#51 0x7f5f15e2d2e5 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:539:16
#52 0x7f5f15e288bc in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:852:26
#53 0x7f5f15e2748a in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:684:15
#54 0x7f5f15e277e5 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:462:36
#55 0x7f5f15e30be6 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:188:37
#56 0x7f5f15e30be6 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_2>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:546:5
#57 0x7f5f15e460e5 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1197:16
#58 0x7f5f15e4c40d in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:477:10
#59 0x7f5f16a4f2d3 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
#60 0x7f5f16970ec8 in MessageLoop::RunInternal() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:381:10
#61 0x7f5f16970dd1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:374:3
#62 0x7f5f16970dd1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:356:3
#63 0x7f5f1af903c8 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
#64 0x7f5f1d1e2cbb in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:742:20
#65 0x7f5f16a50199 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:235:9
#66 0x7f5f16970ec8 in MessageLoop::RunInternal() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:381:10
#67 0x7f5f16970dd1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:374:3
#68 0x7f5f16970dd1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:356:3
#69 0x7f5f1d1e2818 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:675:34
#70 0x55e1e4c7ece0 in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#71 0x55e1e4c7ece0 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:353:18
#72 0x7f5f29626d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#73 0x7f5f29626e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#74 0x55e1e4c55348 in _start (/home/user/workspace/browsers/m-c-20230131093335-fuzzing-debug/firefox-bin+0x5b348) (BuildId: e895ae0929e70cb9ac9412a3bebaddc87d8b87d8)

Comment 1

[Bugmon [:jkratzer for issues]]

Unable to reproduce bug 1814314 using build mozilla-central 20221212094833-e6f125e02b6f. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 2

[Daniel Holbert [:dholbert]]

I can reproduce with the steps in comment 0, but:

• I can't reproduce when I try to just directly load the extracted testcase in a debug build (I used python3 -m http.server 8123 to serve the testcase over http, in case it matters, since I noticed grizzly seems to use a local web server).
• I also can't reproduce when I directly load the testcase in the Firefox build that fuzzfetch downloaded for me, with a fresh profile.

So presumably one of the fuzzing default-enabled about:config prefs is required here.

jkratzer (or Tyson when you're back from PTO), would you mind generating a pernosco session for this?

(It also might be interesting to check why bugmon had trouble reproducing, per comment 1. If it's using something like Tyson's STR in comment 0, it seems like it should be able to reproduce... I can reproduce ~immediately with those steps.)

Comment 3

[Tyson Smith [:tsmith]]

ni? myself, in case Jason does not get to this before I'm back.

Comment 4

[Jason Kratzer [:jkratzer]]

It looks like the reason that bugmon can't reproduce this is because we run bugmon using xvfb. This testcase does not reproduce when run under xvfb. I'll go ahead and get a pernosco session for this.

Comment 5

[Daniel Holbert [:dholbert]]

Thanks!

Comment 6

[Jason Kratzer [:jkratzer]]

A pernosco session for this bug can be found here.

Comment 7

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

The severity field is not set for this bug.
:boris, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Timothy Nikkel (:tnikkel)]

Based on the stack this is a regression from bug 1803834.

Comment 9

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

Set release status flags based on info from the regressing bug 1803834

Comment 10

[Boris Chiou [:boris]]

This is a regression by bug 1803834, so I cc Alastor as well.

Comment 11

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

:alwu, since you are the author of the regressor, bug 1803834, could you take a look?

For more information, please visit auto_nag documentation.

Comment 12

[Alastor Wu [:alwu]]

bug 1803834 implements an optional behavior in the html spec, where we would resume the non-audible autoplay when it starts intersecting with the viewport.

From the callstack in the comment0, the visibility change triggered resuming the media element, which has a text track, so it also triggered updating the text track in order to show the correct subtitle based on the media time. The updating cues would be done in vtt.jsm which would calculate how cues should be rendered and append corresponding div elements. That appending seems triggering some layout mechanism causing an assertion? I have no idea about the whatever happens in the layout part, but the media parts look reasonable to me.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

HTMLMediaElement::RunAutoplay needs to use a script runner. You can't modify the DOM while painting. Alastor can you take a look and fix it?

Comment 14

[Emilio Cobos Álvarez (:emilio)]

This is a soundness bug and can cause security issues.

Comment 15

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

Set release status flags based on info from the regressing bug 1803834

Comment 16

[Alastor Wu [:alwu]]

Attachment: Bug 1814314 - use the script runner for calling ProcessCues.

Comment 17

[Alastor Wu [:alwu]]

Comment on attachment 9323102 [details]
Bug 1814314 - use the script runner for calling ProcessCues.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Hard because this function call has many possible call paths and my patch didn't expose any information about that.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: 109
• If not all supported branches, which bug introduced the flaw?: Bug 1803834
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: This patch can be uploaded to previous branches as well.
• How likely is this patch to cause regressions; how much testing does it need?: Low, because if the function is running in the expected situation, my patch won't cause any changes. It only addresses the case where the crash might happen by delaying processing the cues' style. Even assume that it would cause a regression, it would only affect how subtitle would be rendered on the screen, which is a very minor effect.
• Is Android affected?: Unknown

Comment 18

[Tom Ritter [:tjr]]

Comment on attachment 9323102 [details]
Bug 1814314 - use the script runner for calling ProcessCues.

approved to land and uplift

Comment 19

[Alastor Wu [:alwu]]

Attachment: Bug 1814314 - use the script runner for calling ProcessCues.

Original Revision: https://phabricator.services.mozilla.com/D172606

Comment 20

[Alastor Wu [:alwu]]

Ni for uplift later.

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

use the script runner for calling ProcessCues. r=emiliohttps://hg.mozilla.org/integration/autoland/rev/0377760deeb3174a0e86844575ac3f7c77888966
https://hg.mozilla.org/mozilla-central/rev/0377760deeb3

Comment 22

[Alastor Wu [:alwu]]

Comment on attachment 9323102 [details]
Bug 1814314 - use the script runner for calling ProcessCues.

Beta/Release Uplift Approval Request

• User impact if declined: Potential unexpected security issue might happen per the comment14, because modifying DOM during repainting is something shouldn't happen.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch delays the call to the script when necessary, but in most cases it won't change anything. It doesn't change the code logic or introduce any new feature.
• String changes made/needed: No
• Is Android affected?: Unknown

Comment 23

[Dianna Smith [:diannaS]]

Comment on attachment 9323102 [details]
Bug 1814314 - use the script runner for calling ProcessCues.

Approved for 112.0b4

Comment 24

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ddd456444035