Closed Bug 846096 Opened 12 years ago Closed 12 years ago

varying test causes ASSERTION: Start parent and end parent give different root!: 'newRoot == IsValidBoundary(mEndParent)' and ASSERTION: Wrong root: '!aRoot || aNotInsertedYet || (nsContentUtils::ContentIsDescendantOf(aStartN, aRoot) && nsContentUtils::Co

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla22

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

stacks
8.63 KB, text/plain
Details
Bail out of nsRange::ParentChainChanged if the nodes aren't in a connected subtree.
1.81 KB, patch
smaug
: review+
Details | Diff | Splinter Review
There's a varying test that's causing two assertions: 16:41:00 INFO - ^G###!!! ASSERTION: Start parent and end parent give different root!: 'newRoot == IsValidBoundary(mEndParent)', file ../../../../content/base/src/nsRange.cpp, line 667 16:41:00 INFO - ^G###!!! ASSERTION: Wrong root: '!aRoot || aNotInsertedYet || (nsContentUtils::ContentIsDescendantOf(aStartN, aRoot) && nsContentUtils::ContentIsDescendantOf(aEndN, aRoot) && aRoot == IsValidBoundary(aStartN) && aRoot == IsValidBoundary(aEndN))', file ../../../../content/base/src/nsRange.cpp, line 795 I incorrectly annotated something as 0-2 for this this morning, before noticing it was an assertion during GC and therefore fluctuating across many tests. We should figure out which test is causing it (probably around the earliest of the tests that hit it, force that test to have a SpecialPowers.gc() call in it, and then annotate that test for the assertions. https://tbpl.mozilla.org/php/getParsedLog.php?id=20162534&tree=Mozilla-Inbound Rev4 MacOSX Snow Leopard 10.6 mozilla-inbound debug test mochitest-4 on 2013-02-27 16:32:34 PST for push 43a54aaca03c slave: talos-r4-snow-075 layout/base/tests/test_preserve3d_sorting_hit_testing.html
Histogram: 1 /tests/layout/base/tests/test_bug93077-6.html 2 /tests/layout/base/tests/test_event_target_radius.html 7 /tests/layout/base/tests/test_mozPaintCount.html 4 /tests/layout/base/tests/test_reftests_with_caret.html 350 /tests/layout/base/tests/test_reftests_with_caret.html 5 /tests/layout/base/tests/test_remote_frame.html 1 /tests/layout/base/tests/test_scroll_event_ordering.html 1 /tests/layout/base/tests/test_scroll_selection_into_view.html
Above came from: $ find mozilla-inbound-* -name '*mochitest-4*.gz' -exec sh -c "zgrep -B1000 'Start parent and end parent give different root' {} | grep 'TEST-' | tail -1" \; | cut -d\| -f2 | sort | uniq -c
And the assertion never occurred more than once in a log: $ find mozilla-inbound-* -name '*mochitest-4*.gz' -exec zgrep -H 'Start parent and end parent give different root' {} \; | cut -d: -f1 | uniq -c | cut -b1-8 | uniq -c 371 1
https://tbpl.mozilla.org/php/getParsedLog.php?id=20166512&tree=Mozilla-Inbound Rev5 MacOSX Mountain Lion 10.8 mozilla-inbound debug test mochitest-4 on 2013-02-27 18:45:21 PST for push 2186eacb635c slave: talos-mtnlion-r5-014 18:52:04 INFO - 17583 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_remote_frame.html | Assertion count 2 is greater than expected range 0-0 assertions.
https://tbpl.mozilla.org/php/getParsedLog.php?id=20170285&tree=Mozilla-Inbound Rev5 MacOSX Mountain Lion 10.8 mozilla-inbound debug test mochitest-4 on 2013-02-27 20:46:19 PST for push 535ef5219dac slave: talos-mtnlion-r5-054 20:53:02 INFO - 17616 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_remote_frame.html | Assertion count 2 is greater than expected range 0-0 assertions.
https://tbpl.mozilla.org/php/getParsedLog.php?id=20171598&tree=Mozilla-Inbound Rev3 Fedora 12x64 mozilla-inbound debug test mochitest-4 on 2013-02-27 21:28:40 PST for push 7b7e5220c420 slave: talos-r3-fed64-013 21:35:48 INFO - 15287 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_preserve3d_sorting_hit_testing.html | Assertion count 2 is greater than expected range 0-0 assertions.
...didn't show anything useful for this bug (but did for 2 others, bug 846099 and the unfiled spatter from toolkit/content/tests/chrome/test_bug451286.xul across the rest of that directory).
Though, it's actually interesting that forcing GC between tests prevents this assertion from happening at all. Not useful in figuring out the source, though.
https://tbpl.mozilla.org/php/getParsedLog.php?id=20189791&tree=Mozilla-Inbound Rev4 MacOSX Lion 10.7 mozilla-inbound debug test mochitest-4 on 2013-02-28 11:02:49 PST for push 67060725ec8d slave: talos-r4-lion-074 11:12:10 INFO - 17387 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_event_target_radius.html | Assertion count 2 is greater than expected range 0-0 assertions.
https://tbpl.mozilla.org/php/getParsedLog.php?id=20197804&tree=Mozilla-Inbound Rev4 MacOSX Snow Leopard 10.6 mozilla-inbound debug test mochitest-4 on 2013-02-28 14:48:13 PST for push 7c2b8a955b46 slave: talos-r4-snow-072 14:56:37 INFO - 17379 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_image_layers.html | Assertion count 5 is greater than expected range 0-3 assertions.
Does the testcase in bug 785211 help? Or bug 803410?
https://tbpl.mozilla.org/php/getParsedLog.php?id=20201948&tree=Mozilla-Inbound Rev4 MacOSX Snow Leopard 10.6 mozilla-inbound debug test mochitest-4 on 2013-02-28 17:01:14 PST for push 04ce59868888 slave: talos-r4-snow-011 17:09:22 INFO - 17506 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_preserve3d_sorting_hit_testing.html | Assertion count 2 is greater than expected range 0-0 assertions.
https://tbpl.mozilla.org/php/getParsedLog.php?id=20204924&tree=Mozilla-Inbound 18:39:37 INFO - 19224 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_bug562554.xul | Assertion count 2 is greater than expected range 0-0 assertions.
more complete histogram: 1 /tests/layout/base/tests/test_bug93077-6.html 2 /tests/layout/base/tests/test_event_target_radius.html 11 /tests/layout/base/tests/test_mozPaintCount.html 466 /tests/layout/base/tests/test_reftests_with_caret.html 5 /tests/layout/base/tests/test_remote_frame.html 1 /tests/layout/base/tests/test_scroll_event_ordering.html 1 /tests/layout/base/tests/test_scroll_selection_into_view.html 3 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_bug554279.xul 3 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_bug557987.xul 1 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_bug585946.xul 6 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_bug624329.xul 30 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_bug792324.xul 10 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_button.xul 2 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_chromemargin.xul 57 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_closemenu_attribute.xul 13 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_colorpicker_popup.xul 8 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_contextmenu_list.xul 3 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_datepicker.xul 1 chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_dialogfocus.xul 41 none (between TEST-END and TEST-START; I should have counted these differently)
Stacks (from Linux 32-bit, which tends to have good stacks): ###!!! ASSERTION: Start parent and end parent give different root!: 'newRoot == IsValidBoundary(mEndParent)', file ../../../../content/base/src/nsRange.cpp, line 667 nsRange::ParentChainChanged(nsIContent*) [content/base/src/nsRange.cpp:666] nsNodeUtils::ParentChainChanged(nsIContent*) [content/base/src/nsNodeUtils.h:119] nsGenericHTMLElement::UnbindFromTree(bool, bool) [content/html/content/src/nsGenericHTMLElement.cpp:655] nsGenericHTMLFormElement::UnbindFromTree(bool, bool) [content/html/content/src/nsGenericHTMLElement.cpp:2359] mozilla::dom::HTMLTextAreaElement::UnbindFromTree(bool, bool) [content/html/content/src/HTMLTextAreaElement.cpp:1082] mozilla::dom::Element::UnbindFromTree(bool, bool) [content/base/src/Element.cpp:1373] nsGenericHTMLElement::UnbindFromTree(bool, bool) [content/html/content/src/nsGenericHTMLElement.cpp:655] mozilla::dom::Element::UnbindFromTree(bool, bool) [content/base/src/Element.cpp:1373] nsGenericHTMLElement::UnbindFromTree(bool, bool) [content/html/content/src/nsGenericHTMLElement.cpp:655] mozilla::dom::HTMLSharedElement::UnbindFromTree(bool, bool) [content/html/content/src/HTMLSharedElement.cpp:309] nsDocument::cycleCollection::UnlinkImpl(void*) [content/base/src/nsDocument.cpp:1744] nsHTMLDocument::cycleCollection::UnlinkImpl(void*) [content/html/document/src/nsHTMLDocument.cpp:224] nsCycleCollector::CollectWhite(nsICycleCollectorListener*) [xpcom/base/nsCycleCollector.cpp:2407] nsCycleCollector::FinishCollection(nsICycleCollectorListener*) [xpcom/base/nsCycleCollector.cpp:3034] nsCycleCollectorRunner::Collect(bool, nsCycleCollectorResults*, nsICycleCollectorListener*) [xpcom/base/nsCycleCollector.cpp:3274] nsCycleCollector_collect(bool, nsCycleCollectorResults*, nsICycleCollectorListener*) [xpcom/base/nsCycleCollector.cpp:3362] nsJSContext::CycleCollectNow(nsICycleCollectorListener*, int, bool) [dom/base/nsJSEnvironment.cpp:2771] CCTimerFired [dom/base/nsJSEnvironment.cpp:2977] ... ###!!! ASSERTION: Wrong root: '!aRoot || aNotInsertedYet || (nsContentUtils::ContentIsDescendantOf(aStartN, aRoot) && nsContentUtils::ContentIsDescendantOf(aEndN, aRoot) && aRoot == IsValidBoundary(aStartN) && aRoot == IsValidBoundary(aEndN))', file ../../../../content/base/src/nsRange.cpp, line 795 nsRange::DoSetRange(nsINode*, int, nsINode*, int, nsINode*, bool) [content/base/src/nsRange.cpp:790] nsRange::ParentChainChanged(nsIContent*) [content/base/src/nsRange.cpp:670] nsNodeUtils::ParentChainChanged(nsIContent*) [content/base/src/nsNodeUtils.h:119] nsGenericHTMLElement::UnbindFromTree(bool, bool) [content/html/content/src/nsGenericHTMLElement.cpp:655] nsGenericHTMLFormElement::UnbindFromTree(bool, bool) [content/html/content/src/nsGenericHTMLElement.cpp:2359] mozilla::dom::HTMLTextAreaElement::UnbindFromTree(bool, bool) [content/html/content/src/HTMLTextAreaElement.cpp:1082] mozilla::dom::Element::UnbindFromTree(bool, bool) [content/base/src/Element.cpp:1373] nsGenericHTMLElement::UnbindFromTree(bool, bool) [content/html/content/src/nsGenericHTMLElement.cpp:655] mozilla::dom::Element::UnbindFromTree(bool, bool) [content/base/src/Element.cpp:1373] nsGenericHTMLElement::UnbindFromTree(bool, bool) [content/html/content/src/nsGenericHTMLElement.cpp:655] mozilla::dom::HTMLSharedElement::UnbindFromTree(bool, bool) [content/html/content/src/HTMLSharedElement.cpp:309] nsDocument::cycleCollection::UnlinkImpl(void*) [content/base/src/nsDocument.cpp:1744] nsHTMLDocument::cycleCollection::UnlinkImpl(void*) [content/html/document/src/nsHTMLDocument.cpp:224] nsCycleCollector::CollectWhite(nsICycleCollectorListener*) [xpcom/base/nsCycleCollector.cpp:2407] nsCycleCollector::FinishCollection(nsICycleCollectorListener*) [xpcom/base/nsCycleCollector.cpp:3034] nsCycleCollectorRunner::Collect(bool, nsCycleCollectorResults*, nsICycleCollectorListener*) [xpcom/base/nsCycleCollector.cpp:3274] nsCycleCollector_collect(bool, nsCycleCollectorResults*, nsICycleCollectorListener*) [xpcom/base/nsCycleCollector.cpp:3362] nsJSContext::CycleCollectNow(nsICycleCollectorListener*, int, bool) [dom/base/nsJSEnvironment.cpp:2771] CCTimerFired [dom/base/nsJSEnvironment.cpp:2977] My suspicion is that forcing GC prevents the assertion from firing by ensuring a particular destruction order in some way.
What should we do about these assertions? I suspect the assertions (or maybe even the code) are invalid in some way under some cycle collection orderings. They're being particularly problematic with unexpected assertions in mochitest causing test failures.
Flags: needinfo?(bugs)
Note that the worst cases of this bug (see comment 18) are currently annotated with SimpleTest.expectAssertions() calls for it, but I'd rather not annotate down to the lowest-frequency ones.
I made the annotations a bit more consistent in: https://hg.mozilla.org/integration/mozilla-inbound/rev/326cf12235ec
Whiteboard: [leave open]
https://tbpl.mozilla.org/php/getParsedLog.php?id=20211460&tree=Mozilla-Inbound Rev4 MacOSX Lion 10.7 mozilla-inbound debug test mochitest-other on 2013-02-28 22:32:46 PST for push b8a59825d718 slave: talos-r4-lion-029 22:49:52 INFO - 16588 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_bug509732.xul | Assertion count 2 is greater than expected range 0-0 assertions.
https://tbpl.mozilla.org/php/getParsedLog.php?id=20212213&tree=Mozilla-Inbound Rev4 MacOSX Snow Leopard 10.6 mozilla-inbound debug test mochitest-other on 2013-02-28 23:05:15 PST for push 777d41b883a9 slave: talos-r4-snow-050 23:21:12 INFO - 16613 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_bug557987.xul | Assertion count 2 is greater than expected range 0-0 assertions.
The chrome assertions are spilling out of bug451540_window.xul
So the problem is roughly this. We have an nsRange whose root is a <textarea>, and whose start and end pointers are in the native anonymous content for that <textarea>. When we tear down the frame tree we end up in HTMLTextAreaElement::UnbindFromFrame, and then nsTextEditorState::UnbindFromFrame. That ends up calling nsContentUtils::DestroyAnonymousContent on the root anonymous node. We set up an AnonymousContentDestroyer to run off a script runner and it calls UnbindFromTree on the root anonymous node. This mStart/EndParent no longer chain up to mRoot. But no ContentRemoved notification was ever fired, so the nsRange has no idea that its messed up. This bug manifests because later the cycle collector runs and it unlinks the NAC before it unlinks the nsRange. The start and end parent end up with null parent pointers and this assertion fires. I believe that if we asserted that mStartParent and mEndParent chain up to mRoot that assertion would fire 100% of the time.
For the time being I would r+ a patch to comment out these assertions.
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #43) > https://tbpl.mozilla.org/?tree=Try&rev=107a6f1eba0a may lead to something > more useful (based on comment 32) It didn't; doing GC that way didn't trigger the assertions on that test, and didn't suppress them from later tests.
Assignee: nobody → dbaron
Try run: https://tbpl.mozilla.org/?tree=Try&rev=b378710e85da If this patch gets review and the try run looks good, feel free to push it to inbound to (hopefully) resolve the oranges. (Also, if it's just orange de-annotation that's needed to clean up the try run, feel free to do that and push to inbound.)
Comment on attachment 720550 [details] [diff] [review] Bail out of nsRange::ParentChainChanged if the nodes aren't in a connected subtree. So this should happen only with NAC, right? Could you add NS_ASSERTION(mEndParent->IsInNativeAnonymousSubtree(), "This special case should happen only with NAC!") inside the if.
Attachment #720550 - Flags: review?(bugs) → review+
Flags: needinfo?(bugs)
Whiteboard: [leave open]
Component: Mochitest → DOM: Traversal-Range
Product: Testing → Core
(In reply to Olli Pettay [:smaug] from comment #62) > So this should happen only with NAC, right? > Could you add NS_ASSERTION(mEndParent->IsInNativeAnonymousSubtree(), > "This special case should happen only with NAC!") > inside the if. This assertion (I expanded the "NAC" abbreviation, though) hasn't fired yet in our tests (I've been pulling logs from tinderbox), so I think that confirms at least that what we're seeing is native-anonymous content only.
https://hg.mozilla.org/mozilla-central/rev/9b6039f3101a
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
SimpleTest.expectAssertions() functions covering this assertion removed in: https://hg.mozilla.org/integration/mozilla-inbound/rev/38a945f2c79d
Component: DOM: Traversal-Range → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: