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)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
(Keywords: intermittent-failure)
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
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
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
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
Assignee | ||
Comment 3•12 years ago
|
||
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
Assignee | ||
Comment 4•12 years ago
|
||
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
Assignee | ||
Comment 5•12 years ago
|
||
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.
Assignee | ||
Comment 6•12 years ago
|
||
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.
Assignee | ||
Comment 7•12 years ago
|
||
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.
Assignee | ||
Comment 8•12 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=28cb6944fd5c is designed to help figure this out
Assignee | ||
Comment 9•12 years ago
|
||
...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).
Assignee | ||
Comment 10•12 years ago
|
||
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.
Assignee | ||
Comment 11•12 years ago
|
||
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.
Assignee | ||
Comment 12•12 years ago
|
||
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.
Comment 13•12 years ago
|
||
Does the testcase in bug 785211 help? Or bug 803410?
Assignee | ||
Comment 14•12 years ago
|
||
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.
Comment 15•12 years ago
|
||
Comment 16•12 years ago
|
||
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.
Comment 17•12 years ago
|
||
Assignee | ||
Comment 18•12 years ago
|
||
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)
Assignee | ||
Comment 19•12 years ago
|
||
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.
Assignee | ||
Comment 20•12 years ago
|
||
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)
Assignee | ||
Comment 21•12 years ago
|
||
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.
Assignee | ||
Comment 22•12 years ago
|
||
I made the annotations a bit more consistent in:
https://hg.mozilla.org/integration/mozilla-inbound/rev/326cf12235ec
Whiteboard: [leave open]
Assignee | ||
Comment 23•12 years ago
|
||
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.
Assignee | ||
Comment 24•12 years ago
|
||
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.
Comment 25•12 years ago
|
||
Comment 26•12 years ago
|
||
Comment 27•12 years ago
|
||
Comment 28•12 years ago
|
||
Comment 29•12 years ago
|
||
Comment 30•12 years ago
|
||
Comment 31•12 years ago
|
||
The chrome assertions are spilling out of bug451540_window.xul
Comment 33•12 years ago
|
||
Comment 34•12 years ago
|
||
Comment 35•12 years ago
|
||
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.
Comment 39•12 years ago
|
||
Comment 40•12 years ago
|
||
Comment 41•12 years ago
|
||
Blocks: 847076
Assignee | ||
Comment 43•12 years ago
|
||
Reverted Ms2ger's patch in:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c23504b77703
https://tbpl.mozilla.org/?tree=Try&rev=107a6f1eba0a may lead to something more useful (based on comment 32)
For the time being I would r+ a patch to comment out these assertions.
Comment 45•12 years ago
|
||
Assignee | ||
Comment 46•12 years ago
|
||
(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.
Comment 47•12 years ago
|
||
Comment 48•12 years ago
|
||
Assignee | ||
Comment 49•12 years ago
|
||
Attachment #720550 -
Flags: review?(bugs)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → dbaron
Assignee | ||
Comment 50•12 years ago
|
||
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 51•12 years ago
|
||
Comment 52•12 years ago
|
||
Comment 53•12 years ago
|
||
Comment 54•12 years ago
|
||
Comment 55•12 years ago
|
||
Comment 56•12 years ago
|
||
Comment 57•12 years ago
|
||
Comment 58•12 years ago
|
||
Comment 59•12 years ago
|
||
Comment 60•12 years ago
|
||
Keywords: intermittent-failure
Comment 61•12 years ago
|
||
Comment 62•12 years ago
|
||
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)
Assignee | ||
Comment 63•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Whiteboard: [leave open]
Assignee | ||
Updated•12 years ago
|
Component: Mochitest → DOM: Traversal-Range
Product: Testing → Core
Assignee | ||
Comment 64•12 years ago
|
||
(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.
Comment 65•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Assignee | ||
Comment 66•12 years ago
|
||
SimpleTest.expectAssertions() functions covering this assertion removed in:
https://hg.mozilla.org/integration/mozilla-inbound/rev/38a945f2c79d
Comment 67•12 years ago
|
||
Updated•12 years ago
|
Component: DOM: Traversal-Range → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•