Last Comment Bug 605672 - "ASSERTION: Incorrect scope passed"
: "ASSERTION: Incorrect scope passed"
Status: RESOLVED FIXED
: [sg:critical?]
: assertion, testcase, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: XPConnect
: Trunk
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Peter Van der Beken [:peterv]
: xpconnect
:
:
: 326633 594645 622456
  Show dependency treegraph
 
Reported: 2010-10-19 17:08 PDT by Jesse Ruderman
Modified: 2011-06-20 18:08 PDT (History)
12 users (show)
jruderman: in‑testsuite+
See Also:
Crash Signature:
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  final+
  ---
  .14+
  .14-fixed
  .17+
  .17-fixed


Attachments
testcase (505 bytes, image/svg+xml)
2010-10-19 17:08 PDT, Jesse Ruderman
no flags Details
stack trace (3.55 KB, text/plain)
2010-10-19 17:09 PDT, Jesse Ruderman
no flags Details
crash testcase (703 bytes, text/html)
2010-10-25 05:19 PDT, moz_bug_r_a4
no flags Details
crash stack trace (11.78 KB, text/plain)
2010-10-25 09:39 PDT, Jesse Ruderman
no flags Details
v1 (8.36 KB, patch)
2010-11-12 13:37 PST, Peter Van der Beken [:peterv]
jst: review+
Details | Diff | Splinter Review
v1 followup (11.36 KB, patch)
2010-12-03 06:02 PST, Peter Van der Beken [:peterv]
jst: review+
Details | Diff | Splinter Review
191 branch patch (23.39 KB, patch)
2011-01-19 12:24 PST, Peter Van der Beken [:peterv]
clegnitto: approval1.9.1.17+
Details | Diff | Splinter Review
192 branch patch (22.71 KB, patch)
2011-01-19 15:18 PST, Peter Van der Beken [:peterv]
clegnitto: approval1.9.2.14+
Details | Diff | Splinter Review

Summon comment box

Description Jesse Ruderman 2010-10-19 17:08:40 PDT
Created attachment 484551 [details]
testcase

###!!! ASSERTION: Incorrect scope passed: 'wrapper->GetScope() == aOldScope', file js/src/xpconnect/src/xpcwrappednative.cpp, line 1506
Comment 1 Jesse Ruderman 2010-10-19 17:09:36 PDT
Created attachment 484552 [details]
stack trace
Comment 2 moz_bug_r_a4 2010-10-25 05:18:43 PDT
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.
Comment 3 moz_bug_r_a4 2010-10-25 05:19:52 PDT
Created attachment 485707 [details]
crash testcase

Trunk, 1.9.2 and 1.9.1 are affected.
Comment 4 Jesse Ruderman 2010-10-25 09:39:18 PDT
Created attachment 485754 [details]
crash stack trace
Comment 5 Johnny Stenback (:jst, jst@mozilla.com) 2010-11-01 16:57:09 PDT
Peter, you've dealt with bugs like this before :)
Comment 6 Jesse Ruderman 2010-11-11 17:01:11 PST
I'm seeing cases where this assertion is followed by additional assertions, which is distracting and scary.
Comment 7 Peter Van der Beken [:peterv] 2010-11-12 13:37:39 PST
Created attachment 490219 [details] [review]
v1
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2010-11-23 13:59:04 PST
Peter, is this patch ready for review?
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2010-11-30 11:57:48 PST
Comment on attachment 490219 [details] [review]
v1

r=jst
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2010-11-30 12:37:20 PST
Landed:

http://hg.mozilla.org/mozilla-central/rev/78df45be19d1
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2010-12-03 00:14:09 PST
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.
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2010-12-03 00:14:42 PST
http://hg.mozilla.org/mozilla-central/rev/e243ef4f1e0e (backout)
http://hg.mozilla.org/mozilla-central/rev/0ff6d5984287 (merge)
Comment 13 Johnny Stenback (:jst, jst@mozilla.com) 2010-12-03 00:31:42 PST
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
Comment 14 Peter Van der Beken [:peterv] 2010-12-03 06:02:25 PST
Created attachment 494984 [details] [review]
v1 followup

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.
Comment 15 Johnny Stenback (:jst, jst@mozilla.com) 2010-12-03 10:00:38 PST
Comment on attachment 494984 [details] [review]
v1 followup

Thanks peterv! r=jst
Comment 16 Johnny Stenback (:jst, jst@mozilla.com) 2010-12-03 15:22:30 PST
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
Comment 17 David Baron [:dbaron] 2010-12-08 14:43:27 PST
I backed this out to investigate whether it caused bug 617048:
http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=c7b784648b02
Comment 18 David Baron [:dbaron] 2010-12-09 10:14:13 PST
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.
Comment 19 Peter Van der Beken [:peterv] 2010-12-17 08:13:01 PST
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
Comment 20 ben turner [:bent] (vacation until 7/25) 2011-01-13 12:16:40 PST
I think this needs to be backported, it's most likely what is causing cross_fuzz bugs on 3.6 and 3.5.
Comment 21 ben turner [:bent] (vacation until 7/25) 2011-01-18 10:55:49 PST
*** Bug 622456 has been marked as a duplicate of this bug. ***
Comment 22 ben turner [:bent] (vacation until 7/25) 2011-01-18 10:56:28 PST
Given that bug 622456 is a dupe I think this should block the next stable releases.
Comment 23 Peter Van der Beken [:peterv] 2011-01-18 11:20:50 PST
The patches need small changes, I'm working on merging.
Comment 24 Christian Legnitto [:LegNeato] 2011-01-19 10:36:08 PST
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.
Comment 25 Peter Van der Beken [:peterv] 2011-01-19 11:30:54 PST
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.
Comment 26 Peter Van der Beken [:peterv] 2011-01-19 12:24:38 PST
Created attachment 505151 [details] [review]
191 branch patch

Passes mochitest.
Comment 27 Peter Van der Beken [:peterv] 2011-01-19 15:18:43 PST
Created attachment 505231 [details] [review]
192 branch patch

Passes mochitest.
Comment 28 Johnny Stenback (:jst, jst@mozilla.com) 2011-01-19 15:30:04 PST
Landed on branches:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8465508094bc
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d7c24aa07c30
Comment 29 Al Billings [:abillings] 2011-01-25 15:41:44 PST
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).

Note You need to log in before you can comment on or make changes to this bug.