861461 - Greasemonkey 1.8 stops working in Nightly23.0a1

Status: VERIFIED FIXED

(Core :: XPConnect, defect)

Description

[Alice0775 White]

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/7b8ed29c6bc0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130412 Firefox/23.0 ID:20130412030828

GreaseMonkey1.8 stops working in Nightly23.0a1

Error appears in Error Console as follows:

Error: NS_ERROR_UNEXPECTED: Unexpected error
Source file: jar:file:///L:/MozillaFirefox/Profiles/y2kimfii.Fx18/extensions/%7Be4a8a97b-f2ed-450b-b12d-ee082ba24781%7D.xpi!/components/greasemonkey.js
Line: 111

The error pointed:
    var imp = sandbox.importFunction;
    if (GM_util.inArray(aScript.grants, 'GM_addStyle')) {
      imp(function(css) { GM_addStyle(aContentWin.document, css); },
>>        'GM_addStyle');
    }


Alternate Steps To Reproduce:
  1. Evaluate the following code in Scratchpad with chrome privilege(i.e, Browser Enviroment)

//-------------
  function GGGG(aDocument, aCss) {}
  var sandbox = new Components.utils.Sandbox(
      content,
      {
        'sandboxName': "aaa",
        'sandboxPrototype': content,
        'wantXrays': true,
      });
  var imp = sandbox.importFunction;
  imp(function(css) { GGGG(content.document, css); }, 'GGGG');
//------------

Actual Results:
	/*
	Exception: Unexpected error
	@19
	*/

Expected Results:
  No error


Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/b9d56a1e0a61
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130411 Firefox/23.0 ID:20130411081408
Bad:
http://hg.mozilla.org/mozilla-central/rev/7b8ed29c6bc0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130411 Firefox/23.0 ID:20130411122104
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b9d56a1e0a61&tochange=7b8ed29c6bc0


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/5e1117b3ad7f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130410 Firefox/23.0 ID:20130410111302
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/a28728d22f22
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130410 Firefox/23.0 ID:20130410124503
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5e1117b3ad7f&tochange=a28728d22f22

In local build
Last Good: 8e5a1410e6fe-9b9a5c42fb46
First Bad: b018c2f116e4-9b9a5c42fb46
Triggered by:
  b018c2f116e4	Tom Schuster — Bug 856477 - Root XPComponents. r=bholley f=terrence

Comment 1

[Bobby Holley (:bholley)]

Assigning to evilpies - my guess would be that the patch broke ImportFunction in some subtle way. That function is actually just a pass-through (obsoleted by compartments) that defines the function on the sandbox. But people still use it, so we have to make sure it works.

Comment 2

[Anthony Lieuallen]

Greasemonkey separately received a pull request for this just a few hours ago:
https://github.com/greasemonkey/greasemonkey/pull/1733

Sounds like I should accept it even if this is un-broken, because it's unnecessary?

Comment 3

[Bobby Holley (:bholley)]

(In reply to Anthony Lieuallen from comment #2)
> Greasemonkey separately received a pull request for this just a few hours
> ago:
> https://github.com/greasemonkey/greasemonkey/pull/1733
> 
> Sounds like I should accept it even if this is un-broken, because it's
> unnecessary?

Yeah, please do! It'd be great to kill importFunction someday.

Comment 4

[Tom S [:evilpie]]

I have a hard time understand how this was supposed to work correctly previously. I identified the cause of this error, but I believe this actually caught wrong behavior. From a pure JS-point of view, I believe just applying importFunction without an explicit base like |sandbox|, would previously import that functions in the global scope and not into the sandbox? Why did that even work?

Comment 5

[Bobby Holley (:bholley)]

(In reply to Tom Schuster [:evilpie] from comment #4)
> I have a hard time understand how this was supposed to work correctly
> previously. I identified the cause of this error, but I believe this
> actually caught wrong behavior. From a pure JS-point of view, I believe just
> applying importFunction without an explicit base like |sandbox|, would
> previously import that functions in the global scope and not into the
> sandbox? Why did that even work?

Probably because JS_THIS_OBJECT boxes a non-strict |this|, whereas args.thisv() doesn't.

Comment 6

[Tom S [:evilpie]]

But wouldn't you want to import the function into the sandbox and not in your own global? Or is the sandbox for some reason linked to that? (I obviously don't know anything about how this stuff is set-up)

Comment 7

[Tom S [:evilpie]]

Attachment: v1

Try push: https://tbpl.mozilla.org/?tree=Try&rev=991c84b72fb7

Comment 8

[Bobby Holley (:bholley)]

Presumably you figured out the issue in comment 6 - can you explain that here?

Comment 9

[Tom S [:evilpie]]

I think the main reason is that, importFunction is actually still a CrossCompartmentProxy even after we got it of the sandbox. So when we call that function we enter that compartment, and during the this resolving we use that compartment's global. I didn't verify that to 100%, but it sounds kind of sane.

Comment 10

[Bobby Holley (:bholley)]

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

Review of attachment 737125 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/src/XPCComponents.cpp
@@ +2969,5 @@
>      RootedId id(cx);
>      if (!JS_ValueToId(cx, StringValue(funname), id.address()))
>          return false;
>  
> +    RootedObject thisObject(cx, JS_THIS_OBJECT(cx, vp));

Please add a comment that we're depending on the boxing here.

Comment 11

[Tom S [:evilpie]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7143c88f59dc

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7143c88f59dc

Comment 13

[Ioana (away)]

I tried to reproduce this bug on the 05/01 Nightly (Win7 x64) in order to verify it, but I can't reproduce it with either of the steps in comment 0 with Greasemonkey 1.10.

If anyone can reproduce this bug, please also try to verify it.

Comment 14

[silverwind]

@ioana: That's because it got fixed on Greasemonkey's side in 1.9. See https://github.com/greasemonkey/greasemonkey/pull/1733

Comment 15

[Ioana (away)]

(In reply to silverwind from comment #14)
> @ioana: That's because it got fixed on Greasemonkey's side in 1.9. See
> https://github.com/greasemonkey/greasemonkey/pull/1733

I actually couldn't reproduce the issue with GM 1.8 either (04/13 Nightly). I tried again and I managed to reproduce it eventually with the alternate steps. Verified it as fixed on Firefox 26b1 and the 11/03 Nightly.