Assertion failure: !mForbiddenToFlush (This is bad!), at src/layout/base/PresShell.cpp:3966
Categories
(Core :: Layout, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr91 | --- | wontfix |
firefox-esr102 | --- | wontfix |
firefox70 | --- | unaffected |
firefox71 | - | wontfix |
firefox72 | - | wontfix |
firefox73 | - | wontfix |
firefox74 | - | wontfix |
firefox76 | --- | wontfix |
firefox77 | --- | wontfix |
firefox78 | --- | wontfix |
firefox99 | --- | wontfix |
firefox100 | --- | wontfix |
firefox101 | --- | wontfix |
firefox102 | --- | wontfix |
firefox103 | --- | wontfix |
firefox104 | --- | wontfix |
firefox105 | --- | wontfix |
firefox106 | --- | wontfix |
firefox107 | --- | fixed |
People
(Reporter: tsmith, Assigned: emilio)
References
(Blocks 1 open bug, Regression)
Details
(5 keywords, Whiteboard: [bugmon:bisected,confirmed][post-critsmash-triage][adv-main107+r])
Attachments
(1 file, 3 obsolete files)
543 bytes,
text/html
|
Details |
Found with m-c 20190927-dcfdecc355c0
Setting s-s flag based on the assertion message. Feel free to ping me if this is incorrect.
Assertion failure: !mForbiddenToFlush (This is bad!), at src/layout/base/PresShell.cpp:3966
#0 RecordMeasurementEnd src/obj-firefox/dist/include/mozilla/PerfStats.h:47:11
#1 ~AutoMetricRecording src/obj-firefox/dist/include/mozilla/PerfStats.h:57
#2 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4142
#3 FlushPendingNotifications src/obj-firefox/dist/include/mozilla/PresShell.h:1451:5
#4 mozilla::dom::Document::FlushPendingNotifications(mozilla::ChangesToFlush) src/dom/base/Document.cpp:10073
#5 nsEditingSession::SetupEditorOnWindow(nsPIDOMWindowOuter&) src/editor/composer/nsEditingSession.cpp:299:10
#6 nsEditingSession::MakeWindowEditable(mozIDOMWindowProxy*, char const*, bool, bool, bool) src/editor/composer/nsEditingSession.cpp:166:10
#7 mozilla::dom::Document::EditingStateChanged() src/dom/base/Document.cpp:5279:25
#8 mozilla::dom::Document::EndUpdate() src/dom/base/Document.cpp:7074:3
#9 ~mozAutoDocUpdate src/dom/base/mozAutoDocUpdate.h:34:18
#10 mozilla::dom::Element::SetAttr(int, nsAtom*, nsAtom*, nsTSubstring<char16_t> const&, nsIPrincipal*, bool) src/dom/base/Element.cpp:2386
#11 SetAttr src/obj-firefox/dist/include/mozilla/dom/Element.h:835:12
#12 SetAttr src/obj-firefox/dist/include/mozilla/dom/Element.h:831
#13 mozilla::ScrollFrameHelper::SetCoordAttribute(mozilla::dom::Element*, nsAtom*, int) src/layout/generic/nsGfxScrollFrame.cpp:6322
#14 mozilla::ScrollFrameHelper::UpdateScrollbarPosition() src/layout/generic/nsGfxScrollFrame.cpp:5060:5
#15 mozilla::ScrollFrameHelper::ScrollToImpl(nsPoint, nsRect const&, nsAtom*) src/layout/generic/nsGfxScrollFrame.cpp:3010:5
#16 mozilla::ScrollFrameHelper::CompleteAsyncScroll(nsRect const&, nsAtom*) src/layout/generic/nsGfxScrollFrame.cpp:2210:3
#17 mozilla::ScrollFrameHelper::ScrollToWithOrigin(nsPoint, mozilla::ScrollMode, nsAtom*, nsRect const*, nsIScrollbarMediator::ScrollSnapMode) src/layout/generic/nsGfxScrollFrame.cpp:2342:5
#18 mozilla::ScrollFrameHelper::ScrollTo(nsPoint, mozilla::ScrollMode, nsAtom*, nsRect const*, nsIScrollbarMediator::ScrollSnapMode) src/layout/generic/nsGfxScrollFrame.cpp:2245:3
#19 mozilla::layout::ScrollAnchorContainer::ApplyAdjustments() src/layout/generic/ScrollAnchorContainer.cpp:378:17
#20 FlushPendingScrollAnchorAdjustments src/layout/base/PresShell.cpp:2566:23
#21 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4162
#22 mozilla::PresShell::DidDoReflow(bool) src/layout/base/PresShell.cpp:9035:3
#23 mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9408:7
#24 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4159:11
#25 FlushPendingNotifications src/obj-firefox/dist/include/mozilla/PresShell.h:1451:5
#26 nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:2011
#27 TickDriver src/layout/base/nsRefreshDriver.cpp:373:13
#28 mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:350
#29 mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:367:5
#30 RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:807:5
#31 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:727
#32 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::VsyncEvent const&) src/layout/base/nsRefreshDriver.cpp:622:9
#33 mozilla::layout::VsyncChild::RecvNotify(mozilla::VsyncEvent const&) src/layout/ipc/VsyncChild.cpp:65:16
#34 mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:187:54
#35 mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:5876:32
#36 mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2185:25
#37 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2109:9
#38 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1954:3
#39 mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1985:13
#40 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1225:14
#41 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:486:10
#42 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:88:21
#43 RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
#44 RunHandler src/ipc/chromium/src/base/message_loop.cc:308
#45 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
#46 nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#47 XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:934:20
#48 RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
#49 RunHandler src/ipc/chromium/src/base/message_loop.cc:308
#50 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
#51 XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:769:34
#52 content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#53 main src/browser/app/nsBrowserApp.cpp:272
Assignee | ||
Comment 1•5 years ago
|
||
Fun! I guess this is caused by the assertions I added in bug 1553772.
So not great, though pre-existing, as this is me trying to harden our code against reentrant layouts like these, which are quite nasty.
Updated•5 years ago
|
Comment 2•5 years ago
|
||
"caused" by the assertions, or merely detected by them?
Assuming assertions that say "this is bad" mean it: sec-high
Reporter | ||
Comment 3•5 years ago
•
|
||
A Pernosco session is available here: https://pernos.co/debug/xZpaTpbBYNlOZVl8rCDF6w/index.html
Assignee | ||
Comment 4•5 years ago
|
||
(In reply to Daniel Veditz [:dveditz] from comment #2)
"caused" by the assertions, or merely detected by them?
Detected by, of course.
(In reply to Tyson Smith [:tsmith] from comment #3)
A Pernosco session is available here: https://pernos.co/debug/xZpaTpbBYNlOZVl8rCDF6w/index.html
Awesome! Tyson, could you attach the test-case you reproduced this with? I see that it's calling a few different functions, so the test-case is a bit different.
For example in the pernosco session the entry point is scrollContentIntoView() called from RAF callbacks, the attached test-case has no such call.
Reporter | ||
Comment 5•5 years ago
|
||
More reliable test case.
Reporter | ||
Comment 6•5 years ago
|
||
emilio: Let me know if testcase.html has what you expect and if not let me know if you'd like a new Pernosco session.
Comment 7•5 years ago
|
||
Emilio: Are you actively working on this? Can we assign to you?
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Comment 9•5 years ago
|
||
Unlikely to be fixed in 71 given where we are in the schedule, I am keeping it as fix-optional for 71 because it is as sec-high issue, in case we have a late dot release and a fix happens.
Updated•5 years ago
|
Comment 10•5 years ago
|
||
Any progress here?
Comment 11•5 years ago
|
||
Emilio's been away on PTO. Though from conversations with Emilio I think we can safely say this is not sec-high. The assertions were recently added, and the re-entrant layout was detected by the new assertions, not caused by them.
Comment 12•5 years ago
|
||
I don't think this needs to be tracked if it's a sec-moderate, but I'd still take a low-risk fix in Fx73 if one were available.
Reporter | ||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•3 years ago
|
Reporter | ||
Comment 13•3 years ago
|
||
A new Pernosco session is available here: https://pernos.co/debug/LN7etsHmxFDZxY0OWQvvFQ/index.html
Reporter | ||
Comment 14•3 years ago
|
||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 15•3 years ago
|
||
Okay, so I took a look at this. The issue is that editor fails to initialize because this fails, and we semi-randomly try to initialize on the next Document::EndUpdate
, which is a bad time.
Masayuki, when the editor fails to initialize, can we either avoid trying to initialize again, or make the retry more deterministic? It seems bad to potentially be in a state where every document update tries to initialize the editor...
For reference, editor failing to initialize here.
Comment 16•3 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)
Masayuki, when the editor fails to initialize, can we either avoid trying to initialize again, or make the retry more deterministic? It seems bad to potentially be in a state where every document update tries to initialize the editor...
In general speaking, it's hard to say. I have no idea of general solution. However, in this case, it seems that HTMLEditor
should ignore the selection initialization failure because it can be valid, and HTMLEditor
should not assume that Selection::RangeCount
returns non-zero in the initialization path. But I've not checked enough yet. I'll be back later.
Updated•3 years ago
|
Comment 18•3 years ago
|
||
Just FYI, there's an additional testcase in my duplicate bug 1774383. (See attachment 9281391 [details])
AFAIU it arises from the same root issue (i.e. editor initialization), but gets there via an XML document.
Updated•3 years ago
|
Comment 19•2 years ago
|
||
Redirect a needinfo that is pending on an inactive user to the triage owner.
:dholbert, since the bug has high priority, high severity and recent activity, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 20•2 years ago
|
||
It seems there was a bug with the bot.
Comment 21•2 years ago
|
||
(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away -8/31) from comment #16)
But I've not checked enough yet. I'll be back later.
Hi Masayuki - just checking in to be sure this is still on your radar. Thanks!
Updated•2 years ago
|
Updated•2 years ago
|
Comment 22•2 years ago
|
||
EditorBase::CollapseSelectionToEndOf
is called only in the following stack for initializing the HTMLEditor
:
EditorBase::InitEditorContentAndSelection
HTMLEditor::InitEditorContentAndSelection
HTMLEditor::Init
There is bug 195802... So it might have already been caused infinite retrying, though. Perhaps, the ideal fix is, HTMLEditor::Init
should return special error code to make the caller retry it later. However, fixing this bug for now, I think that we should make EditorBase::InitEditorContentAndSelection
be careful more before calling CollapseSelectionToEndOf
.
Should I fix the editor part in a public bug?
Updated•2 years ago
|
Comment 24•2 years ago
|
||
Filed bug 1789967 for fixing the editor's initialization bug (but not linking it with this bug due to security reason).
Comment 25•2 years ago
|
||
Looks like we have patches on bug 1789967 now, which is great! Just checking, have you confirmed that they seem to fix this issue? (if you can repro with one of the testcases)
Comment 26•2 years ago
|
||
(In reply to Daniel Holbert [:dholbert][mostly away until Oct 6] from comment #25)
Looks like we have patches on bug 1789967 now, which is great! Just checking, have you confirmed that they seem to fix this issue? (if you can repro with one of the testcases)
I ran the testcases for a while in debug build, but I don't see any crashes/assertion hits.
Comment 27•2 years ago
|
||
I am still able to reproduce this issue on mozilla-central 20220920-45d33d6757ba.
Comment 28•2 years ago
|
||
FYI: bug 1789967 was fixed in mozilla-central.
Comment 29•2 years ago
|
||
(In reply to Jason Kratzer [:jkratzer] from comment #27)
I am still able to reproduce this issue on mozilla-central 20220920-45d33d6757ba.
Could you post a pernosco trace? Sounds like it might be fiddly / difficult to repro+investigate without a trace, per comment 26.
Comment 30•2 years ago
|
||
Bugmon Analysis
Testcase crashes using the initial build (mozilla-central 20220701213554-5140fba12e4a) but not with tip (mozilla-central 20221011093208-5cbd3d92a78c.)
The bug appears to have been fixed in the following build range:
Start: 4d1e93c629daa361f9acd60c1f4e2c594f3bd312 (20220922062700)
End: 4bfcb4ab080ffa3b69f831c840d1212e94fc7199 (20220922063009)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4d1e93c629daa361f9acd60c1f4e2c594f3bd312&tochange=4bfcb4ab080ffa3b69f831c840d1212e94fc7199
emilio, can you confirm that the above bisection range is responsible for fixing this issue?
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.
Assignee | ||
Comment 31•2 years ago
|
||
Yes, that's likely given the above discussion.
Updated•2 years ago
|
Comment 32•2 years ago
|
||
Well, but bug 1789967 did not fix the root cause (infinite retries of initializing HTMLEditor
), so I guess that there is another way to reproduce this.
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Description
•