Remove nsGlobalWindow::CreateOuterObject

RESOLVED FIXED in mozilla31

Status

()

defect
RESOLVED FIXED
6 years ago
5 months ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

Trunk
mozilla31
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Posted patch Patch v1Splinter Review
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.
Attachment #8376859 - Flags: review?(bobbyholley)
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.
Attachment #8376859 - Flags: review?(bobbyholley) → review+
https://hg.mozilla.org/mozilla-central/rev/a54f5ae6956d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.