Closed Bug 846096 Opened 10 years ago Closed 10 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
Attached file stacks — 

    
    

    
  
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.

    
    

    
  
https://tbpl.mozilla.org/?tree=Try&rev=28cb6944fd5c is designed to help figure this out

    
    

    
  
...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: 10 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.