See bug 1381157 comment 17.

For what it's worth, I think this should be higher priority than P4. We're violating an invariant we depend on in various places in the codebase; it's just luck that it's not biting us badly.

I'll take this one I think. I worked on bug 1401706, which killed the last remaining anon content bit that wasn't owned by a frame, so should be easy enough now... https://treeherder.mozilla.org/#/jobs?repo=try&revision=75ec5c306361e50e7813c12196284de1db2494dc

Note: we might want to try to remove the nsIFrame::GenConProperty while we're at it: http://searchfox.org/mozilla-central/rev/31606bbabc50b08895d843b9f5f3da938ccdfbbf/layout/base/nsCSSFrameConstructor.cpp#6298-6299

I poked around a bit in this code and found a few existing bugs etc, so I have a patch set that I think might work... Emilio, perhaps I should take this bug now, unless you have something that's ready?

Please be my guest :) Basically, the patch over my try run in comment 2 works, but makes the assertion added in that same patch (that afaict should hold) not hold in some situations. I was meaning to investigate them sometime soon, but if you have something working please go ahead!

So, the last remaining issue seems to be this assertion in nsIContent::SetPrimaryFrame: http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/dom/base/nsIContentInlines.h#32 which fails for non-native anonymous content: https://treeherder.mozilla.org/#/jobs?repo=try&revision=76b8cc663542637715fab9b5e4d5b2288564a877&selectedJob=139541268 The failed content is the error message we display for unstyled XML documents, e.g. data:text/xml,blah It looks like we have a remaining UnbindFromTree before destroying frames like so: #0 nsINode::SetSubtreeRootPointer(nsINode*) (this=0x7f390768ee00, aSubtreeRoot=0x7f390769f3a0) #1 nsGenericDOMDataNode::UnbindFromTree(bool, bool) (this=0x7f390768ee00, aDeep=true, aNullParent=false) #2 nsTextNode::UnbindFromTree(bool, bool) (this=0x7f390768ee00, aDeep=true, aNullParent=false) #3 mozilla::dom::Element::UnbindFromTree(bool, bool) (this=0x7f390768fa10, aDeep=true, aNullParent=false) #4 nsGenericHTMLElement::UnbindFromTree(bool, bool) (this=0x7f390768fa10, aDeep=true, aNullParent=false) #5 mozilla::dom::Element::UnbindFromTree(bool, bool) (this=0x7f390768f980, aDeep=true, aNullParent=false) #6 nsGenericHTMLElement::UnbindFromTree(bool, bool) (this=0x7f390768f980, aDeep=true, aNullParent=false) #7 mozilla::dom::Element::UnbindFromTree(bool, bool) (this=0x7f390769f3a0, aDeep=true, aNullParent=true) #8 nsGenericHTMLElement::UnbindFromTree(bool, bool) (this=0x7f390769f3a0, aDeep=true, aNullParent=true) #9 nsXBLBinding::UnbindAnonymousContent(nsIDocument*, nsIContent*, bool) #10 nsXBLBinding::ChangeDocument(nsIDocument*, nsIDocument*) #11 nsBindingManager::ClearBinding(mozilla::dom::Element*) #12 nsXMLPrettyPrinter::Unhook() (this=0x7f390a5c1b50) called from a script runner... Do you have a suggestion on how we should handle this case? I'd prefer if we could destroy the frame tree at some point before the above unbind call somehow, to maintain the invariant that we always destroy them first, with no exceptions. (FYI, the frames are later destroyed in nsDocumentViewer::Show, as part of a "prevViewer->Destroy()")

FTR, our imagemap <area> hack comes back to bite us once again when doing these changes, because the SetPrimaryFrame(nullptr) here: http://searchfox.org/mozilla-central/rev/dd47bee6468de7e1221b4d006342ad6b9813d0e5/layout/generic/nsImageMap.cpp#721 removes the bit on the frame (for the <img>) that says it's primary... I worked around it by not letting the <area>s affect that bit in this case. We really should fix that hack someday though...

This appears to fix the issue in comment 6: https://hg.mozilla.org/try/rev/0afd81e4c511a985f8e4eaead8012b58d22a2617 There's more though: https://treeherder.mozilla.org/#/jobs?repo=try&revision=58d4b4005aa3af7d85c6a81a7830d510151cd5c9&selectedJob=139704412

Right, so tearing down the frames in nsBindingManager::ClearBinding before we ChangeDocument to null makes sense. I wonder whether we want something similar in nsXBLService::FlushStyleBindings. I wish I knew what's going on in that Marionette case...

I think I found the AccessibleCaret issue (still waiting for Try results): https://hg.mozilla.org/try/rev/57b588663c69383773c159c344a858e960c57fcb (it inserts some caret anon content and removes it w/o destroying frames first) I'll take a look at FlushStyleBindings, thanks.

The only two calls to FlushStyleBindings are from nsXBLService::LoadBindings: http://searchfox.org/mozilla-central/search?q=FlushStyleBindings LoadBindings is called from a few places: http://searchfox.org/mozilla-central/search?q=symbol:_ZN12nsXBLService12LoadBindingsEP10nsIContentP6nsIURIP12nsIPrincipalPP12nsXBLBindingPb&redirect=false The latter three call sites, nsXMLPrettyPrinter.cpp and two in nsCSSFrameConstructor.cpp always happens before frames are created for the element AFAICT. The call in Element::WrapObject only occurs if |bindingURL| is non-null: http://searchfox.org/mozilla-central/rev/40e8eb46609dcb8780764774ec550afff1eed3a5/dom/base/Element.cpp#589,595 which it isn't if the element already has a frame: http://searchfox.org/mozilla-central/rev/40e8eb46609dcb8780764774ec550afff1eed3a5/dom/base/Element.cpp#498 So, I think I'll leave FlushStyleBindings as is for now. I think we have enough assertions to catch any future problems. Also, nsXBLService is soon going away I hope ;-)

The callsite in nsXBLPrettyPrinter can, afaict, run after frame construction. It runs when the parser finishes parsing the document, which can be after we've already notified on the root and started constructing frames for it.

(this patch won't compile standalone, but I'll fold the next three parts before landing)

Just adding aPostDestroyData in more places, nothing of interest to see here.

s/DestroyAnonymousContent/aPostDestroyData.AddAnonymousContent/

The test is testing mutation observers on ::before/::after. I think this is a feature for internal use only (devtools) and not exposed to the web. Apparently we now notify "remove" on the ::after node before the ::before. I don't think this matters.

Bugfix for: http://searchfox.org/mozilla-central/rev/21363323fd4aa21db074c808fb5358a46df6d698/layout/generic/nsImageMap.cpp#721,848 We don't want these to mess with the frame state bit.

I'm sorry for the horrible lag here. I'm aiming to get to this tomorrow.

Comment on attachment 8923099 [details] [diff] [review] part 1 - Collect NAC / generated content and call DestroyAnonymousContent / UnbindFromTree on those after the frames are destroyed >+ mAnonymousContent.AppendElement(aContent); Move(aContent)? >+ mGeneratedContent.AppendElement(aContent); And here? >+ for (size_t i = data.mAnonymousContent.Length(); i-- > 0;) { >+ auto& content = data.mAnonymousContent[i]; You're not removing things from the array, right? Why not just: for (auto& content : data.mAnonymousContent) { and similar for the loop over mGeneratedContent? r=me. I assume you will be folding this with some of the later changesets when you push it, since it doesn't build on its own.

Comment on attachment 8923100 [details] [diff] [review] part 2 - Add aPostDestroyData param to DestroyFrom methods (scripted change) It would be nice to include the actual script used in the commit message, for future reference...

Comment on attachment 8923101 [details] [diff] [review] part 3 - Add aPostDestroyData param to DestroyFrom methods (manual changes) r=me

Comment on attachment 8923102 [details] [diff] [review] part 4 - Convert DestroyAnonymousContent calls to aPostDestroyData.AddAnonymousContent calls (scripted change) r=me

Comment on attachment 8923103 [details] [diff] [review] part 5 - Remove the nsIFrame::GenConProperty property and make aPostDestroyData deal with unbinding it >@@ -6543,23 +6527,21 @@ AdjustAppendParentForAfterContent(nsFram The comments here no longer match the code (e.g. we're not checking for "any pseudo-elements"). >+ aFrameManager->GetAllRegisteredDisplayContentsStylesIn(aContainer)) { Why is this new check needed? Please document. r=me modulo that.

Comment on attachment 8923105 [details] [diff] [review] part 7 - Destroy frames before calling nsXBLBinding::ChangeDocument in a few places r=me

Comment on attachment 8923106 [details] [diff] [review] part 8 - Destroy frames before removing the anon content (to ensure it happens before the UnbindFromTree) The commit message should say something about how this is specific to AccessibleCaret.

Comment on attachment 8923107 [details] [diff] [review] part 9 - Remove the workaround for the broken IsPrimaryFrame() in nsFrame::DestroyFrom since it now works also for NAC frames r=me. Thank you for cleaning all this up!

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #24) > Comment on attachment 8923099 [details] [diff] [review] > part 1 - Collect NAC / generated content and call DestroyAnonymousContent / > UnbindFromTree on those after the frames are destroyed > > >+ mAnonymousContent.AppendElement(aContent); > > Move(aContent)? aContent is already T&& so Move() would do literally nothing here. Besides, even if it were T or T&, already_AddRefed<> have move-semantics so Move wouldn't really matter in those cases either since constructor (and assignment) runs identical code to T&&: http://searchfox.org/mozilla-central/rev/af86a58b157fbed26b0e86fcd81f1b421e80e60a/mfbt/RefPtr.h#122-133,198-213 > >+ for (size_t i = data.mAnonymousContent.Length(); i-- > 0;) { > >+ auto& content = data.mAnonymousContent[i]; > > You're not removing things from the array, right? Correct. > for (auto& content : data.mAnonymousContent) { I figured it be saner to destroy them bottom-up, which is why I'm iterating in reverse. But yeah, I'll use mozilla::Reversed instead of manual indexing.

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #28) > >@@ -6543,23 +6527,21 @@ AdjustAppendParentForAfterContent(nsFram > > The comments here no longer match the code (e.g. we're not checking for "any > pseudo-elements"). > > >+ aFrameManager->GetAllRegisteredDisplayContentsStylesIn(aContainer)) { > > Why is this new check needed? Please document. Right, the code comment is already slightly off actually since GetProperty(nsIFrame::GenConProperty()) is not /only/ for "If the parent frame has any pseudo-elements" but also for aContainer with a display:contents child that has bubbled up a ::before/::after frame (on its own or from descendants). The new GetAllRegisteredDisplayContentsStylesIn check is slightly less precise since it only checks for the existence of a display:contents child but not if there is a ::before/::after frame from it. I don't think it should matter much for performance though since display:contents is rare and the extra work we do here isn't that slow anyway. I'll update the code comment to reflect the code...

The code I was referring to in the last comment: http://searchfox.org/mozilla-central/rev/af86a58b157fbed26b0e86fcd81f1b421e80e60a/layout/base/nsCSSFrameConstructor.cpp#6546-6557

> since display:contents is rare It might or might not be in a lots-of-shadow-DOM world, where all the slots are display:contents, right?

True, but I'd still expect insertions into shadow roots to be rare in that future world. It doesn't seem worth micro-optimizing it at this time.

Pushed by mpalmgren@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a6461f80307c part 1 - Collect NAC / generated content and call DestroyAnonymousContent / UnbindFromTree on those after the frames are destroyed. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/9d35e52771e0 part 2 - Remove the nsIFrame::GenConProperty property and make aPostDestroyData deal with unbinding it. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/82f3de55cfdf part 3 - Don't let <area>s mess up the frame's IsPrimaryFrame bit, since the frame is actually owned by the <img>. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/fc5b7c39dd16 part 4 - Destroy frames before calling nsXBLBinding::ChangeDocument in a few places. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/14c27d1d3d54 part 5 - Destroy the frames before removing the AccessibleCaret anonymous content (to ensure it happens before the UnbindFromTree). r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/98a34f658119 part 6 - Remove the workaround for the broken IsPrimaryFrame() in nsFrame::DestroyFrom since it now works also for NAC frames. r=bz

Backed out for asserting in clipboard's dom/browser-element/mochitest/test_browserElement_inproc_CopyPaste.html on Windows 7 debug without e10s: https://hg.mozilla.org/integration/mozilla-inbound/rev/ebc611dc8c85055ac62bc945e9871cdc317cf6f9 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=98a34f65811942ddd47f28ea047d8542c35eea16&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=142221033&repo=mozilla-inbound 01:32:44 INFO - TEST-START | dom/browser-element/mochitest/test_browserElement_inproc_CopyPaste.html 01:32:44 INFO - GECKO(3764) | ++DOMWINDOW == 19 (22E7B000) [pid = 3764] [serial = 19] [outer = 22B7E800] 01:32:44 INFO - GECKO(3764) | [3764, Main Thread] WARNING: NS_ENSURE_TRUE(aSecondURI) failed: file z:/build/build/src/dom/base/ThirdPartyUtil.cpp, line 98 01:32:45 INFO - GECKO(3764) | ++DOMWINDOW == 20 (2384DC00) [pid = 3764] [serial = 20] [outer = 22B7E800] 01:32:46 INFO - GECKO(3764) | ++DOCSHELL 23856400 == 8 [pid = 3764] [id = {8a68a3cf-d074-4270-b766-9e4d069e8465}] 01:32:46 INFO - GECKO(3764) | ++DOMWINDOW == 21 (23856C00) [pid = 3764] [serial = 21] [outer = 00000000] 01:32:46 INFO - GECKO(3764) | ++DOMWINDOW == 22 (23E4DC00) [pid = 3764] [serial = 22] [outer = 23856C00] 01:32:46 INFO - GECKO(3764) | ++DOMWINDOW == 23 (23E51800) [pid = 3764] [serial = 23] [outer = 23856C00] 01:32:46 INFO - GECKO(3764) | ++DOMWINDOW == 24 (23E55C00) [pid = 3764] [serial = 24] [outer = 23856C00] 01:32:46 INFO - GECKO(3764) | [3764, Main Thread] WARNING: NS_ENSURE_TRUE(content && aRange) failed: file z:/build/build/src/editor/txtsvc/nsFilteredContentIterator.cpp, line 290 01:32:46 INFO - GECKO(3764) | [3764, Main Thread] WARNING: '!NodeIsInTraversalRange(mLast, mPre, RawRangeBoundary(mFirst, 0), aEnd)', file z:/build/build/src/dom/base/nsContentIterator.cpp, line 473 01:32:46 INFO - GECKO(3764) | ++DOMWINDOW == 25 (121F6C00) [pid = 3764] [serial = 25] [outer = 23856C00] 01:32:46 INFO - GECKO(3764) | JavaScript error: chrome://formautofill/content/FormAutofillFrameScript.js, line 35: ReferenceError: setTimeout is not defined 01:32:47 INFO - GECKO(3764) | ++DOMWINDOW == 26 (2074C800) [pid = 3764] [serial = 26] [outer = 23856C00] 01:32:47 INFO - GECKO(3764) | ++DOMWINDOW == 27 (2384E400) [pid = 3764] [serial = 27] [outer = 23856C00] 01:32:47 INFO - GECKO(3764) | ++DOMWINDOW == 28 (24012800) [pid = 3764] [serial = 28] [outer = 23856C00] 01:32:47 INFO - GECKO(3764) | JavaScript error: data:,docShell.doCommand("cmd_cut");, line 1: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDocShell.doCommand] 01:32:47 INFO - GECKO(3764) | ++DOMWINDOW == 29 (22E5A800) [pid = 3764] [serial = 29] [outer = 23856C00] 01:32:47 INFO - GECKO(3764) | [3764, Main Thread] WARNING: NS_ENSURE_TRUE(content && aRange) failed: file z:/build/build/src/editor/txtsvc/nsFilteredContentIterator.cpp, line 290 01:32:47 INFO - GECKO(3764) | [3764, Main Thread] WARNING: NS_ENSURE_TRUE(aNode) failed: file z:/build/build/src/editor/libeditor/EditorBase.cpp, line 3636 01:32:47 INFO - GECKO(3764) | [3764, Main Thread] ###!!! ASSERTION: Should be in an update while destroying frames: 'mUpdateCount != 0', file z:/build/build/src/layout/base/nsCSSFrameConstructor.cpp, line 1681 01:32:48 INFO - GECKO(3764) | #01: nsCSSFrameConstructor::NotifyDestroyingFrame(nsIFrame *) [layout/base/nsCSSFrameConstructor.cpp:1680] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #02: mozilla::PresShell::NotifyDestroyingFrame(nsIFrame *) [layout/base/PresShell.cpp:2115] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #03: nsFrame::DestroyFrom(nsIFrame *,mozilla::layout::PostFrameDestroyData &) [layout/generic/nsFrame.cpp:810] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #04: nsSplittableFrame::DestroyFrom(nsIFrame *,mozilla::layout::PostFrameDestroyData &) [layout/generic/nsSplittableFrame.cpp:41] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #05: nsContainerFrame::DestroyFrom(nsIFrame *,mozilla::layout::PostFrameDestroyData &) [layout/generic/nsContainerFrame.cpp:300] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #06: nsBlockFrame::DestroyFrom(nsIFrame *,mozilla::layout::PostFrameDestroyData &) [layout/generic/nsBlockFrame.cpp:361] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #07: nsFrameList::DestroyFramesFrom(nsIFrame *,mozilla::layout::PostFrameDestroyData &) [layout/generic/nsFrameList.cpp:58] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #08: nsContainerFrame::DestroyAbsoluteFrames(nsIFrame *,mozilla::layout::PostFrameDestroyData &) [layout/generic/nsContainerFrame.cpp:190] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #09: nsBlockFrame::DestroyFrom(nsIFrame *,mozilla::layout::PostFrameDestroyData &) [layout/generic/nsBlockFrame.cpp:328] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #10: nsIFrame::Destroy() [layout/generic/nsIFrame.h:676] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #11: nsFrameList::DestroyFrame(nsIFrame *) [layout/generic/nsFrameList.cpp:140] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #12: nsAbsoluteContainingBlock::RemoveFrame(nsIFrame *,mozilla::layout::FrameChildListID,nsIFrame *) [layout/generic/nsAbsoluteContainingBlock.cpp:111] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #13: nsFrameManager::RemoveFrame(mozilla::layout::FrameChildListID,nsIFrame *) [layout/base/nsFrameManager.cpp:534] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #14: nsPlaceholderFrame::DestroyFrom(nsIFrame *,mozilla::layout::PostFrameDestroyData &) [layout/generic/nsPlaceholderFrame.cpp:194] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #15: nsIFrame::Destroy() [layout/generic/nsIFrame.h:676] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #16: nsBlockFrame::DoRemoveFrame(nsIFrame *,unsigned int) [layout/generic/nsBlockFrame.cpp:5980] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #17: nsBlockFrame::RemoveFrame(mozilla::layout::FrameChildListID,nsIFrame *) [layout/generic/nsBlockFrame.cpp:5293] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #18: mozilla::AccessibleCaret::RemoveCaretElement(nsIDocument *) [layout/base/AccessibleCaret.cpp:273] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #19: mozilla::AccessibleCaret::~AccessibleCaret() [layout/base/AccessibleCaret.cpp:101] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #20: mozilla::AccessibleCaretManager::Terminate() [layout/base/AccessibleCaretManager.cpp:133] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #21: mozilla::PresShell::Destroy() [layout/base/PresShell.cpp:1254] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #22: nsDocumentViewer::DestroyPresShell() [layout/base/nsDocumentViewer.cpp:4645] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #23: nsDocumentViewer::Destroy() [layout/base/nsDocumentViewer.cpp:1754] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #24: nsSHistory::EvictContentViewerForTransaction(nsISHTransaction *) [docshell/shistory/nsSHistory.cpp:226] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #25: nsSHistory::EvictAllContentViewers() [docshell/shistory/nsSHistory.cpp:927] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #26: nsDocShell::Destroy() [docshell/base/nsDocShell.cpp:5989] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #27: nsFrameLoader::DestroyDocShell() [dom/base/nsFrameLoader.cpp:2313] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #28: nsFrameLoaderDestroyRunnable::Run() [dom/base/nsFrameLoader.cpp:2256] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #29: nsDocument::MaybeInitializeFinalizeFrameLoaders() [dom/base/nsDocument.cpp:7610] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #30: nsDocument::EndUpdate(unsigned int) [dom/base/nsDocument.cpp:5427] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #31: nsHTMLDocument::EndUpdate(unsigned int) [dom/html/nsHTMLDocument.cpp:2523] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #32: mozAutoDocUpdate::~mozAutoDocUpdate() [dom/base/mozAutoDocUpdate.h:42] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #33: mozilla::dom::FragmentOrElement::RemoveChildAt(unsigned int,bool) [dom/base/FragmentOrElement.cpp:1338] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #34: nsINode::RemoveChild(nsINode &,mozilla::ErrorResult &) [dom/base/nsINode.cpp:620] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #35: mozilla::dom::NodeBinding::removeChild [s3:gecko-generated-sources:2005eb22e124a61f594e91d13ba04b77ed8dd260814cc41e77a9b9cc89a3e36c14e5d396dd5bf96fb186ede0274839604ff5fa83efdb97bb26ad5be001abf343/dom/bindings/NodeBinding.cpp::1013] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #36: mozilla::dom::GenericBindingMethod(JSContext *,unsigned int,JS::Value *) [dom/bindings/BindingUtils.cpp:3040] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #37: js::CallJSNative(JSContext *,bool (*)(JSContext *,unsigned int,JS::Value *),JS::CallArgs const &) [js/src/jscntxtinlines.h:291] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #38: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:472] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #39: InternalCall [js/src/vm/Interpreter.cpp:521] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #40: Interpret [js/src/vm/Interpreter.cpp:3061] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #41: js::RunScript(JSContext *,js::RunState &) [js/src/vm/Interpreter.cpp:422] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #42: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:494] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #43: InternalCall [js/src/vm/Interpreter.cpp:521] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #44: js::Call(JSContext *,JS::Handle<JS::Value>,JS::Handle<JS::Value>,js::AnyInvokeArgs const &,JS::MutableHandle<JS::Value>) [js/src/vm/Interpreter.cpp:540] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #45: js::ForwardingProxyHandler::call(JSContext *,JS::Handle<JSObject *>,JS::CallArgs const &) [js/src/proxy/Wrapper.cpp:176] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #46: js::CrossCompartmentWrapper::call(JSContext *,JS::Handle<JSObject *>,JS::CallArgs const &) [js/src/proxy/CrossCompartmentWrapper.cpp:358] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #47: xpc::JSXrayTraits::call(JSContext *,JS::Handle<JSObject *>,JS::CallArgs const &,js::Wrapper const &) [js/xpconnect/wrappers/XrayWrapper.h:282] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #48: xpc::XrayWrapper<js::CrossCompartmentWrapper,xpc::JSXrayTraits>::call(JSContext *,JS::Handle<JSObject *>,JS::CallArgs const &) [js/xpconnect/wrappers/XrayWrapper.cpp:2453] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #49: js::proxy_Call(JSContext *,unsigned int,JS::Value *) [js/src/proxy/Proxy.cpp:770] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #50: js::CallJSNative(JSContext *,bool (*)(JSContext *,unsigned int,JS::Value *),JS::CallArgs const &) [js/src/jscntxtinlines.h:291] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #51: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:454] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #52: InternalCall [js/src/vm/Interpreter.cpp:521] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #53: js::Call(JSContext *,JS::Handle<JS::Value>,JS::Handle<JS::Value>,js::AnyInvokeArgs const &,JS::MutableHandle<JS::Value>) [js/src/vm/Interpreter.cpp:540] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #54: JS_CallFunctionValue(JSContext *,JS::Handle<JSObject *>,JS::Handle<JS::Value>,JS::HandleValueArray const &,JS::MutableHandle<JS::Value>) [js/src/jsapi.cpp:2974] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #55: nsFrameMessageManager::ReceiveMessage(nsISupports *,nsIFrameLoader *,bool,nsTSubstring<char16_t> const &,bool,mozilla::dom::ipc::StructuredCloneData *,mozilla::jsipc::CpowHolder *,nsIPrincipal *,nsTArray<mozilla::dom::ipc::StructuredCloneData> *) [dom/base/nsFrameMessageManager.cpp:1097] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #56: nsSameProcessAsyncMessageBase::ReceiveMessage(nsISupports *,nsIFrameLoader *,nsFrameMessageManager *) [dom/base/nsFrameMessageManager.cpp:2093] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #57: nsAsyncMessageToParent::HandleMessage() [dom/base/nsInProcessTabChildGlobal.cpp:63] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #58: mozilla::dom::SameProcessMessageQueue::Runnable::Run() [dom/base/SameProcessMessageQueue.cpp:74] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #59: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:1038] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #60: NS_ProcessNextEvent(nsIThread *,bool) [xpcom/threads/nsThreadUtils.cpp:513] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #61: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:97] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #62: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:326] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #63: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:320] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #64: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:300] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #65: nsBaseAppShell::Run() [widget/nsBaseAppShell.cpp:160] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #66: nsAppShell::Run() [widget/windows/nsAppShell.cpp:230] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #67: nsAppStartup::Run() [toolkit/components/startup/nsAppStartup.cpp:289] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #68: XREMain::XRE_mainRun() [toolkit/xre/nsAppRunner.cpp:4675] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #69: XREMain::XRE_main(int,char * * const,mozilla::BootstrapConfig const &) [toolkit/xre/nsAppRunner.cpp:4837] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #70: XRE_main(int,char * * const,mozilla::BootstrapConfig const &) [toolkit/xre/nsAppRunner.cpp:4932] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #71: mozilla::BootstrapImpl::XRE_main(int,char * * const,mozilla::BootstrapConfig const &) [toolkit/xre/Bootstrap.cpp:49] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #72: do_main [browser/app/nsBrowserApp.cpp:232] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #73: NS_internal_main(int,char * *,char * *) [browser/app/nsBrowserApp.cpp:304] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #74: wmain [toolkit/xre/nsWindowsWMain.cpp:114] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #75: __scrt_common_main_seh [f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:283] 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #76: kernel32.dll + 0x53c45 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #77: ntdll.dll + 0x637f5 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | #78: ntdll.dll + 0x637c8 01:32:48 INFO - 01:32:48 INFO - GECKO(3764) | [3764, Main Thread] ###!!! ASSERTION: Should be in an update while destroying frames: <more assertions> 01:32:48 INFO - GECKO(3764) | ++DOCSHELL 119EDC00 == 9 [pid = 3764] [id = {acceaf4e-01b3-428e-a34d-0819af26399d}] 01:32:48 INFO - GECKO(3764) | ++DOMWINDOW == 30 (2263CC00) [pid = 3764] [serial = 30] [outer = 00000000] 01:32:48 INFO - GECKO(3764) | ++DOMWINDOW == 31 (22E69C00) [pid = 3764] [serial = 31] [outer = 2263CC00] 01:32:48 INFO - GECKO(3764) | ++DOMWINDOW == 32 (123CD400) [pid = 3764] [serial = 32] [outer = 2263CC00] 01:32:48 INFO - GECKO(3764) | ++DOCSHELL 13683000 == 10 [pid = 3764] [id = {acb49cf5-98f2-4eae-9bcb-b2bfd3a8a70f}] 01:32:48 INFO - GECKO(3764) | ++DOMWINDOW == 33 (1404C000) [pid = 3764] [serial = 33] [outer = 00000000] 01:32:48 INFO - GECKO(3764) | ++DOMWINDOW == 34 (1C7D1400) [pid = 3764] [serial = 34] [outer = 1404C000] 01:32:48 INFO - GECKO(3764) | ++DOMWINDOW == 35 (1C7D4400) [pid = 3764] [serial = 35] [outer = 1404C000] 01:32:48 INFO - GECKO(3764) | ++DOMWINDOW == 36 (20BEB000) [pid = 3764] [serial = 36] [outer = 1404C000] 01:32:48 INFO - GECKO(3764) | [3764, Main Thread] WARNING: NS_ENSURE_TRUE(content && aRange) failed: file z:/build/build/src/editor/txtsvc/nsFilteredContentIterator.cpp, line 290 01:32:48 INFO - GECKO(3764) | [3764, Main Thread] WARNING: '!NodeIsInTraversalRange(mLast, mPre, RawRangeBoundary(mFirst, 0), aEnd)', file z:/build/build/src/dom/base/nsContentIterator.cpp, line 473 01:32:48 INFO - GECKO(3764) | ++DOMWINDOW == 37 (22A11800) [pid = 3764] [serial = 37] [outer = 1404C000] 01:32:49 INFO - GECKO(3764) | JavaScript error: chrome://formautofill/content/FormAutofillFrameScript.js, line 35: ReferenceError: setTimeout is not defined 01:32:49 INFO - GECKO(3764) | JavaScript error: chrome://formautofill/content/FormAutofillFrameScript.js, line 35: ReferenceError: setTimeout is not defined 01:32:49 INFO - GECKO(3764) | ++DOMWINDOW == 38 (24004800) [pid = 3764] [serial = 38] [outer = 1404C000] 01:32:49 INFO - GECKO(3764) | ++DOMWINDOW == 39 (117F4800) [pid = 3764] [serial = 39] [outer = 1404C000] 01:32:49 INFO - GECKO(3764) | ++DOMWINDOW == 40 (1CCF0400) [pid = 3764] [serial = 40] [outer = 1404C000] 01:32:49 INFO - GECKO(3764) | JavaScript error: data:,docShell.doCommand("cmd_cut");, line 1: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDocShell.doCommand] 01:32:49 INFO - GECKO(3764) | ++DOMWINDOW == 41 (22E38C00) [pid = 3764] [serial = 41] [outer = 1404C000] 01:32:49 INFO - GECKO(3764) | [3764, Main Thread] WARNING: NS_ENSURE_TRUE(content && aRange) failed: file z:/build/build/src/editor/txtsvc/nsFilteredContentIterator.cpp, line 290 01:32:49 INFO - GECKO(3764) | [3764, Main Thread] WARNING: NS_ENSURE_TRUE(aNode) failed: file z:/build/build/src/editor/libeditor/EditorBase.cpp, line 3636 01:32:49 INFO - GECKO(3764) | MEMORY STAT | vsize 904MB | vsizeMaxContiguous 532MB | residentFast 297MB | heapAllocated 96MB 01:32:50 INFO - TEST-OK | dom/browser-element/mochitest/test_browserElement_inproc_CopyPaste.html | took 5260ms 01:32:50 INFO - TEST-UNEXPECTED-FAIL | dom/browser-element/mochitest/test_browserElement_inproc_CopyPaste.html | assertion count 12 is more than expected 0 assertions 01:32:50 INFO - TEST-START | Shutdown

Need a bit of boilerplate here to avoid asserting. (I'll fold this into the AccessibleCaret patch)

Comment on attachment 8925353 [details] [diff] [review] addendum for part 8 r=me

Pushed by mpalmgren@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e4815e8465a2 part 1 - Collect NAC / generated content and call DestroyAnonymousContent / UnbindFromTree on those after the frames are destroyed. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/9a72b179c10d part 2 - Remove the nsIFrame::GenConProperty property and make aPostDestroyData deal with unbinding it. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/50c2aba68909 part 3 - Don't let <area>s mess up the frame's IsPrimaryFrame bit, since the frame is actually owned by the <img>. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/82659642e058 part 4 - Destroy frames before calling nsXBLBinding::ChangeDocument in a few places. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/e46bd0b42454 part 5 - Destroy the frames before removing the AccessibleCaret anonymous content (to ensure it happens before the UnbindFromTree). r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/cd9f14f9ea6d part 6 - Remove the workaround for the broken IsPrimaryFrame() in nsFrame::DestroyFrom since it now works also for NAC frames. r=bz

https://hg.mozilla.org/mozilla-central/rev/e4815e8465a2 https://hg.mozilla.org/mozilla-central/rev/9a72b179c10d https://hg.mozilla.org/mozilla-central/rev/50c2aba68909 https://hg.mozilla.org/mozilla-central/rev/82659642e058 https://hg.mozilla.org/mozilla-central/rev/e46bd0b42454 https://hg.mozilla.org/mozilla-central/rev/cd9f14f9ea6d