Bugzilla@Mozilla – Bug 605672
"ASSERTION: Incorrect scope passed"
Last modified: 2011-06-20 18:08:31 PDT
Summon comment box
Created attachment 484551 [details] testcase ###!!! ASSERTION: Incorrect scope passed: 'wrapper->GetScope() == aOldScope', file js/src/xpconnect/src/xpcwrappednative.cpp, line 1506
Created attachment 484552 [details] stack trace
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.
Created attachment 485707 [details] crash testcase Trunk, 1.9.2 and 1.9.1 are affected.
Created attachment 485754 [details] crash stack trace
Peter, you've dealt with bugs like this before :)
I'm seeing cases where this assertion is followed by additional assertions, which is distracting and scary.
Created attachment 490219 [details] [review] v1
Peter, is this patch ready for review?
Comment on attachment 490219 [details] [review] v1 r=jst
Landed: http://hg.mozilla.org/mozilla-central/rev/78df45be19d1
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.
http://hg.mozilla.org/mozilla-central/rev/e243ef4f1e0e (backout) http://hg.mozilla.org/mozilla-central/rev/0ff6d5984287 (merge)
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
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 on attachment 494984 [details] [review] v1 followup Thanks peterv! r=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
I backed this out to investigate whether it caused bug 617048: http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=c7b784648b02
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
I think this needs to be backported, it's most likely what is causing cross_fuzz bugs on 3.6 and 3.5.
*** Bug 622456 has been marked as a duplicate of this bug. ***
Given that bug 622456 is a dupe I think this should block the next stable releases.
The patches need small changes, I'm working on merging.
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.
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.
Created attachment 505151 [details] [review] 191 branch patch Passes mochitest.
Created attachment 505231 [details] [review] 192 branch patch Passes mochitest.
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
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).
http://hg.mozilla.org/mozilla-central/rev/90c1ab45b6c5