From c617f3e3b94d6c1570c692709f897aaabfe78d2a Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 9 Dec 2014 14:44:37 -0500 Subject: [PATCH] Bug 1012798 part 2. Stop defining a value property named "window" on the global. r=peterv --- dom/base/nsGlobalWindow.cpp | 25 +++++++++++-------- dom/base/nsGlobalWindow.h | 2 +- .../test/test_window_cross_origin_props.html | 7 +++--- dom/bindings/Codegen.py | 6 +++++ dom/webidl/Window.webidl | 4 +-- .../window-properties.html.ini | 4 --- .../meta/html/dom/interfaces.html.ini | 3 --- 7 files changed, 26 insertions(+), 25 deletions(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index d5e59ad6ea2b..f30a2743fee4 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -2612,11 +2612,14 @@ nsGlobalWindow::SetNewDocument(nsIDocument* aDocument, xpc::Scriptability::Get(GetWrapperPreserveColor()).SetDocShellAllowsScript(allow); if (!aState) { - JS::Rooted rootedWrapper(cx, GetWrapperPreserveColor()); - if (!JS_DefineProperty(cx, newInnerGlobal, "window", rootedWrapper, - JSPROP_ENUMERATE | JSPROP_READONLY | - JSPROP_PERMANENT, - JS_STUBGETTER, JS_STUBSETTER)) { + // Get the "window" property once so it will be cached on our inner. We + // have to do this here, not in binding code, because this has to happen + // after we've created the outer window proxy and stashed it in the outer + // nsGlobalWindow, so GetWrapperPreserveColor() on that outer + // nsGlobalWindow doesn't return null and nsGlobalWindow::OuterObject + // works correctly. + JS::Rooted unused(cx); + if (!JS_GetProperty(cx, newInnerGlobal, "window", &unused)) { NS_ERROR("can't create the 'window' property"); return NS_ERROR_FAILURE; } @@ -3509,11 +3512,9 @@ nsGlobalWindow::GetDocument(nsIDOMDocument** aDocument) return NS_OK; } -nsIDOMWindow* -nsGlobalWindow::GetWindow(ErrorResult& aError) +nsGlobalWindow* +nsGlobalWindow::Window() { - FORWARD_TO_OUTER_OR_THROW(GetWindow, (aError), aError, nullptr); - return this; } @@ -3521,10 +3522,12 @@ NS_IMETHODIMP nsGlobalWindow::GetWindow(nsIDOMWindow** aWindow) { ErrorResult rv; - nsCOMPtr window = GetWindow(rv); + FORWARD_TO_OUTER_OR_THROW(GetWindow, (aWindow), rv, rv.ErrorCode()); + + nsCOMPtr window = Window(); window.forget(aWindow); - return rv.ErrorCode(); + return NS_OK; } nsIDOMWindow* diff --git a/dom/base/nsGlobalWindow.h b/dom/base/nsGlobalWindow.h index 2c83f8b43c65..20c245c071ce 100644 --- a/dom/base/nsGlobalWindow.h +++ b/dom/base/nsGlobalWindow.h @@ -799,7 +799,7 @@ public: static JSObject* CreateNamedPropertiesObject(JSContext *aCx, JS::Handle aProto); - nsIDOMWindow* GetWindow(mozilla::ErrorResult& aError); + nsGlobalWindow* Window(); nsIDOMWindow* GetSelf(mozilla::ErrorResult& aError); nsIDocument* GetDocument() { diff --git a/dom/base/test/test_window_cross_origin_props.html b/dom/base/test/test_window_cross_origin_props.html index 05ef109abbc4..d16a103fb681 100644 --- a/dom/base/test/test_window_cross_origin_props.html +++ b/dom/base/test/test_window_cross_origin_props.html @@ -75,10 +75,9 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=946067 "Our subframe's top should still be our top"); is(frames[0].window, frames[0], "window getter should work"); - // "window" is not a getter property yet - //is(doGet("window", frames[0]), frames[0], "window getter should still work"); - todo_isnot(Object.getOwnPropertyDescriptor(window, "window").get, undefined, - "Should have a getter here"); + is(doGet("window", frames[0]), frames[0], "window getter should still work"); + isnot(Object.getOwnPropertyDescriptor(window, "window").get, undefined, + "Should have a getter here"); // Finally, check that we can set the location frames[0].location = "about:blank"; diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 3c3553cc3280..3dc6832a50a2 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -3427,6 +3427,12 @@ class CGUpdateMemberSlotsMethod(CGAbstractStaticMethod): "JSJitGetterCallArgs args(&temp);\n") for m in self.descriptor.interface.members: if m.isAttr() and m.getExtendedAttribute("StoreInSlot"): + # Skip doing this for the "window" attribute on the Window + # interface, because that can't be gotten safely until we have + # hooked it up correctly to the outer window. + if (self.descriptor.interface.identifier.name == "Window" and + m.identifier.name == "window"): + continue body += fill( """ diff --git a/dom/webidl/Window.webidl b/dom/webidl/Window.webidl index b5c1f9976b9f..64c1881fe54a 100644 --- a/dom/webidl/Window.webidl +++ b/dom/webidl/Window.webidl @@ -27,8 +27,8 @@ typedef any Transferable; [PrimaryGlobal, NeedResolve] /*sealed*/ interface Window : EventTarget { // the current browsing context - [Unforgeable, Throws, - CrossOriginReadable] readonly attribute WindowProxy window; + [Unforgeable, Constant, StoreInSlot, + CrossOriginReadable] readonly attribute Window window; [Replaceable, Throws, CrossOriginReadable] readonly attribute WindowProxy self; [Unforgeable, StoreInSlot, Pure] readonly attribute Document? document; diff --git a/testing/web-platform/meta/html/browsers/the-window-object/window-properties.html.ini b/testing/web-platform/meta/html/browsers/the-window-object/window-properties.html.ini index 25f15e9d7ddc..86a969f18f4a 100644 --- a/testing/web-platform/meta/html/browsers/the-window-object/window-properties.html.ini +++ b/testing/web-platform/meta/html/browsers/the-window-object/window-properties.html.ini @@ -53,7 +53,3 @@ [Window attribute: onstorage] expected: FAIL - - [Window unforgeable attribute: window] - expected: FAIL - diff --git a/testing/web-platform/meta/html/dom/interfaces.html.ini b/testing/web-platform/meta/html/dom/interfaces.html.ini index 85be110382fb..f6bbeb02f90b 100644 --- a/testing/web-platform/meta/html/dom/interfaces.html.ini +++ b/testing/web-platform/meta/html/dom/interfaces.html.ini @@ -2886,9 +2886,6 @@ [Window interface: attribute localStorage] expected: FAIL - [Window interface: window must have own property "window"] - expected: FAIL - [Window interface: window must inherit property "self" with the proper type (1)] expected: FAIL