Bug 824217 - Remove some easy-to-remove tests of JSRESOLVE_ASSIGNING. r=bz
authorJeff Walden <jwalden@mit.edu>
Wed, 19 Dec 2012 19:32:34 -0500
changeset 126431 0ce29ce2ea7ccfa33053eee3ff0a76bfa303542a
parent 126430 21d5fbda37b8ab018fa9aca9fe1b4c10446a0ba5
child 126432 1004c01bb8178868284c8bcb696154019010bb23
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs824217
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 824217 - Remove some easy-to-remove tests of JSRESOLVE_ASSIGNING. r=bz
browser/components/preferences/in-content/tests/privacypane_tests.js
browser/components/preferences/tests/privacypane_tests.js
dom/base/nsDOMClassInfo.cpp
dom/base/test/Makefile.in
dom/base/test/test_window_constructor.html
dom/workers/RuntimeService.cpp
dom/workers/test/Makefile.in
dom/workers/test/test_resolveWorker-assignment.html
dom/workers/test/test_resolveWorker.html
js/src/shell/js.cpp
js/xpconnect/shell/xpcshell.cpp
toolkit/components/places/tests/browser/browser_favicon_privatebrowsing.js
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@@ -4418,17 +4418,17 @@ nsDOMClassInfo::ResolveConstructor(JSCon
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMClassInfo::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                            JSObject *obj, jsid id, uint32_t flags,
                            JSObject **objp, bool *_retval)
 {
-  if (id == sConstructor_id && !(flags & JSRESOLVE_ASSIGNING)) {
+  if (id == sConstructor_id) {
     return ResolveConstructor(cx, obj, objp);
   }
 
   return NS_OK;
 }
 
 nsISupports*
 nsDOMTouchListSH::GetItemAt(nsISupports *aNative, uint32_t aIndex,
@@ -4902,18 +4902,18 @@ GetDocument(JSObject *obj)
 }
 
 // static
 JSBool
 nsWindowSH::GlobalScopePolluterNewResolve(JSContext *cx, JSHandleObject obj,
                                           JSHandleId id, unsigned flags,
                                           JSMutableHandleObject objp)
 {
-  if ((flags & JSRESOLVE_ASSIGNING) || !JSID_IS_STRING(id)) {
-    // Nothing to do if we're assigning or resolving a non-string property.
+  if (!JSID_IS_STRING(id)) {
+    // Nothing to do if we're resolving a non-string property.
     return JS_TRUE;
   }
 
   nsHTMLDocument *document = GetDocument(obj);
 
   if (!document) {
     // If we don't have a document, return early.
 
@@ -6727,23 +6727,21 @@ nsWindowSH::NewResolve(nsIXPConnectWrapp
     }
 
     if (did_resolve) {
       *objp = obj;
       return NS_OK;
     }
   }
 
-  if (!(flags & JSRESOLVE_ASSIGNING)) {
-    // We want this code to be before the child frame lookup code
-    // below so that a child frame named 'constructor' doesn't
-    // shadow the window's constructor property.
-    if (sConstructor_id == id) {
-      return ResolveConstructor(cx, obj, objp);
-    }
+  // We want this code to be before the child frame lookup code
+  // below so that a child frame named 'constructor' doesn't
+  // shadow the window's constructor property.
+  if (sConstructor_id == id) {
+    return ResolveConstructor(cx, obj, objp);
   }
 
   if (!my_context || !my_context->IsContextInitialized()) {
     // The context is not yet initialized so there's nothing we can do
     // here yet.
 
     return NS_OK;
   }
@@ -6778,17 +6776,17 @@ nsWindowSH::NewResolve(nsIXPConnectWrapp
       return NS_ERROR_FAILURE;
     }
 
     *objp = obj;
 
     return NS_OK;
   }
 
-  if (sTop_id == id && !(flags & JSRESOLVE_ASSIGNING)) {
+  if (sTop_id == id) {
     nsCOMPtr<nsIDOMWindow> top;
     rv = win->GetScriptableTop(getter_AddRefs(top));
     NS_ENSURE_SUCCESS(rv, rv);
 
     jsval v;
     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
     rv = WrapNative(cx, obj, top, &NS_GET_IID(nsIDOMWindow), true,
                     &v, getter_AddRefs(holder));
@@ -7189,17 +7187,17 @@ nsLocationSH::AddProperty(nsIXPConnectWr
 
 // DOM Navigator helper
 
 NS_IMETHODIMP
 nsNavigatorSH::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                           JSObject *obj, jsid id, uint32_t flags,
                           JSObject **objp, bool *_retval)
 {
-  if (!JSID_IS_STRING(id) || (flags & JSRESOLVE_ASSIGNING)) {
+  if (!JSID_IS_STRING(id)) {
     return NS_OK;
   }
 
   nsScriptNameSpaceManager *nameSpaceManager =
     nsJSRuntime::GetNameSpaceManager();
   NS_ENSURE_TRUE(nameSpaceManager, NS_ERROR_NOT_INITIALIZED);
 
   nsDependentJSString name(id);
@@ -8383,22 +8381,16 @@ nsHTMLDocumentSH::DocumentAllGetProperty
 
   return JS_TRUE;
 }
 
 JSBool
 nsHTMLDocumentSH::DocumentAllNewResolve(JSContext *cx, JSHandleObject obj, JSHandleId id,
                                         unsigned flags, JSMutableHandleObject objp)
 {
-  if (flags & JSRESOLVE_ASSIGNING) {
-    // Nothing to do here if we're assigning
-
-    return JS_TRUE;
-  }
-
   js::RootedValue v(cx);
 
   if (sItem_id == id || sNamedItem_id == id) {
     // Define the item() or namedItem() method.
 
     JSFunction *fnc = ::JS_DefineFunctionById(cx, obj, id, CallToGetPropMapper,
                                               0, JSPROP_ENUMERATE);
     objp.set(obj);
--- a/dom/base/test/Makefile.in
+++ b/dom/base/test/Makefile.in
@@ -14,16 +14,17 @@ MOCHITEST_FILES = \
   test_document.all_unqualified.html \
   test_domrequest.html \
   test_e4x_for_each.html \
   test_gsp-standards.html \
   test_gsp-quirks.html \
   test_gsp-qualified.html \
   test_nondomexception.html \
   test_screen_orientation.html \
+  test_window_constructor.html \
   test_window_enumeration.html \
   test_writable-replaceable.html \
   $(NULL)
 
 MOCHITEST_CHROME_FILES = \
    test_bug715041.xul \
    test_bug715041_removal.xul \
    $(NULL)
new file mode 100644
--- /dev/null
+++ b/dom/base/test/test_window_constructor.html
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=824217
+-->
+<head>
+  <meta charset="UTF-8">
+  <title>Test for Bug 824217</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=622491">Mozilla Bug 824217</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+</div>
+<iframe name="constructor" src="about:blank"></iframe>
+<pre id="test">
+<script type="application/javascript">
+SimpleTest.waitForExplicitFinish();
+
+function run()
+{
+  // Ideally we'd test that this property lives on the right place in the
+  // [[Prototype]] chain, with the right attributes there, but we don't
+  // implement this right yet, so just test the value's what's expected.
+  is(window.constructor, Window,
+     "should have gotten Window, not the constructor frame");
+  SimpleTest.finish();
+}
+
+window.addEventListener("load", run, false);
+</script>
+</pre>
+</body>
+</html>
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -504,22 +504,16 @@ BEGIN_WORKERS_NAMESPACE
 
 // Entry point for the DOM.
 JSBool
 ResolveWorkerClasses(JSContext* aCx, JSHandleObject aObj, JSHandleId aId, unsigned aFlags,
                      JSMutableHandleObject aObjp)
 {
   AssertIsOnMainThread();
 
-  // Don't care about assignments, bail now.
-  if (aFlags & JSRESOLVE_ASSIGNING) {
-    aObjp.set(nullptr);
-    return true;
-  }
-
   // Make sure our strings are interned.
   if (JSID_IS_VOID(gStringIDs[0])) {
     for (uint32_t i = 0; i < ID_COUNT; i++) {
       JSString* str = JS_InternString(aCx, gStringChars[i]);
       if (!str) {
         while (i) {
           gStringIDs[--i] = JSID_VOID;
         }
--- a/dom/workers/test/Makefile.in
+++ b/dom/workers/test/Makefile.in
@@ -36,16 +36,18 @@ MOCHITEST_FILES = \
   test_importScripts.html \
   importScripts_worker.js \
   importScripts_worker_imported1.js \
   importScripts_worker_imported2.js \
   importScripts_worker_imported3.js \
   importScripts_worker_imported4.js \
   test_instanceof.html \
   instanceof_worker.js \
+  test_resolveWorker.html \
+  test_resolveWorker-assignment.html \
   test_json.html \
   json_worker.js \
   test_location.html \
   location_worker.js \
   test_longThread.html \
   longThread_worker.js \
   test_navigator.html \
   navigator_worker.js \
new file mode 100644
--- /dev/null
+++ b/dom/workers/test/test_resolveWorker-assignment.html
@@ -0,0 +1,32 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<!DOCTYPE html>
+<html>
+  <head>
+    <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  </head>
+  <body>
+    <script type="application/javascript">
+      window.Worker = 17; // resolve through assignment
+
+      var desc = Object.getOwnPropertyDescriptor(window, "Worker");
+      ok(typeof desc === "object" && desc !== null, "Worker property must exist");
+
+      is(desc.value, 17, "Overwrite didn't work correctly");
+      // The JSRESOLVE_ASSIGNING flag-check around the "Resolve special classes"
+      // block in nsWindowSH::NewResolve needs to die to enable this.
+      todo_is(desc.enumerable, false,
+              "Initial descriptor was non-enumerable, and [[Put]] changes the " +
+              "property value but not its enumerability");
+      is(desc.configurable, true,
+         "Initial descriptor was configurable, and [[Put]] changes the " +
+         "property value but not its configurability");
+      is(desc.writable, true,
+         "Initial descriptor was writable, and [[Put]] changes the " +
+         "property value but not its writability");
+    </script>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/workers/test/test_resolveWorker.html
@@ -0,0 +1,31 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<!DOCTYPE html>
+<html>
+  <head>
+    <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  </head>
+  <body>
+    <script type="application/javascript">
+      window.Worker; // resolve not through assignment
+      Worker = 17;
+
+      var desc = Object.getOwnPropertyDescriptor(window, "Worker");
+      ok(typeof desc === "object" && desc !== null, "Worker property must exist");
+
+      is(desc.value, 17, "Overwrite didn't work correctly");
+      is(desc.enumerable, false,
+         "Initial descriptor was non-enumerable, and [[Put]] changes the " +
+         "property value but not its enumerability");
+      is(desc.configurable, true,
+         "Initial descriptor was configurable, and [[Put]] changes the " +
+         "property value but not its configurability");
+      is(desc.writable, true,
+         "Initial descriptor was writable, and [[Put]] changes the " +
+         "property value but not its writability");
+    </script>
+  </body>
+</html>
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -2524,17 +2524,17 @@ sandbox_resolve(JSContext *cx, HandleObj
 {
     jsval v;
     JSBool b, resolved;
 
     if (!JS_GetProperty(cx, obj, "lazy", &v))
         return false;
 
     JS_ValueToBoolean(cx, v, &b);
-    if (b && (flags & JSRESOLVE_ASSIGNING) == 0) {
+    if (b) {
         if (!JS_ResolveStandardClass(cx, obj, id, &resolved))
             return false;
         if (resolved) {
             objp.set(obj);
             return true;
         }
     }
     objp.set(NULL);
@@ -4578,19 +4578,16 @@ env_enumerate(JSContext *cx, HandleObjec
 
 static JSBool
 env_resolve(JSContext *cx, HandleObject obj, HandleId id, unsigned flags,
             MutableHandleObject objp)
 {
     JSString *valstr;
     const char *name, *value;
 
-    if (flags & JSRESOLVE_ASSIGNING)
-        return true;
-
     IdStringifier idstr(cx, id, true);
     if (idstr.threw())
         return false;
 
     name = idstr.getBytes();
     value = getenv(name);
     if (value) {
         valstr = JS_NewStringCopyZ(cx, value);
--- a/js/xpconnect/shell/xpcshell.cpp
+++ b/js/xpconnect/shell/xpcshell.cpp
@@ -940,19 +940,16 @@ env_enumerate(JSContext *cx, JSHandleObj
 }
 
 static JSBool
 env_resolve(JSContext *cx, JSHandleObject obj, JSHandleId id, unsigned flags,
             JSMutableHandleObject objp)
 {
     JSString *idstr, *valstr;
 
-    if (flags & JSRESOLVE_ASSIGNING)
-        return true;
-
     jsval idval;
     if (!JS_IdToValue(cx, id, &idval))
         return false;
 
     idstr = JS_ValueToString(cx, idval);
     if (!idstr)
         return false;
     JSAutoByteString name(cx, idstr);