1065452 - Redirect errors that reach addon scope globals to their appropriate DOM window (if any)

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Bobby Holley (:bholley)]

This is causing orange when we try to flip the switch in bug 1030420. Thanks to the recent refactoring work I've done, this fix should be pretty straightforward, if a bit hacky. The hackiness will go away in bug 981187.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Re-route exceptions directed at addon globals to their associated DOMWindow. v1

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8487274 [details] [diff] [review]
Re-route exceptions directed at addon globals to their associated DOMWindow. v1

>+  // Note that they way we do this right now is sloppy. Error reporters can

s/they/the/

>+    if (!JS_GetPrototype(cx, global, &proto)) {

Could we have this "get window from addon global" code somewhere nearer the code that sets up the relationship?

r=me modulo that.

Comment 3

[Bobby Holley (:bholley)]

Attachment: Part 1 - Add an API to access the associated window of addon scopes. v1

Comment 4

[Bobby Holley (:bholley)]

Attachment: Part 2 - Re-route exceptions directed at addon globals to their associated DOMWindow. v2 r=bz

Comment 5

[Bobby Holley (:bholley)]

Comment on attachment 8488022 [details] [diff] [review]
Part 1 - Add an API to access the associated window of addon scopes. v1

Valgrind found something wrong with this. Investigating.

Comment 6

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=aed7e9abe325

Comment 7

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=7a859fc266c0

Comment 8

[Bobby Holley (:bholley)]

Attachment: Part 1 - Add an API to access the associated window of addon scopes. v3

Comment 9

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8489122 [details] [diff] [review]
Part 1 - Add an API to access the associated window of addon scopes. v3

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

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +588,5 @@
> +    // we pretty much just want to crash if that happens so that we find out
> +    // about it and get them to change their code.
> +    MOZ_RELEASE_ASSERT(js::IsCrossCompartmentWrapper(proto));
> +    JSObject *mainGlobal = js::UncheckedUnwrap(proto, /* stopAtOuter = */ false);
> +    MOZ_RELEASE_ASSERT(JS_IsGlobalObject(mainGlobal));

I guess it's too much trouble to iterate over mAddonScopes. That seems fine to me.

Perhaps eventually we could change mAddonScopes to a lazily created hashtable rather than a list.

Comment 10

[Bobby Holley (:bholley)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e32c121d2bb3
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e934ae44e2a7

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e32c121d2bb3
https://hg.mozilla.org/mozilla-central/rev/e934ae44e2a7