Bug 639720. Get rid of the Window class setter so that SETNAME on the global is faster in the browser. r=mrbkap
☠☠ backed out by dc27139edda0 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 31 Aug 2011 18:10:16 -0400
changeset 76415 ee8c8daffe4355f2024d6cc1b17771b480f0e7d8
parent 76414 7ee51f8ea94042c4b5748765539b5dbaae23693d
child 76416 6a70000360a09104de741a7d3c93374f6652b665
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersmrbkap
bugs639720
milestone9.0a1
Bug 639720. Get rid of the Window class setter so that SETNAME on the global is faster in the browser. r=mrbkap
dom/base/nsDOMClassInfo.cpp
dom/base/nsDOMClassInfo.h
dom/tests/mochitest/dom-level0/Makefile.in
dom/tests/mochitest/dom-level0/test_location_setters.html
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@@ -43,16 +43,17 @@
 #include "jsprvtd.h"    // we are using private JS typedefs...
 #include "jscntxt.h"
 #include "jsobj.h"
 #include "jsdbgapi.h"
 #include "WrapperFactory.h"
 #include "AccessCheck.h"
 
 #include "xpcprivate.h"
+#include "XPCWrapper.h"
 
 #include "nscore.h"
 #include "nsDOMClassInfo.h"
 #include "nsCRT.h"
 #include "nsCRTGlue.h"
 #include "nsIServiceManager.h"
 #include "nsICategoryManager.h"
 #include "nsIComponentRegistrar.h"
@@ -511,17 +512,16 @@ static NS_DEFINE_CID(kDOMSOF_CID, NS_DOM
 static const char kDOMStringBundleURL[] =
   "chrome://global/locale/dom/dom.properties";
 
 // NOTE: DEFAULT_SCRIPTABLE_FLAGS and DOM_DEFAULT_SCRIPTABLE_FLAGS
 //       are defined in nsIDOMClassInfo.h.
 
 #define WINDOW_SCRIPTABLE_FLAGS                                               \
  (nsIXPCScriptable::WANT_GETPROPERTY |                                        \
-  nsIXPCScriptable::WANT_SETPROPERTY |                                        \
   nsIXPCScriptable::WANT_PRECREATE |                                          \
   nsIXPCScriptable::WANT_FINALIZE |                                           \
   nsIXPCScriptable::WANT_EQUALITY |                                           \
   nsIXPCScriptable::WANT_ENUMERATE |                                          \
   nsIXPCScriptable::DONT_ENUM_QUERY_INTERFACE |                               \
   nsIXPCScriptable::WANT_OUTER_OBJECT)
 
 #define NODE_SCRIPTABLE_FLAGS                                                 \
@@ -5265,49 +5265,16 @@ nsWindowSH::GetProperty(nsIXPConnectWrap
     *vp = OBJECT_TO_JSVAL(obj);
     return NS_SUCCESS_I_DID_SOMETHING;
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsWindowSH::SetProperty(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
-                        JSObject *obj, jsid id, jsval *vp, PRBool *_retval)
-{
-  if (id == sLocation_id) {
-    JSAutoRequest ar(cx);
-
-    JSString *val = ::JS_ValueToString(cx, *vp);
-    NS_ENSURE_TRUE(val, NS_ERROR_UNEXPECTED);
-
-    nsCOMPtr<nsIDOMWindow> window = do_QueryWrappedNative(wrapper);
-    NS_ENSURE_TRUE(window, NS_ERROR_UNEXPECTED);
-
-    nsCOMPtr<nsIDOMLocation> location;
-    nsresult rv = window->GetLocation(getter_AddRefs(location));
-    NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && location, rv);
-
-    nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-    rv = WrapNative(cx, obj, location, &NS_GET_IID(nsIDOMLocation), PR_TRUE,
-                    vp, getter_AddRefs(holder));
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    nsDependentJSString depStr;
-    NS_ENSURE_TRUE(depStr.init(cx, val), NS_ERROR_UNEXPECTED);
-
-    rv = location->SetHref(depStr);
-
-    return NS_FAILED(rv) ? rv : NS_SUCCESS_I_DID_SOMETHING;
-  }
-
-  return NS_OK;
-}
-
-NS_IMETHODIMP
 nsWindowSH::Enumerate(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                       JSObject *obj, PRBool *_retval)
 {
   if (!ObjectIsNativeWrapper(cx, obj)) {
     *_retval = JS_EnumerateStandardClasses(cx, obj);
   }
 
   return NS_OK;
@@ -6387,16 +6354,78 @@ ContentWindowGetter(JSContext *cx, uintN
 
   return ::JS_GetProperty(cx, obj, "content", vp);
 }
 
 static JSNewResolveOp sOtherResolveFuncs[] = {
   mozilla::dom::workers::ResolveWorkerClasses
 };
 
+template<class Interface>
+static nsresult
+LocationSetterGuts(JSContext *cx, JSObject *obj, jsval *vp)
+{
+  // This function duplicates some of the logic in XPC_WN_HelperSetProperty
+  XPCWrappedNative *wrapper =
+    XPCWrappedNative::GetWrappedNativeOfJSObject(cx, obj);
+
+  // The error checks duplicate code in THROW_AND_RETURN_IF_BAD_WRAPPER
+  NS_ENSURE_TRUE(wrapper, NS_ERROR_XPC_BAD_OP_ON_WN_PROTO);
+  NS_ENSURE_TRUE(wrapper->IsValid(), NS_ERROR_XPC_HAS_BEEN_SHUTDOWN);
+
+  nsCOMPtr<Interface> xpcomObj = do_QueryWrappedNative(wrapper);
+  NS_ENSURE_TRUE(xpcomObj, NS_ERROR_UNEXPECTED);
+
+  nsCOMPtr<nsIDOMLocation> location;
+  nsresult rv = xpcomObj->GetLocation(getter_AddRefs(location));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  JSString *val = ::JS_ValueToString(cx, *vp);
+  NS_ENSURE_TRUE(val, NS_ERROR_UNEXPECTED);
+
+  nsDependentJSString depStr;
+  NS_ENSURE_TRUE(depStr.init(cx, val), NS_ERROR_UNEXPECTED);
+  
+  rv = location->SetHref(depStr);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
+  return WrapNative(cx, JS_GetGlobalForScopeChain(cx), location,
+                    &NS_GET_IID(nsIDOMLocation), PR_TRUE, vp,
+                    getter_AddRefs(holder));
+}
+
+template<class Interface>
+static JSBool
+LocationSetter(JSContext *cx, JSObject *obj, jsid id, JSBool strict,
+               jsval *vp)
+{
+  nsresult rv = LocationSetterGuts<Interface>(cx, obj, vp);
+  if (NS_FAILED(rv)) {
+    if (!::JS_IsExceptionPending(cx)) {
+      nsDOMClassInfo::ThrowJSException(cx, rv);
+    }
+    return JS_FALSE;
+  }
+
+  return JS_TRUE;
+}
+
+static JSBool
+LocationSetterUnwrapper(JSContext *cx, JSObject *obj, jsid id, JSBool strict,
+                        jsval *vp)
+{
+  JSObject *wrapped = XPCWrapper::UnsafeUnwrapSecurityWrapper(obj);
+  if (wrapped) {
+    obj = wrapped;
+  }
+
+  return LocationSetter<nsIDOMWindow>(cx, obj, id, strict, vp);
+}
+
 NS_IMETHODIMP
 nsWindowSH::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                        JSObject *obj, jsid id, PRUint32 flags,
                        JSObject **objp, PRBool *_retval)
 {
   nsGlobalWindow *win = nsGlobalWindow::FromWrapper(wrapper);
 
   if (!JSID_IS_STRING(id)) {
@@ -6539,17 +6568,18 @@ nsWindowSH::NewResolve(nsIXPConnectWrapp
 
     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
     jsval v;
     rv = WrapNative(cx, scope, location, &NS_GET_IID(nsIDOMLocation), PR_TRUE,
                     &v, getter_AddRefs(holder));
     NS_ENSURE_SUCCESS(rv, rv);
 
     JSBool ok = JS_WrapValue(cx, &v) &&
-                JS_DefinePropertyById(cx, obj, id, v, nsnull, nsnull,
+                JS_DefinePropertyById(cx, obj, id, v, nsnull,
+                                      LocationSetterUnwrapper,
                                       JSPROP_PERMANENT | JSPROP_ENUMERATE);
 
     if (!ok) {
       return NS_ERROR_FAILURE;
     }
 
     *objp = obj;
 
@@ -8172,17 +8202,18 @@ nsDocumentSH::NewResolve(nsIXPConnectWra
     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
     rv = WrapNative(cx, JS_GetGlobalForScopeChain(cx), location,
                     &NS_GET_IID(nsIDOMLocation), PR_TRUE, &v,
                     getter_AddRefs(holder));
     NS_ENSURE_SUCCESS(rv, rv);
 
     JSAutoRequest ar(cx);
 
-    JSBool ok = ::JS_DefinePropertyById(cx, obj, id, v, nsnull, nsnull,
+    JSBool ok = ::JS_DefinePropertyById(cx, obj, id, v, nsnull,
+                                        LocationSetter<nsIDOMDocument>,
                                         JSPROP_PERMANENT | JSPROP_ENUMERATE);
 
     if (!ok) {
       return NS_ERROR_FAILURE;
     }
 
     *objp = obj;
 
@@ -8217,44 +8248,16 @@ nsDocumentSH::GetProperty(nsIXPConnectWr
 
   return nsNodeSH::GetProperty(wrapper, cx, obj, id, vp, _retval);
 }
 
 NS_IMETHODIMP
 nsDocumentSH::SetProperty(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                           JSObject *obj, jsid id, jsval *vp, PRBool *_retval)
 {
-  if (id == sLocation_id) {
-    nsCOMPtr<nsIDOMDocument> doc = do_QueryWrappedNative(wrapper);
-    NS_ENSURE_TRUE(doc, NS_ERROR_UNEXPECTED);
-
-    nsCOMPtr<nsIDOMLocation> location;
-    nsresult rv = doc->GetLocation(getter_AddRefs(location));
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    if (location) {
-      JSAutoRequest ar(cx);
-
-      JSString *val = ::JS_ValueToString(cx, *vp);
-      NS_ENSURE_TRUE(val, NS_ERROR_UNEXPECTED);
-
-      nsDependentJSString depStr;
-      NS_ENSURE_TRUE(depStr.init(cx, val), NS_ERROR_UNEXPECTED);
-
-      rv = location->SetHref(depStr);
-      NS_ENSURE_SUCCESS(rv, rv);
-
-      nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-      rv = WrapNative(cx, JS_GetGlobalForScopeChain(cx), location,
-                      &NS_GET_IID(nsIDOMLocation), PR_TRUE, vp,
-                      getter_AddRefs(holder));
-      return NS_FAILED(rv) ? rv : NS_SUCCESS_I_DID_SOMETHING;
-    }
-  }
-
   if (id == sDocumentURIObject_id && IsPrivilegedScript()) {
     // We don't want privileged script that can read this property to set it,
     // but _do_ want to allow everyone else to set a value they can then read.
     //
     // XXXbz Is there a better error we could use here?
     return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
   }
   
--- a/dom/base/nsDOMClassInfo.h
+++ b/dom/base/nsDOMClassInfo.h
@@ -402,18 +402,16 @@ public:
       *aFlags = flags | nsIXPCScriptable::WANT_POSTCREATE;
     }
 
     return rv;
   }
 #endif
   NS_IMETHOD GetProperty(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                          JSObject *obj, jsid id, jsval *vp, PRBool *_retval);
-  NS_IMETHOD SetProperty(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
-                         JSObject *obj, jsid id, jsval *vp, PRBool *_retval);
   NS_IMETHOD Enumerate(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                        JSObject *obj, PRBool *_retval);
   NS_IMETHOD NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                         JSObject *obj, jsid id, PRUint32 flags,
                         JSObject **objp, PRBool *_retval);
   NS_IMETHOD Finalize(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                       JSObject *obj);
   NS_IMETHOD Equality(nsIXPConnectWrappedNative *wrapper, JSContext * cx,
--- a/dom/tests/mochitest/dom-level0/Makefile.in
+++ b/dom/tests/mochitest/dom-level0/Makefile.in
@@ -50,12 +50,13 @@ include $(topsrcdir)/config/rules.mk
 		test_setting_document.domain_to_shortened_ipaddr.html \
 		child_ip_address.html \
 		test_setting_document.domain_idn.html \
 		idn_child.html \
 		file_location.html \
 		test_location.html \
 		test_innerWidthHeight_script.html \
 		innerWidthHeight_script.html \
+		test_location_setters.html \
 		$(NULL)
 
 libs:: 	$(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/dom-level0/test_location_setters.html
@@ -0,0 +1,78 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=639720
+-->
+<head>
+  <title>Test for Bug 639720</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=639720">Mozilla Bug 639720</a>
+<p id="display">
+  <iframe id="f"></iframe>
+</p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 639720 **/
+SimpleTest.waitForExplicitFinish();
+
+var tests = [
+ { url: "data:text/plain,1" },
+ { url: "data:text/plain,2",
+   useDocument: true },
+ { prepURL: "http://www.example.com",
+   url: "data:text/plain,3" }
+];
+
+var currentTest = 0;
+
+function checkTest() {
+  is($("f").contentWindow.location.href, tests[currentTest].url,
+     "href of content window's location should match url of current test");
+  ++currentTest;
+  runNextTest();
+}
+
+function runCurrentTest() {
+  var test = tests[currentTest];
+  $("f").onload = checkTest;
+  if (test.useDocument) {
+    $("f").contentDocument.location = test.url;
+  } else {
+    $("f").contentWindow.location = test.url;
+  }
+  is(typeof($("f").contentWindow.location), "object",
+     "Location should not have become string");
+}
+
+function prepComplete() {
+  runCurrentTest();
+}
+
+function runNextTest() {
+  if (currentTest == tests.length) {
+    SimpleTest.finish();
+    return;
+  }
+
+  var test = tests[currentTest];
+  if ("prepURL" in test) {
+    $("f").onload = prepComplete;
+    $("f").src = test.prepURL;
+    return;
+  }
+
+  runCurrentTest();
+}
+
+addLoadEvent(runNextTest);
+</script>
+</pre>
+</body>
+</html>