Bug 866450 Part 5: Fix rooting hazards under content/ and dom/ r=bz
☠☠ backed out by 03b95758bc2a ☠ ☠
authorDavid Zbarsky <dzbarsky@gmail.com>
Thu, 02 May 2013 05:12:47 -0400
changeset 130596 c5ba9c0dc252a5046e3500728fd73f8dd2633379
parent 130595 a1e877fa8d67dfbe636b2f2af34f19fd3b72f10d
child 130597 7c0ace2560c4b6949f915c25a60b0ff60e839b3a
push id1579
push userphilringnalda@gmail.com
push dateSat, 04 May 2013 04:38:04 +0000
treeherderfx-team@a56432a42a41 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs866450
milestone23.0a1
Bug 866450 Part 5: Fix rooting hazards under content/ and dom/ r=bz
content/base/src/nsContentUtils.cpp
content/base/src/nsDOMBlobBuilder.cpp
content/base/src/nsDOMFileReader.cpp
content/base/src/nsDocument.cpp
content/events/src/nsEventListenerService.cpp
content/html/content/src/HTMLCanvasElement.cpp
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@@ -6375,21 +6375,21 @@ nsContentUtils::IsPatternMatching(nsAStr
   JSObject* re = JS_NewUCRegExpObjectNoStatics(cx, static_cast<jschar*>
                                                  (aPattern.BeginWriting()),
                                                aPattern.Length(), 0);
   if (!re) {
     JS_ClearPendingException(cx);
     return true;
   }
 
-  JS::Value rval = JS::NullValue();
+  JS::Rooted<JS::Value> rval(cx, JS::NullValue());
   size_t idx = 0;
   if (!JS_ExecuteRegExpNoStatics(cx, re,
                                  static_cast<jschar*>(aValue.BeginWriting()),
-                                 aValue.Length(), &idx, true, &rval)) {
+                                 aValue.Length(), &idx, true, rval.address())) {
     JS_ClearPendingException(cx);
     return true;
   }
 
   return !rval.isNull();
 }
 
 // static
--- a/content/base/src/nsDOMBlobBuilder.cpp
+++ b/content/base/src/nsDOMBlobBuilder.cpp
@@ -204,53 +204,53 @@ nsDOMMultipartFile::InitBlob(JSContext* 
     }
   }
 
   if (aArgc > 0) {
     if (!aArgv[0].isObject()) {
       return NS_ERROR_TYPE_ERR; // We're not interested
     }
 
-    JSObject& obj = aArgv[0].toObject();
-    if (!JS_IsArrayObject(aCx, &obj)) {
+    JS::Rooted<JSObject*> obj(aCx, &aArgv[0].toObject());
+    if (!JS_IsArrayObject(aCx, obj)) {
       return NS_ERROR_TYPE_ERR; // We're not interested
     }
 
     BlobSet blobSet;
 
     uint32_t length;
-    JS_ALWAYS_TRUE(JS_GetArrayLength(aCx, &obj, &length));
+    JS_ALWAYS_TRUE(JS_GetArrayLength(aCx, obj, &length));
     for (uint32_t i = 0; i < length; ++i) {
-      JS::Value element;
-      if (!JS_GetElement(aCx, &obj, i, &element))
+      JS::Rooted<JS::Value> element(aCx);
+      if (!JS_GetElement(aCx, obj, i, element.address()))
         return NS_ERROR_TYPE_ERR;
 
       if (element.isObject()) {
-        JSObject& obj = element.toObject();
-        nsCOMPtr<nsIDOMBlob> blob = aUnwrapFunc(aCx, &obj);
+        JS::Rooted<JSObject*> obj(aCx, &element.toObject());
+        nsCOMPtr<nsIDOMBlob> blob = aUnwrapFunc(aCx, obj);
         if (blob) {
           // Flatten so that multipart blobs will never nest
           nsDOMFileBase* file = static_cast<nsDOMFileBase*>(
               static_cast<nsIDOMBlob*>(blob));
           const nsTArray<nsCOMPtr<nsIDOMBlob> >*
               subBlobs = file->GetSubBlobs();
           if (subBlobs) {
             blobSet.AppendBlobs(*subBlobs);
           } else {
             blobSet.AppendBlob(blob);
           }
           continue;
         }
-        if (JS_IsArrayBufferViewObject(&obj)) {
-          blobSet.AppendVoidPtr(JS_GetArrayBufferViewData(&obj),
-                                JS_GetArrayBufferViewByteLength(&obj));
+        if (JS_IsArrayBufferViewObject(obj)) {
+          blobSet.AppendVoidPtr(JS_GetArrayBufferViewData(obj),
+                                JS_GetArrayBufferViewByteLength(obj));
           continue;
         }
-        if (JS_IsArrayBufferObject(&obj)) {
-          blobSet.AppendArrayBuffer(&obj);
+        if (JS_IsArrayBufferObject(obj)) {
+          blobSet.AppendArrayBuffer(obj);
           continue;
         }
         // neither Blob nor ArrayBuffer(View)
       } else if (element.isString()) {
         blobSet.AppendString(element.toString(), nativeEOL, aCx);
         continue;
       }
       // coerce it to a string
--- a/content/base/src/nsDOMFileReader.cpp
+++ b/content/base/src/nsDOMFileReader.cpp
@@ -179,18 +179,18 @@ nsDOMFileReader::GetReadyState(uint16_t 
 {
   *aReadyState = ReadyState();
   return NS_OK;
 }
 
 JS::Value
 nsDOMFileReader::GetResult(JSContext* aCx, ErrorResult& aRv)
 {
-  JS::Value result = JS::UndefinedValue();
-  aRv = GetResult(aCx, &result);
+  JS::Rooted<JS::Value> result(aCx, JS::UndefinedValue());
+  aRv = GetResult(aCx, result.address());
   return result;
 }
 
 NS_IMETHODIMP
 nsDOMFileReader::GetResult(JSContext* aCx, JS::Value* aResult)
 {
   if (mDataFormat == FILE_AS_ARRAYBUFFER) {
     if (mReadyState == nsIDOMFileReader::DONE && mResultArrayBuffer) {
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@@ -4961,39 +4961,37 @@ nsIDocument::CreateAttributeNS(const nsA
   nsRefPtr<Attr> attribute = new Attr(nullptr, nodeInfo.forget(),
                                       EmptyString(), true);
   return attribute.forget();
 }
 
 static JSBool
 CustomElementConstructor(JSContext *aCx, unsigned aArgc, JS::Value* aVp)
 {
-  JS::Value calleeVal = JS_CALLEE(aCx, aVp);
-
-  JS::Rooted<JSObject*> global(aCx, JS_GetGlobalForObject(aCx, &calleeVal.toObject()));
+  JS::CallArgs args = JS::CallArgsFromVp(aArgc, aVp);
+
+  JS::Rooted<JSObject*> global(aCx, JS_GetGlobalForObject(aCx, &args.callee()));
   nsCOMPtr<nsPIDOMWindow> window = do_QueryWrapper(aCx, global);
   MOZ_ASSERT(window, "Should have a non-null window");
 
   nsIDocument* document = window->GetDoc();
 
   // Function name is the type of the custom element.
-  JSString* jsFunName = JS_GetFunctionId(JS_ValueToFunction(aCx, calleeVal));
+  JSString* jsFunName = JS_GetFunctionId(JS_ValueToFunction(aCx, args.calleev()));
   nsDependentJSString elemName;
   if (!elemName.init(aCx, jsFunName)) {
     return false;
   }
 
   nsCOMPtr<nsIContent> newElement;
   nsresult rv = document->CreateElem(elemName, nullptr, kNameSpaceID_XHTML,
                                      getter_AddRefs(newElement));
-  JS::Value v;
-  rv = nsContentUtils::WrapNative(aCx, global, newElement, newElement, &v);
+  rv = nsContentUtils::WrapNative(aCx, global, newElement, newElement, args.rval().address());
   NS_ENSURE_SUCCESS(rv, false);
 
-  JS_SET_RVAL(aCx, aVp, v);
   return true;
 }
 
 bool
 nsDocument::RegisterEnabled()
 {
   static bool sPrefValue =
     Preferences::GetBool("dom.webcomponents.enabled", false);
@@ -5045,49 +5043,49 @@ nsDocument::Register(JSContext* aCx, con
   if (!sgo) {
     rv.Throw(NS_ERROR_UNEXPECTED);
     return nullptr;
   }
   JS::Rooted<JSObject*> global(aCx, sgo->GetGlobalJSObject());
 
   JSAutoCompartment ac(aCx, global);
 
-  JSObject* htmlProto = HTMLElementBinding::GetProtoObject(aCx, global);
+  JS::Handle<JSObject*> htmlProto(HTMLElementBinding::GetProtoObject(aCx, global));
   if (!htmlProto) {
     rv.Throw(NS_ERROR_OUT_OF_MEMORY);
     return nullptr;
   }
 
-  JSObject* protoObject;
+  JS::Rooted<JSObject*> protoObject(aCx);
   if (!aOptions.mPrototype) {
     protoObject = JS_NewObject(aCx, nullptr, htmlProto, nullptr);
     if (!protoObject) {
       rv.Throw(NS_ERROR_UNEXPECTED);
       return nullptr;
     }
   } else {
     // If a prototype is provided, we must check to ensure that it inherits
     // from HTMLElement.
     protoObject = aOptions.mPrototype;
-    if (!JS_WrapObject(aCx, &protoObject)) {
+    if (!JS_WrapObject(aCx, protoObject.address())) {
       rv.Throw(NS_ERROR_UNEXPECTED);
       return nullptr;
     }
 
     // Check the proto chain for HTMLElement prototype.
-    JSObject* protoProto;
-    if (!JS_GetPrototype(aCx, protoObject, &protoProto)) {
+    JS::Rooted<JSObject*> protoProto(aCx);
+    if (!JS_GetPrototype(aCx, protoObject, protoProto.address())) {
       rv.Throw(NS_ERROR_UNEXPECTED);
       return nullptr;
     }
     while (protoProto) {
       if (protoProto == htmlProto) {
         break;
       }
-      if (!JS_GetPrototype(aCx, protoProto, &protoProto)) {
+      if (!JS_GetPrototype(aCx, protoProto, protoProto.address())) {
         rv.Throw(NS_ERROR_UNEXPECTED);
         return nullptr;
       }
     }
 
     if (!protoProto) {
       rv.Throw(NS_ERROR_DOM_TYPE_MISMATCH_ERR);
       return nullptr;
@@ -6579,18 +6577,18 @@ nsIDocument::AdoptNode(nsINode& aAdopted
     newScope = GetWrapper();
     if (!newScope && GetScopeObject() && GetScopeObject()->GetGlobalJSObject()) {
       // We need to pass some sort of scope object to WrapNative. It's kind of
       // irrelevant, given that we're passing aAllowWrapping = false, and
       // documents should always insist on being wrapped in an canonical
       // scope. But we try to pass something sane anyway.
       JS::Rooted<JSObject*> global(cx, GetScopeObject()->GetGlobalJSObject());
 
-      JS::Value v;
-      rv = nsContentUtils::WrapNative(cx, global, this, this, &v, nullptr,
+      JS::Rooted<JS::Value> v(cx);
+      rv = nsContentUtils::WrapNative(cx, global, this, this, v.address(), nullptr,
                                       /* aAllowWrapping = */ false);
       if (rv.Failed())
         return nullptr;
       newScope = &v.toObject();
     }
   }
 
   nsCOMArray<nsINode> nodesWithProperties;
@@ -11212,21 +11210,21 @@ nsIDocument::PostCreateWrapper(JSContext
 
   if (this != win->GetExtantDoc()) {
     // We're not the current document; we're also done here
     return true;
   }
 
   JSAutoCompartment ac(aCx, aNewObject);
 
-  jsval winVal;
+  JS::Rooted<JS::Value> winVal(aCx);
   nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
   nsresult rv = nsContentUtils::WrapNative(aCx, aNewObject, win,
                                            &NS_GET_IID(nsIDOMWindow),
-                                           &winVal, getter_AddRefs(holder),
+                                           winVal.address(), getter_AddRefs(holder),
                                            false);
   if (NS_FAILED(rv)) {
     return Throw<true>(aCx, rv);
   }
 
   NS_NAMED_LITERAL_STRING(doc_str, "document");
 
   return JS_DefineUCProperty(aCx, JSVAL_TO_OBJECT(winVal),
--- a/content/events/src/nsEventListenerService.cpp
+++ b/content/events/src/nsEventListenerService.cpp
@@ -79,18 +79,18 @@ NS_IMPL_ISUPPORTS1(nsEventListenerServic
 bool
 nsEventListenerInfo::GetJSVal(JSContext* aCx,
                               mozilla::Maybe<JSAutoCompartment>& aAc,
                               JS::Value* aJSVal)
 {
   *aJSVal = JSVAL_NULL;
   nsCOMPtr<nsIXPConnectWrappedJS> wrappedJS = do_QueryInterface(mListener);
   if (wrappedJS) {
-    JSObject* object = nullptr;
-    if (NS_FAILED(wrappedJS->GetJSObject(&object))) {
+    JS::Rooted<JSObject*> object(aCx, nullptr);
+    if (NS_FAILED(wrappedJS->GetJSObject(object.address()))) {
       return false;
     }
     aAc.construct(aCx, object);
     *aJSVal = OBJECT_TO_JSVAL(object);
     return true;
   }
 
   nsCOMPtr<nsIJSEventListener> jsl = do_QueryInterface(mListener);
@@ -110,18 +110,18 @@ nsEventListenerInfo::ToSource(nsAString&
 {
   aResult.SetIsVoid(true);
 
   SafeAutoJSContext cx;
   {
     // Extra block to finish the auto request before calling pop
     JSAutoRequest ar(cx);
     mozilla::Maybe<JSAutoCompartment> ac;
-    JS::Value v = JSVAL_NULL;
-    if (GetJSVal(cx, ac, &v)) {
+    JS::Rooted<JS::Value> v(cx, JSVAL_NULL);
+    if (GetJSVal(cx, ac, v.address())) {
       JSString* str = JS_ValueToSource(cx, v);
       if (str) {
         nsDependentJSString depStr;
         if (depStr.init(cx, str)) {
           aResult.Assign(depStr);
         }
       }
     }
@@ -134,28 +134,28 @@ nsEventListenerInfo::GetDebugObject(nsIS
 {
   *aRetVal = nullptr;
 
 #ifdef MOZ_JSDEBUGGER
   nsresult rv = NS_OK;
   nsCOMPtr<jsdIDebuggerService> jsd =
     do_GetService("@mozilla.org/js/jsd/debugger-service;1", &rv);
   NS_ENSURE_SUCCESS(rv, NS_OK);
-  
+
   bool isOn = false;
   jsd->GetIsOn(&isOn);
   NS_ENSURE_TRUE(isOn, NS_OK);
 
   SafeAutoJSContext cx;
   {
     // Extra block to finish the auto request before calling pop
     JSAutoRequest ar(cx);
     mozilla::Maybe<JSAutoCompartment> ac;
-    JS::Value v = JSVAL_NULL;
-    if (GetJSVal(cx, ac, &v)) {
+    JS::Rooted<JS::Value> v(cx, JSVAL_NULL);
+    if (GetJSVal(cx, ac, v.address())) {
       nsCOMPtr<jsdIValue> jsdValue;
       rv = jsd->WrapValue(v, getter_AddRefs(jsdValue));
       NS_ENSURE_SUCCESS(rv, rv);
       jsdValue.forget(aRetVal);
     }
   }
 #endif
 
--- a/content/html/content/src/HTMLCanvasElement.cpp
+++ b/content/html/content/src/HTMLCanvasElement.cpp
@@ -757,23 +757,23 @@ HTMLCanvasElement::GetContext(const nsAS
     // than objects, e.g. plain strings, then we'll need to expand
     // this to know how to create nsISupportsStrings etc.
 
     nsCOMPtr<nsIWritablePropertyBag2> contextProps;
     if (aContextOptions.isObject()) {
       MOZ_ASSERT(aCx);
       contextProps = do_CreateInstance("@mozilla.org/hash-property-bag;1");
 
-      JSObject& opts = aContextOptions.toObject();
-      JS::AutoIdArray props(aCx, JS_Enumerate(aCx, &opts));
+      JS::Rooted<JSObject*> opts(aCx, &aContextOptions.toObject());
+      JS::AutoIdArray props(aCx, JS_Enumerate(aCx, opts));
       for (size_t i = 0; !!props && i < props.length(); ++i) {
         jsid propid = props[i];
-        JS::Value propname, propval;
-        if (!JS_IdToValue(aCx, propid, &propname) ||
-            !JS_GetPropertyById(aCx, &opts, propid, &propval)) {
+        JS::Rooted<JS::Value> propname(aCx), propval(aCx);
+        if (!JS_IdToValue(aCx, propid, propname.address()) ||
+            !JS_GetPropertyById(aCx, opts, propid, propval.address())) {
           return NS_ERROR_FAILURE;
         }
 
         JSString *propnameString = JS_ValueToString(aCx, propname);
         nsDependentJSString pstr;
         if (!propnameString || !pstr.init(aCx, propnameString)) {
           mCurrentContext = nullptr;
           return NS_ERROR_FAILURE;