605672 - "ASSERTION: Incorrect scope passed"

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: Incorrect scope passed: 'wrapper->GetScope() == aOldScope', file js/src/xpconnect/src/xpcwrappednative.cpp, line 1506

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[moz_bug_r_a4]

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]

Attachment: crash testcase

Trunk, 1.9.2 and 1.9.1 are affected.

Comment 4

[Jesse Ruderman]

Attachment: crash stack trace

Comment 5

[Johnny Stenback (:jst)]

Peter, you've dealt with bugs like this before :)

Comment 6

[Jesse Ruderman]

I'm seeing cases where this assertion is followed by additional assertions, which is distracting and scary.

Comment 7

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 8

[Johnny Stenback (:jst)]

Peter, is this patch ready for review?

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 490219 [details] [diff] [review]
v1

r=jst

Comment 10

[Johnny Stenback (:jst)]

Landed:

http://hg.mozilla.org/mozilla-central/rev/78df45be19d1

Comment 11

[Johnny Stenback (:jst)]

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)]

http://hg.mozilla.org/mozilla-central/rev/e243ef4f1e0e (backout)
http://hg.mozilla.org/mozilla-central/rev/0ff6d5984287 (merge)

Comment 13

[Johnny Stenback (:jst)]

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]]

Attachment: 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)]

Comment on attachment 494984 [details] [diff] [review]
v1 followup

Thanks peterv! r=jst

Comment 16

[Johnny Stenback (:jst)]

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: (⌚️UTC-4, no longer working on Mozilla)]

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: (⌚️UTC-4, no longer working on Mozilla)]

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]]

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 (not reading bugmail, use the needinfo flag!)]

I think this needs to be backported, it's most likely what is causing cross_fuzz bugs on 3.6 and 3.5.

Comment 22

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Given that bug 622456 is a dupe I think this should block the next stable releases.

Comment 23

[Peter Van der Beken [:peterv]]

The patches need small changes, I'm working on merging.

Comment 24

[LegNeato]

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]]

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]]

Attachment: 191 branch patch

Passes mochitest.

Comment 27

[Peter Van der Beken [:peterv]]

Attachment: 192 branch patch

Passes mochitest.

Comment 28

[Johnny Stenback (:jst)]

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 - ex-MoCo]]

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).

Comment 30

[Jesse Ruderman]

http://hg.mozilla.org/mozilla-central/rev/90c1ab45b6c5