Last Comment Bug 679494 - "compartment mismatched" when listening message event
: "compartment mismatched" when listening message event
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Bobby Holley (:bholley) (busy with Stylo)
:
Mentors:
: 688364 (view as bug list)
Depends on:
Blocks: 679539
  Show dependency treegraph
 
Reported: 2011-08-16 13:57 PDT by Alexandre Poirot [:ochameau]
Modified: 2015-01-14 05:32 PST (History)
8 users (show)
Ms2ger: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stacktrace (7.96 KB, text/plain)
2011-08-17 11:41 PDT, Bobby Holley (:bholley) (busy with Stylo)
no flags Details
Bug 679494 - Wrap value returned from nsDOMMessageEvent::GetData. v1 (1.64 KB, patch)
2011-08-18 12:10 PDT, Bobby Holley (:bholley) (busy with Stylo)
mrbkap: review+
christian: approval‑mozilla‑beta-
Details | Diff | Splinter Review
Bug 679494 - Tests. v1 (1.83 KB, patch)
2011-08-18 12:10 PDT, Bobby Holley (:bholley) (busy with Stylo)
no flags Details | Diff | Splinter Review
Bug 679494 - Tests v2 (2.55 KB, patch)
2011-08-18 13:36 PDT, Bobby Holley (:bholley) (busy with Stylo)
khuey: review+
christian: approval‑mozilla‑beta-
Details | Diff | Splinter Review
Bug 679494 - UUID rev followup. v1 (822 bytes, patch)
2011-08-22 10:53 PDT, Bobby Holley (:bholley) (busy with Stylo)
khuey: review+
jst: approval‑mozilla‑beta-
Details | Diff | Splinter Review

Description Alexandre Poirot [:ochameau] 2011-08-16 13:57:35 PDT
Assertion failure: compartment mismatched, at /home/pouf/mozilla-central/js/src/jscntxtinlines.h:120

when running the following script in JS console:

var w=top.opener.gBrowser.contentDocument.defaultView; 
w.addEventListener("message", function(event) {
  alert(event.data);
}, false);
w.postMessage("test");

I suppose that it only occurs when we listen to `message` event from another compartment than the event source document.
Comment 1 Blake Kaplan (:mrbkap) 2011-08-16 14:26:44 PDT
Alex found me on IRC and was kind enough to pastebin a stack trace. The relevant bit is:

#9  0x025800a8 in js::CallJSPropertyOp (cx=0xb1d0b570, op=
    0x19b0a65 <nsIDOMMessageEvent_GetData(JSContext*, JSObject*, jsid, jsval*)>, receiver=0xad49be40, id=..., vp=0xbfffb638)

this shows us that the problem is a getter returning a value in another compartment. Some quick MXRing later shows us that the problem is that nsDOMMessageEvent::GetData passes through its jsval without wrapping it. So, we need to call JS_WrapValue before returning the data, but in order to do that, we're going to need to add [implicit_jscontext] to the IDL in order to get a cx with with to call JS_WrapValue.
Comment 2 Bobby Holley (:bholley) (busy with Stylo) 2011-08-17 11:41:31 PDT
Created attachment 553858 [details]
stacktrace

Attaching the full stack trace to give n00bs like myself a better sense of what's going on.
Comment 3 Dave Townsend [:mossop] 2011-08-18 11:36:53 PDT
What is the priority of getting a fix here? This is causing the Jetpack tests to fail and presumably causing problems for add-ons using the functionality we expose
Comment 4 Bobby Holley (:bholley) (busy with Stylo) 2011-08-18 11:41:05 PDT
(In reply to Dave Townsend (:Mossop) from comment #3)
> What is the priority of getting a fix here? This is causing the Jetpack
> tests to fail and presumably causing problems for add-ons using the
> functionality we expose

I've got a patch. Just tweaking it a bit and I'll post it for review real soon now.
Comment 5 Bobby Holley (:bholley) (busy with Stylo) 2011-08-18 12:10:36 PDT
Created attachment 554167 [details] [diff] [review]
Bug 679494 - Wrap value returned from nsDOMMessageEvent::GetData. v1
Comment 6 Bobby Holley (:bholley) (busy with Stylo) 2011-08-18 12:10:40 PDT
Created attachment 554168 [details] [diff] [review]
Bug 679494 - Tests. v1
Comment 7 Bobby Holley (:bholley) (busy with Stylo) 2011-08-18 12:12:16 PDT
Comment on attachment 554168 [details] [diff] [review]
Bug 679494 - Tests. v1

Added a patch and tests. Flagging bkap for review.
Comment 8 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-18 12:18:46 PDT
Why do you need a browser chrome test here?  You should be able to do it with a chrome test with a content iframe, no?
Comment 9 Bobby Holley (:bholley) (busy with Stylo) 2011-08-18 12:24:48 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #8)
> Why do you need a browser chrome test here?  You should be able to do it
> with a chrome test with a content iframe, no?

Probably - I was just trying to match the test-case as much as possible. Would that be preferable? I'm not sure of the relative overhead.
Comment 10 Bobby Holley (:bholley) (busy with Stylo) 2011-08-18 13:36:16 PDT
Created attachment 554195 [details] [diff] [review]
Bug 679494 - Tests v2

> Would that be preferable? I'm not sure of the relative overhead.

I'm going to assume from comment 8 that it is. Here's an updated version. Flagging khuey for review.
Comment 11 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-18 13:52:10 PDT
Comment on attachment 554195 [details] [diff] [review]
Bug 679494 - Tests v2

Yeah, this is better.

Chrome tests are preferable to browser-chrome tests because they run in all Gecko things and not Firefox.  We generally avoid browser-chrome tests for anything except the browser UI.
Comment 12 Mozilla RelEng Bot 2011-08-18 22:20:47 PDT
Try run for e05ba95b7e9e is complete.
Detailed breakdown of the results available here:
    http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=e05ba95b7e9e
Results (out of 27 total builds):
    success: 25
    warnings: 2
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bobbyholley@gmail.com-e05ba95b7e9e
Comment 13 Bobby Holley (:bholley) (busy with Stylo) 2011-08-19 19:30:58 PDT
Landed on mozilla-inbound:

http://hg.mozilla.org/integration/mozilla-inbound/rev/70a6be6a9caa
http://hg.mozilla.org/integration/mozilla-inbound/rev/b1844f9cf777
Comment 15 :Ms2ger (⌚ UTC+1/+2) 2011-08-22 02:03:40 PDT
And this should have had a uuid bump, surely?
Comment 16 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-22 04:29:29 PDT
(In reply to Ms2ger from comment #15)
> And this should have had a uuid bump, surely?

It needs one, yes.
Comment 17 Bobby Holley (:bholley) (busy with Stylo) 2011-08-22 10:53:25 PDT
Created attachment 554905 [details] [diff] [review]
Bug 679494 - UUID rev followup. v1

D'oh!. Attaching a followup. Flagging khuey for review.
Comment 18 Bobby Holley (:bholley) (busy with Stylo) 2011-08-22 12:24:02 PDT
Landed on mozilla-inbound:

http://hg.mozilla.org/integration/mozilla-inbound/rev/98725bc76d7e
Comment 19 Mounir Lamouri (:mounir) 2011-08-23 01:30:25 PDT
http://hg.mozilla.org/mozilla-central/rev/98725bc76d7e
Comment 20 Blake Kaplan (:mrbkap) 2011-09-22 17:49:45 PDT
*** Bug 688364 has been marked as a duplicate of this bug. ***
Comment 21 christian 2011-10-03 14:41:21 PDT
Why was this nominated for beta? What is the risk vs reward? We're heavily leaning against taking this due to the uuid change.
Comment 22 Johnny Stenback (:jst, jst@mozilla.com) 2011-10-10 14:47:04 PDT
Comment on attachment 554905 [details] [diff] [review]
Bug 679494 - UUID rev followup. v1

We're going to wait another 6 weeks and get this in Firefox 9 since this is already in aurora (and the reasons that made me ask bholley to request approval here have since been invalidated, or at least lowered in weight).
Comment 23 Birunthan Mohanathas [:poiru] 2015-01-13 21:42:45 PST
The test incorrectly referenced file_679494.html instead of file_bug679494.html, fixed in:

https://hg.mozilla.org/integration/mozilla-inbound/rev/28a7a3744901
Comment 24 Carsten Book [:Tomcat] - PTO-back Sept 4th 2015-01-14 05:32:31 PST
https://hg.mozilla.org/mozilla-central/rev/28a7a3744901

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