Closed Bug 1472020 Opened 2 years ago Closed Last year

Assertion failure: !mMutationGuard.Mutated(0), at src/dom/base/ChildIterator.h:220

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- fixed

People

(Reporter: tsmith, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

Attached file testcase.html
Reduced with m-c:
BuildID=20180628100518
SourceStamp=b429b9fb68f1a954c4a9f8dba8e845cf7f569a56

Assertion failure: !mMutationGuard.Mutated(0), at src/dom/base/ChildIterator.h:220

#0 mozilla::dom::AllChildrenIterator::~AllChildrenIterator() src/dom/base/ChildIterator.h:220:28
#1 nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, nsCSSFrameConstructor::RemoveFlags) src/layout/base/nsCSSFrameConstructor.cpp:7990:3
#2 nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind) src/layout/base/nsCSSFrameConstructor.cpp:9169:5
#3 mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) src/layout/base/RestyleManager.cpp:1512:25
#4 mozilla::RestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) src/layout/base/RestyleManager.cpp:2997:9
#5 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4287:41
#6 nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1897:18
#7 mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:301:7
#8 mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:319:5
#9 mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:760:5
#10 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:673:35
#11 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() src/layout/base/nsRefreshDriver.cpp:519:20
#12 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1051:14
#13 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
#14 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
#15 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:325:10
#16 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298:3
#17 nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:158:27
#18 nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:290:30
#19 XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4746:22
#20 XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4891:8
#21 XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4983:21
#22 do_main(int, char**, char**) src/browser/app/nsBrowserApp.cpp:233:22
#23 main src/browser/app/nsBrowserApp.cpp:311:16
#24 __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
#25 _start (firefox+0x4236e4)
Flags: in-testsuite?
Attached file prefs.js
required to repro
Flags: needinfo?(emilio)
Heh, I knew fuzzing accessiblecaret was going to yield some interesting stuff. Thanks!
This one is fun, I'm glad I asked the Fuzzing Team to fuzz with layout.accessiblecaret.enabled... :)

So this is basically complaining because we're doing a bunch of DOM mutations from the nsCanvasFrame code, cloning nodes and what not:

  https://searchfox.org/mozilla-central/rev/d2966246905102b36ef5221b0e3cbccf7ea15a86/layout/generic/nsCanvasFrame.cpp#160

We also do a bunch of notifying insertions, etc. while creating the canvas frame from:

  https://searchfox.org/mozilla-central/rev/d2966246905102b36ef5221b0e3cbccf7ea15a86/layout/generic/nsCanvasFrame.cpp#127

Init() creates an AccessibleCaret, which calls InsertAnonymousContent:

  https://searchfox.org/mozilla-central/rev/d2966246905102b36ef5221b0e3cbccf7ea15a86/layout/base/AccessibleCaret.cpp#220

I'm not sure how we can organize this code to avoid this but the current setup is not quite ideal... :(
Can we move that work to a scriptrunner?  The destroy work presumably not, but the InsertAnonymousContent bits.
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #4)
> Can we move that work to a scriptrunner?  The destroy work presumably not,
> but the InsertAnonymousContent bits.

Sure, we could also not notify and let nsCanvasFrame do the right thing in the loop we do before calling Init().

I don't really understand:

  https://searchfox.org/mozilla-central/rev/d2966246905102b36ef5221b0e3cbccf7ea15a86/layout/generic/nsCanvasFrame.cpp#160

Though... We're just cloning subtrees just because, and replacing the old pointer with a clone. I don't understand why that's needed. DestroyAnonymousContent will unbind all those, and CreateAnonymousContent will bind them again to the new tree...
> and replacing the old pointer with a clone

Are we?  We stash the CloneNode result inside doc->GetAnonymousContents(), no?  Then DestroyAnonymousContent unbinds and destroys the originals and CreateAnonymousContent grabs the things from doc->GetAnonymousContents().

Though that stuff looks like it only supports one canvasframe per document; not sure whether that makes sense...
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #6)
> > and replacing the old pointer with a clone
> 
> Are we?  We stash the CloneNode result inside doc->GetAnonymousContents(),
> no?  Then DestroyAnonymousContent unbinds and destroys the originals and
> CreateAnonymousContent grabs the things from doc->GetAnonymousContents().

Right, the point is that DestroyAnonymousContent only unbinds the subtree, which makes sense, but there's no reason we couldn't reuse the same nodes, I'd think...

> Though that stuff looks like it only supports one canvasframe per document;
> not sure whether that makes sense...

Right...
Assignee: nobody → emilio
Flags: needinfo?(emilio)
* Avoid processing anon content in nsCanvasFrame, then getting more anon
   content via AccessibleCaretEventHub::Init. Instead call Init before creating
   the custom content container. We could also throw a script runner at it I
   guess, but this prevents the reentrancy issue.

 * Avoid cloning nodes on shutdown, just use the same node (already cloned in
   InsertAnonymousContent) node instead.

   The RemoveChild in GetAnonymousContent to handle the reframes instead of
   cloning around is a bit hacky, but I don't think it's really worth extending
   PostDestroyData for this special case.

 * Reuse the ContentInfo stuff, instead of notifying while appending and that
   kind of stuff.
Flags: needinfo?(emilio)
Priority: -- → P3
Comment on attachment 8988850 [details]
Bug 1472020: Make AccessibleCaret a bit saner. r=bz

Ting-Yu Lin [:TYLin] (UTC-7) has approved the revision.

https://phabricator.services.mozilla.com/D1889
Attachment #8988850 - Flags: review+
Flags: needinfo?(emilio)
Comment on attachment 8988850 [details]
Bug 1472020: Make AccessibleCaret a bit saner. r=bz

Boris Zbarsky [:bzbarsky, bz on IRC] (vacation Aug 16-27) has approved the revision.
Attachment #8988850 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/5c9a4d66f5f3
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.