Closed Bug 605672 Opened 9 years ago Closed 9 years ago

"ASSERTION: Incorrect scope passed"

Categories

(Core :: XPConnect, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+
blocking1.9.2 --- .14+
status1.9.2 --- .14-fixed
blocking1.9.1 --- .17+
status1.9.1 --- .17-fixed

People

(Reporter: jruderman, Assigned: peterv)

References

(Blocks 2 open bugs)

Details

(4 keywords, Whiteboard: [sg:critical?])

Attachments

(8 files)

Attached image testcase
###!!! ASSERTION: Incorrect scope passed: 'wrapper->GetScope() == aOldScope', file js/src/xpconnect/src/xpcwrappednative.cpp, line 1506
Attached file stack trace
blocking2.0: --- → ?
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDOMDocumentType.cpp#271
271     nsCOMPtr<nsIDocument> oldOwnerDoc =
272       do_QueryInterface(nsContentUtils::GetDocumentFromContext());

It seems that we use an unrelated document as oldOwnerDoc.

I'll attach a crash testcase.
Attached file crash testcase
Trunk, 1.9.2 and 1.9.1 are affected.
Whiteboard: [sg:critical?]
Attached file crash stack trace
Peter, you've dealt with bugs like this before :)
Assignee: nobody → peterv
blocking2.0: ? → final+
I'm seeing cases where this assertion is followed by additional assertions, which is distracting and scary.
Attached patch v1Splinter Review
Peter, is this patch ready for review?
Attachment #490219 - Flags: review?(jst)
Comment on attachment 490219 [details] [diff] [review]
v1

r=jst
Attachment #490219 - Flags: review?(jst) → review+
Landed:

http://hg.mozilla.org/mozilla-central/rev/78df45be19d1
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
This ended up actually causing test failures, except the test failures were hidden because of cross compartment adoptNode throwing. Once the fix for bug 601803 landed the assertions that this patch introduced showed up. Now I'm not 100% sure that backing this out instead of backing out the fix for bug 601803 was the right thing to do, but given that bug 601803 is a beta blocker, and this is not, I decided to back this out instead. And bug 615763 was filed after this landed as well, which seems to suggest that something really wasn't right with this change.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The failures caused by the combination of this and the adoptNode fix was these:

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/mozilla-central_fedora-debug_test-crashtest/build/reftest/tests/editor/libeditor/html/crashtests/382778-1.html | assertion count 1 is more than expected 0 assertions
REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/mozilla-central_fedora-debug_test-crashtest/build/reftest/tests/editor/libeditor/base/crashtests/382527-1.html | assertion count 1 is more than expected 0 assertions
REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/mozilla-central_fedora-debug_test-crashtest/build/reftest/tests/js/src/xpconnect/crashtests/346512-1.xhtml | assertion count 1 is more than expected 0 assertions
Attached patch v1 followupSplinter Review
This should fix the orange. The problem only happened when adopting nodes with children. We were using the toplevel node's wrapper as the scope, but because adopting traverses the tree downwards that wrapper was the first to be reparented and moved to the new scope. Using it as the scope for all the descendants was wrong. I ended up removing the aOldScope argument altogether, we should just use the existing wrapper of every node when reparenting it.
Attachment #494984 - Flags: review?(jst)
Comment on attachment 494984 [details] [diff] [review]
v1 followup

Thanks peterv! r=jst
Attachment #494984 - Flags: review?(jst) → review+
Re-landed v1 and also landed the followup fix.

http://hg.mozilla.org/mozilla-central/rev/3b2d178f7299
http://hg.mozilla.org/mozilla-central/rev/e52f4987ec94
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
I backed this out to investigate whether it caused bug 617048:
http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=c7b784648b02
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
So based on the backout, either this or bug 588873 caused bug 617048.  (I think my money is on bug 588873.)

I think it's ok to reland this after beta8 ships, but make sure it doesn't reland the same day as bug 588873.
I relanded this, I'll be monitoring crash-stats.

http://hg.mozilla.org/mozilla-central/rev/83cb5ea3fe30
http://hg.mozilla.org/mozilla-central/rev/ad8da8012e3a
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
I think this needs to be backported, it's most likely what is causing cross_fuzz bugs on 3.6 and 3.5.
Given that bug 622456 is a dupe I think this should block the next stable releases.
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
The patches need small changes, I'm working on merging.
blocking1.9.1: ? → .17+
blocking1.9.2: ? → .14+
We would like to get these patches into the branches today, as the fuzzer is public (and we are now past code freeze). Is that possible? This should take priority over FF4 work FWIW.
Blocks: 622456
I'm trying to get them done. I have patches that build but am currently running mochitests since the patches are a bit different and I want to make sure I didn't break anything while merging.
Attached patch 191 branch patchSplinter Review
Passes mochitest.
Attachment #505151 - Flags: approval1.9.1.17?
Attached patch 192 branch patchSplinter Review
Passes mochitest.
Attachment #505231 - Flags: approval1.9.2.14?
Attachment #505151 - Flags: approval1.9.1.17? → approval1.9.1.17+
Attachment #505231 - Flags: approval1.9.2.14? → approval1.9.2.14+
Verified crash in 1.9.1.16 with crashtest and fix in 1.9.1.17 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.17) Gecko/20110121 Firefox/3.5.17 ( .NET CLR 3.5.30729).

Verified crash in 1.9.2.13 with crashtest and fix in 1.9.2.14 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.14) Gecko/20110121 Firefox/3.6.14 ( .NET CLR 3.5.30729).
Group: core-security
You need to log in before you can comment on or make changes to this bug.