Backed out changesets 1cf7b5810eb5, 4cb373d9ce33, and 6037db66624b (bug 1042436) for bustage.
authorRyan VanderMeulen <ryanvm@gmail.com>
Fri, 08 Aug 2014 15:22:17 -0400
changeset 208276 53c7aceaf1a8
parent 208275 1cf7b5810eb5
child 208277 08c5b02a125e
push id3797
push userryanvm@gmail.com
push date2014-08-08 19:22 +0000
treeherdermozilla-beta@53c7aceaf1a8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1042436
milestone32.0
backs out1cf7b5810eb5
4cb373d9ce33
6037db66624b
Backed out changesets 1cf7b5810eb5, 4cb373d9ce33, and 6037db66624b (bug 1042436) for bustage.
js/xpconnect/src/xpcprivate.h
js/xpconnect/tests/chrome/chrome.ini
js/xpconnect/tests/chrome/test_bug1042436.xul
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -3469,17 +3469,16 @@ public:
         LocationHintAddon
     };
 
     CompartmentPrivate(JSCompartment *c)
         : wantXrays(false)
         , universalXPConnectEnabled(false)
         , adoptedNode(false)
         , donatedNode(false)
-        , warnedAboutXrays(false)
         , scriptability(c)
         , scope(nullptr)
     {
         MOZ_COUNT_CTOR(xpc::CompartmentPrivate);
     }
 
     ~CompartmentPrivate();
 
@@ -3490,20 +3489,16 @@ public:
     // enablePrivilege. Once set, this value is never unset (i.e., it doesn't follow
     // the old scoping rules of enablePrivilege). Using it is inherently unsafe.
     bool universalXPConnectEnabled;
 
     // for telemetry. See bug 928476.
     bool adoptedNode;
     bool donatedNode;
 
-    // Whether we've emitted a warning about a property that was filtered out
-    // by XrayWrappers. See XrayWrapper.cpp.
-    bool warnedAboutXrays;
-
     // The scriptability of this compartment.
     Scriptability scriptability;
 
     // Our XPCWrappedNativeScope. This is non-null if and only if this is an
     // XPConnect compartment.
     XPCWrappedNativeScope *scope;
 
     const nsACString& GetLocation() {
--- a/js/xpconnect/tests/chrome/chrome.ini
+++ b/js/xpconnect/tests/chrome/chrome.ini
@@ -50,17 +50,16 @@ support-files =
 [test_bug853283.xul]
 [test_bug853571.xul]
 [test_bug858101.xul]
 [test_bug860494.xul]
 [test_bug866823.xul]
 [test_bug895340.xul]
 [test_bug932906.xul]
 [test_bug996069.xul]
-[test_bug1042436.xul]
 [test_xrayToJS.xul]
 [test_chrometoSource.xul]
 [test_cloneInto.xul]
 [test_cows.xul]
 [test_discardSystemSource.xul]
 [test_documentdomain.xul]
 [test_doublewrappedcompartments.xul]
 [test_evalInSandbox.xul]
deleted file mode 100644
--- a/js/xpconnect/tests/chrome/test_bug1042436.xul
+++ /dev/null
@@ -1,49 +0,0 @@
-<?xml version="1.0"?>
-<?xml-stylesheet type="text/css" href="chrome://global/skin"?>
-<?xml-stylesheet type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"?>
-<!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=1042436
--->
-<window title="Mozilla Bug 1042436"
-        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
-  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
-
-  <!-- test results are displayed in the html:body -->
-  <body xmlns="http://www.w3.org/1999/xhtml">
-  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1042436"
-     target="_blank">Mozilla Bug 1042436</a>
-  </body>
-
-  <!-- test code goes here -->
-  <script type="application/javascript">
-  <![CDATA[
-  /** Test for Bug 1042436 **/
-  SimpleTest.waitForExplicitFinish();
-  const Cu = Components.utils;
-
-  var contentSb = Cu.Sandbox('http://www.example.com');
-  var nonXrayableObj = new contentSb.WeakMap();
-  nonXrayableObj.wrappedJSObject.someExpandoProperty = 42;
-  nonXrayableObj.wrappedJSObject.someOtherExpandoProperty = 52;
-
-
-  SimpleTest.expectConsoleMessages(function() {
-      nonXrayableObj.someExpandoProperty;
-      nonXrayableObj.someOtherExpandoProperty;
-
-      var chromeSb = Cu.Sandbox(this);
-      var contentObjWithGetter = contentSb.eval('({ get getterProp() {return 42;}, valueProp: 42 })');
-      is(contentObjWithGetter.wrappedJSObject.getterProp, 42, "Getter prop set up correctly");
-      is(contentObjWithGetter.getterProp, undefined, "Xrays work right");
-      is(contentObjWithGetter.valueProp, 42, "Getter prop set up correctly");
-      chromeSb.contentObjWithGetter = contentObjWithGetter;
-      Cu.evalInSandbox('contentObjWithGetter.getterProp; contentObjWithGetter.valueProp; contentObjWithGetter.getterProp;',
-                       chromeSb, "1.7", "http://phony.example.com/file.js", 99);
-  },
-  [{ errorMessage: /property someExpandoProperty \(reason: object is not safely Xrayable/, sourceName: /test_bug1042436/, isWarning: true },
-   { errorMessage: /property getterProp \(reason: property has accessor/, sourceName: /phony/, lineNumber: 99, isWarning: true } ],
-  SimpleTest.finish.bind(SimpleTest));
-
-  ]]>
-  </script>
-</window>
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -5,17 +5,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "XrayWrapper.h"
 #include "AccessCheck.h"
 #include "WrapperFactory.h"
 
 #include "nsIContent.h"
 #include "nsIControllers.h"
-#include "nsIScriptError.h"
 #include "mozilla/dom/Element.h"
 #include "nsContentUtils.h"
 
 #include "XPCWrapper.h"
 #include "xpcprivate.h"
 
 #include "jsapi.h"
 #include "jsprf.h"
@@ -408,79 +407,27 @@ const JSClass JSXrayTraits::HolderClass 
     JS_PropertyStub, JS_DeletePropertyStub,
     JS_PropertyStub, JS_StrictPropertyStub,
     JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub
 };
 
 inline bool
 SilentFailure(JSContext *cx, HandleId id, const char *reason)
 {
-    CompartmentPrivate *priv = CompartmentPrivate::Get(CurrentGlobalOrNull(cx));
-    bool alreadyWarnedOnce = priv->warnedAboutXrays;
-    priv->warnedAboutXrays = true;
-
-    // The browser console warning is only emitted for the first violation,
-    // whereas the (debug-only) NS_WARNING is emitted for each violation.
-#ifndef DEBUG
-    if (alreadyWarnedOnce)
-        return true;
-#endif
-
-    nsDependentJSString propertyName;
-    if (!propertyName.init(cx, id))
+#ifdef DEBUG
+    nsDependentJSString name;
+    if (!name.init(cx, id))
         return false;
     AutoFilename filename;
     unsigned line = 0;
     DescribeScriptedCaller(cx, &filename, &line);
-
-    // Warn to the terminal for the logs.
     NS_WARNING(nsPrintfCString("Denied access to property |%s| on Xrayed Object: %s (@%s:%u)",
-                               NS_LossyConvertUTF16toASCII(propertyName).get(), reason,
+                               NS_LossyConvertUTF16toASCII(name).get(), reason,
                                filename.get(), line).get());
-
-    // If this isn't the first warning on this topic for this global, we've
-    // already bailed out in opt builds. Now that the NS_WARNING is done, bail
-    // out in debug builds as well.
-    if (alreadyWarnedOnce)
-        return true;
-
-    //
-    // Log a message to the console service.
-    //
-
-    // Grab the pieces.
-    nsCOMPtr<nsIConsoleService> consoleService = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
-    NS_ENSURE_TRUE(consoleService, true);
-    nsCOMPtr<nsIScriptError> errorObject = do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
-    NS_ENSURE_TRUE(errorObject, true);
-
-    // Compute the current window id if any.
-    uint64_t windowId = 0;
-    nsGlobalWindow *win = WindowGlobalOrNull(CurrentGlobalOrNull(cx));
-    if (win)
-      windowId = win->WindowID();
-
-    nsPrintfCString errorMessage("XrayWrapper denied access to property %s (reason: %s). "
-                                 "See https://developer.mozilla.org/en-US/docs/Xray_vision "
-                                 "for more information. Note that only the first denied "
-                                 "property access from a given global object will be reported.",
-                                 NS_LossyConvertUTF16toASCII(propertyName).get(),
-                                 reason);
-    nsString filenameStr(NS_ConvertASCIItoUTF16(filename.get()));
-    nsresult rv = errorObject->InitWithWindowID(NS_ConvertASCIItoUTF16(errorMessage),
-                                                filenameStr,
-                                                EmptyString(),
-                                                line, 0,
-                                                nsIScriptError::warningFlag,
-                                                "XPConnect",
-                                                windowId);
-    NS_ENSURE_SUCCESS(rv, true);
-    rv = consoleService->LogMessage(errorObject);
-    NS_ENSURE_SUCCESS(rv, true);
-
+#endif
     return true;
 }
 
 bool JSXrayTraits::getOwnPropertyFromTargetIfSafe(JSContext *cx,
                                                   HandleObject target,
                                                   HandleObject wrapper,
                                                   HandleId idArg,
                                                   MutableHandle<JSPropertyDescriptor> outDesc)
@@ -499,31 +446,27 @@ bool JSXrayTraits::getOwnPropertyFromTar
     if (!JS_GetOwnPropertyDescriptorById(cx, target, id, &desc))
         return false;
 
     // If the property doesn't exist at all, we're done.
     if (!desc.object())
         return true;
 
     // Disallow accessor properties.
-    if (desc.hasGetterOrSetter()) {
-        JSAutoCompartment ac(cx, wrapper);
-        return SilentFailure(cx, id, "property has accessor");
-    }
+    if (desc.hasGetterOrSetter())
+        return SilentFailure(cx, id, "Property has accessor");
 
     // Apply extra scrutiny to objects.
     if (desc.value().isObject()) {
         RootedObject propObj(cx, js::UncheckedUnwrap(&desc.value().toObject()));
         JSAutoCompartment ac(cx, propObj);
 
         // Disallow non-subsumed objects.
-        if (!AccessCheck::subsumes(target, propObj)) {
-            JSAutoCompartment ac(cx, wrapper);
-            return SilentFailure(cx, id, "value not same-origin with target");
-        }
+        if (!AccessCheck::subsumes(target, propObj))
+            return SilentFailure(cx, id, "Value not same-origin with target");
 
         // Disallow non-Xrayable objects.
         if (GetXrayType(propObj) == NotXray) {
             // Note - We're going add Xrays for Arrays/TypedArrays soon in
             // bug 987163, so we don't want to cause unnecessary compat churn
             // by making xrayedObj.arrayProp stop working temporarily, and then
             // start working again. At the same time, this is an important check,
             // and this patch wouldn't be as useful without it. So we just
@@ -531,40 +474,37 @@ bool JSXrayTraits::getOwnPropertyFromTar
             // lands.
             JSProtoKey key = IdentifyStandardInstanceOrPrototype(propObj);
             if (key != JSProto_Uint8ClampedArray &&
                 key != JSProto_Int8Array && key != JSProto_Uint8Array &&
                 key != JSProto_Int16Array && key != JSProto_Uint16Array &&
                 key != JSProto_Int32Array && key != JSProto_Uint32Array &&
                 key != JSProto_Float32Array && key != JSProto_Float64Array)
             {
-                JSAutoCompartment ac(cx, wrapper);
-                return SilentFailure(cx, id, "value not Xrayable");
+                return SilentFailure(cx, id, "Value not Xrayable");
             }
         }
 
         // Disallow callables.
-        if (JS_ObjectIsCallable(cx, propObj)) {
-            JSAutoCompartment ac(cx, wrapper);
-            return SilentFailure(cx, id, "value is callable");
-        }
+        if (JS_ObjectIsCallable(cx, propObj))
+            return SilentFailure(cx, id, "Value is callable");
     }
 
     // Disallow any property that shadows something on its (Xrayed)
     // prototype chain.
     JSAutoCompartment ac2(cx, wrapper);
     RootedObject proto(cx);
     bool foundOnProto = false;
     if (!JS_GetPrototype(cx, wrapper, &proto) ||
         (proto && !JS_HasPropertyById(cx, proto, id, &foundOnProto)))
     {
         return false;
     }
     if (foundOnProto)
-        return SilentFailure(cx, id, "value shadows a property on the standard prototype");
+        return SilentFailure(cx, id, "Value shadows a property on the standard prototype");
 
     // We made it! Assign over the descriptor, and don't forget to wrap.
     outDesc.assign(desc.get());
     return true;
 }
 
 bool
 JSXrayTraits::resolveOwnProperty(JSContext *cx, Wrapper &jsWrapper,