973367 - Remove nsGlobalWindow::CreateOuterObject

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

There's only one caller of CreateOuterObject, so it's a pretty pointless abstraction.

Note:

* I removed the AutoPushJSContext, since there already is an nsCxPusher upstack;
* I reused the `thisChrome` local;
* the failure of NewOuterWindowProxy is now actually propagated.

I feel like we should be able to share some code between the if/else branches, but they're ever so slightly different. Also not sure what the JSAutoCompartment around SetWindowProxy is good for.

Comment 1

[Bobby Holley (:bholley)]

Comment on attachment 8376859 [details] [diff] [review]
Patch v1

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

Thanks for doing this. r=bholley with the fixes below.

::: dom/base/nsGlobalWindow.cpp
@@ +2416,5 @@
>  
>      if (!mJSObject) {
> +      JS::Rooted<JSObject*> global(cx, newInnerWindow->FastGetGlobalJSObject());
> +      JS::Rooted<JSObject*> outer(cx, NewOuterWindowProxy(cx, global, thisChrome));
> +      if (!outer) {

NS_WARN_IF?

@@ +2423,5 @@
> +
> +      js::SetProxyExtra(outer, 0, js::PrivateValue(ToSupports(this)));
> +
> +      {
> +        JSAutoCompartment ac(cx, outer);

Yeah this |ac| can go.

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a54f5ae6956d

Comment 3

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

https://hg.mozilla.org/mozilla-central/rev/a54f5ae6956d