Closed Bug 419527 Opened 13 years ago Closed 13 years ago

"ASSERTION: Wrong root" with XBL

Categories

(Core :: XBL, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files, 2 obsolete files)

Attached file testcase
Loading the testcase triggers:

###!!! ASSERTION: Wrong root: '!aRoot || (nsContentUtils::ContentIsDescendantOf(aStartN, aRoot) && nsContentUtils::ContentIsDescendantOf(aEndN, aRoot) && aRoot == IsValidBoundary(aStartN) && aRoot == IsValidBoundary(aEndN))', file /Users/jruderman/trunk/mozilla/content/base/src/nsRange.cpp, line 443
I'll look at this. Have to check if this is an regression from the
unconnected-ranges patch.
Assignee: nobody → Olli.Pettay
Flags: blocking1.9?
Flags: tracking1.9? → blocking1.9?
Not a blocker unless we can get a crash out of this. Smaug, would still be great if you can figure out what's going on.
Flags: wanted1.9.0.x+
Flags: blocking1.9?
Flags: blocking1.9+
Sure I will try to fix this once I've done with the P1 I have.
Or perhaps when I want something easier to fix.
Attached patch proposed patch (obsolete) — Splinter Review
I think we want to limit re-rooting of ranges, if mRoot is bindingparent.
Passes mochitest, browser and chrome.
Attachment #308473 - Flags: superreview?(jonas)
Attachment #308473 - Flags: review?(jonas)
Please make sure to check in tests with the patch?
Attached patch same with mochitest (obsolete) — Splinter Review
Attachment #308473 - Attachment is obsolete: true
Attachment #308597 - Flags: superreview?(jonas)
Attachment #308597 - Flags: review?(jonas)
Attachment #308473 - Flags: superreview?(jonas)
Attachment #308473 - Flags: review?(jonas)
Comment on attachment 308597 [details] [diff] [review]
same with mochitest

This doesn't seem right when mMaySpanAnonymousSubtrees is set to true. I think what you want to do is to simply call

newRoot = IsValidBoundary(mStartParent)

And assert that that returns a non-null value.
Attachment #308597 - Flags: superreview?(jonas)
Attachment #308597 - Flags: review?(jonas)
Attachment #308597 - Flags: review-
Attached patch v2Splinter Review
Right, this should be enough.
Attachment #308597 - Attachment is obsolete: true
Attachment #308831 - Flags: superreview?(jonas)
Attachment #308831 - Flags: review?(jonas)
Attachment #308831 - Flags: superreview?(jonas)
Attachment #308831 - Flags: superreview+
Attachment #308831 - Flags: review?(jonas)
Attachment #308831 - Flags: review+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Smaug, the presence of the mochitest in the patch means this doesn't need a crashtest, right?  (Without the patch, does the mochitest trigger the assertion?)
Flags: in-testsuite?
Flags: wanted1.9.0.x+
So.. this mochitest seems to leak sometimes.  From the last leaking test run on tinderbox, the list of leaked urls includes:

  http://localhost:8888/tests/content/base/test/test_bug419527.xhtml
  http://localhost:8888/tests/content/base/test/test_bug419527.xhtml
  http://localhost:8888/tests/content/base/test/test_bug419527.xhtml
  http://localhost:8888/tests/content/base/test/test_bug419527.xhtml
  http://localhost:8888/tests/content/base/test/test_bug419527.xhtml#rangebinding

right at the end (as in, the last-created urls that leaked).  All the other ones are chrome XBL stuff or the urls secman is holding onto as part of its principals hashtable.
Depends on: 478528
Depends on: 939633
You need to log in before you can comment on or make changes to this bug.