Bug 865969 part 7. Fix rooting hazards in DOMJSProxyHandler.cpp. r=ms2ger
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 03 May 2013 19:29:09 -0400
changeset 141760 53c579eac0dee334b576e285a2f6db94b12cdfae
parent 141759 35c6ad8bb2e4aeac66cb6c80eac2216b96fbc43f
child 141761 7b6fb38754e109a5f420395bf7d81b121284c821
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersms2ger
bugs865969
milestone23.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 865969 part 7. Fix rooting hazards in DOMJSProxyHandler.cpp. r=ms2ger
dom/base/nsGlobalWindow.cpp
dom/bindings/DOMJSProxyHandler.cpp
dom/bindings/DOMJSProxyHandler.h
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -609,24 +609,25 @@ protected:
   nsGlobalWindow* GetWindow(JSObject *proxy)
   {
     return nsGlobalWindow::FromSupports(
       static_cast<nsISupports*>(js::GetProxyExtra(proxy, 0).toPrivate()));
   }
 
   // False return value means we threw an exception.  True return value
   // but false "found" means we didn't have a subframe at that index.
-  bool GetSubframeWindow(JSContext *cx, JSObject *proxy, jsid id,
+  bool GetSubframeWindow(JSContext *cx, JS::Handle<JSObject*> proxy,
+                         JS::Handle<jsid> id,
                          JS::Value *vp, bool &found);
 
   // Returns a non-null window only if id is an index and we have a
   // window at that index.
   already_AddRefed<nsIDOMWindow> GetSubframeWindow(JSContext *cx,
-                                                   JSObject *proxy,
-                                                   jsid id);
+                                                   JS::Handle<JSObject*> proxy,
+                                                   JS::Handle<jsid> id);
 
   bool AppendIndexedPropertyNames(JSContext *cx, JSObject *proxy,
                                   JS::AutoIdVector &props);
 };
 
 bool
 nsOuterWindowProxy::isExtensible(JSObject *proxy)
 {
@@ -864,18 +865,19 @@ nsOuterWindowProxy::iterate(JSContext *c
                             unsigned flags, JS::MutableHandle<JS::Value> vp)
 {
   // BaseProxyHandler::iterate seems to do what we want here: fall
   // back on the property names returned from keys() and enumerate().
   return js::BaseProxyHandler::iterate(cx, proxy, flags, vp);
 }
 
 bool
-nsOuterWindowProxy::GetSubframeWindow(JSContext *cx, JSObject *proxy,
-                                      jsid id, JS::Value* vp,
+nsOuterWindowProxy::GetSubframeWindow(JSContext *cx,
+                                      JS::Handle<JSObject*> proxy,
+                                      JS::Handle<jsid> id, JS::Value* vp,
                                       bool& found)
 {
   nsCOMPtr<nsIDOMWindow> frame = GetSubframeWindow(cx, proxy, id);
   if (!frame) {
     found = false;
     return true;
   }
 
@@ -890,17 +892,19 @@ nsOuterWindowProxy::GetSubframeWindow(JS
   if (MOZ_UNLIKELY(!obj)) {
     return xpc::Throw(cx, NS_ERROR_FAILURE);
   }
   *vp = JS::ObjectValue(*obj);
   return JS_WrapValue(cx, vp);
 }
 
 already_AddRefed<nsIDOMWindow>
-nsOuterWindowProxy::GetSubframeWindow(JSContext *cx, JSObject *proxy, jsid id)
+nsOuterWindowProxy::GetSubframeWindow(JSContext *cx,
+                                      JS::Handle<JSObject*> proxy,
+                                      JS::Handle<jsid> id)
 {
   int32_t index = GetArrayIndexFromId(cx, id);
   if (!IsArrayIndex(index)) {
     return nullptr;
   }
 
   nsGlobalWindow* win = GetWindow(proxy);
   bool unused;
--- a/dom/bindings/DOMJSProxyHandler.cpp
+++ b/dom/bindings/DOMJSProxyHandler.cpp
@@ -55,20 +55,20 @@ DOMProxyHandler::GetAndClearExpandoObjec
   XPCWrappedNativeScope* scope = xpc::GetObjectScope(obj);
   scope->RemoveDOMExpandoObject(obj);
   js::SetProxyExtra(obj, JSPROXYSLOT_EXPANDO, UndefinedValue());
   return expando;
 }
 
 // static
 JSObject*
-DOMProxyHandler::EnsureExpandoObject(JSContext* cx, JSObject* obj)
+DOMProxyHandler::EnsureExpandoObject(JSContext* cx, JS::Handle<JSObject*> obj)
 {
   NS_ASSERTION(IsDOMProxy(obj), "expected a DOM proxy object");
-  JSObject* expando = GetExpandoObject(obj);
+  JS::Rooted<JSObject*> expando(cx, GetExpandoObject(obj));
   if (!expando) {
     expando = JS_NewObjectWithGivenProto(cx, nullptr, nullptr,
                                          js::GetObjectParent(obj));
     if (!expando) {
       return NULL;
     }
 
     XPCWrappedNativeScope* scope = xpc::GetObjectScope(obj);
@@ -105,18 +105,18 @@ DOMProxyHandler::getPropertyDescriptor(J
 {
   if (!getOwnPropertyDescriptor(cx, proxy, id, desc, flags)) {
     return false;
   }
   if (desc->obj) {
     return true;
   }
 
-  JSObject* proto;
-  if (!js::GetObjectProto(cx, proxy, &proto)) {
+  JS::Rooted<JSObject*> proto(cx);
+  if (!js::GetObjectProto(cx, proxy, proto.address())) {
     return false;
   }
   if (!proto) {
     desc->obj = NULL;
     return true;
   }
 
   return JS_GetPropertyDescriptorById(cx, proto, id, 0, desc);
@@ -148,33 +148,34 @@ DOMProxyHandler::defineProperty(JSContex
 }
 
 bool
 DOMProxyHandler::delete_(JSContext* cx, JS::Handle<JSObject*> proxy,
                          JS::Handle<jsid> id, bool* bp)
 {
   JSBool b = true;
 
-  JSObject* expando;
+  JS::Rooted<JSObject*> expando(cx);
   if (!xpc::WrapperFactory::IsXrayWrapper(proxy) && (expando = GetExpandoObject(proxy))) {
-    Value v;
-    if (!JS_DeletePropertyById2(cx, expando, id, &v) || !JS_ValueToBoolean(cx, v, &b)) {
+    JS::Rooted<Value> v(cx);
+    if (!JS_DeletePropertyById2(cx, expando, id, v.address()) ||
+        !JS_ValueToBoolean(cx, v, &b)) {
       return false;
     }
   }
 
   *bp = !!b;
   return true;
 }
 
 bool
 DOMProxyHandler::enumerate(JSContext* cx, JS::Handle<JSObject*> proxy, AutoIdVector& props)
 {
-  JSObject* proto;
-  if (!JS_GetPrototype(cx, proxy, &proto)) {
+  JS::Rooted<JSObject*> proto(cx);
+  if (!JS_GetPrototype(cx, proxy, proto.address())) {
     return false;
   }
   return getOwnPropertyNames(cx, proxy, props) &&
          (!proto || js::GetPropertyNames(cx, proto, 0, &props));
 }
 
 bool
 DOMProxyHandler::has(JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id, bool* bp)
@@ -185,18 +186,18 @@ DOMProxyHandler::has(JSContext* cx, JS::
 
   if (*bp) {
     // We have the property ourselves; no need to worry about our prototype
     // chain.
     return true;
   }
 
   // OK, now we have to look at the proto
-  JSObject *proto;
-  if (!js::GetObjectProto(cx, proxy, &proto)) {
+  JS::Rooted<JSObject*> proto(cx);
+  if (!js::GetObjectProto(cx, proxy, proto.address())) {
     return false;
   }
   if (!proto) {
     return true;
   }
   JSBool protoHasProp;
   bool ok = JS_HasPropertyById(cx, proto, id, &protoHasProp);
   if (ok) {
@@ -207,18 +208,18 @@ DOMProxyHandler::has(JSContext* cx, JS::
 
 bool
 DOMProxyHandler::AppendNamedPropertyIds(JSContext* cx,
                                         JS::Handle<JSObject*> proxy,
                                         nsTArray<nsString>& names,
                                         JS::AutoIdVector& props)
 {
   for (uint32_t i = 0; i < names.Length(); ++i) {
-    JS::Value v;
-    if (!xpc::NonVoidStringToJsval(cx, names[i], &v)) {
+    JS::Rooted<JS::Value> v(cx);
+    if (!xpc::NonVoidStringToJsval(cx, names[i], v.address())) {
       return false;
     }
 
     JS::Rooted<jsid> id(cx);
     if (!JS_ValueToId(cx, v, id.address())) {
       return false;
     }
 
@@ -228,17 +229,17 @@ DOMProxyHandler::AppendNamedPropertyIds(
       }
     }
   }
 
   return true;
 }
 
 int32_t
-IdToInt32(JSContext* cx, jsid id)
+IdToInt32(JSContext* cx, JS::Handle<jsid> id)
 {
   JSAutoRequest ar(cx);
 
   JS::Value idval;
   double array_index;
   int32_t i;
   if (!::JS_IdToValue(cx, id, &idval) ||
       !::JS_ValueToNumber(cx, idval, &array_index) ||
--- a/dom/bindings/DOMJSProxyHandler.h
+++ b/dom/bindings/DOMJSProxyHandler.h
@@ -49,36 +49,37 @@ public:
 
   static JSObject* GetExpandoObject(JSObject* obj)
   {
     MOZ_ASSERT(IsDOMProxy(obj), "expected a DOM proxy object");
     JS::Value v = js::GetProxyExtra(obj, JSPROXYSLOT_EXPANDO);
     return v.isUndefined() ? NULL : v.toObjectOrNull();
   }
   static JSObject* GetAndClearExpandoObject(JSObject* obj);
-  static JSObject* EnsureExpandoObject(JSContext* cx, JSObject* obj);
+  static JSObject* EnsureExpandoObject(JSContext* cx,
+                                       JS::Handle<JSObject*> obj);
 
   const DOMClass& mClass;
 
 protected:
   // Append the property names in "names" that don't live on our proto
   // chain to "props"
   bool AppendNamedPropertyIds(JSContext* cx, JS::Handle<JSObject*> proxy,
                               nsTArray<nsString>& names,
                               JS::AutoIdVector& props);
 };
 
 extern jsid s_length_id;
 
-int32_t IdToInt32(JSContext* cx, jsid id);
+int32_t IdToInt32(JSContext* cx, JS::Handle<jsid> id);
 
 // XXXbz this should really return uint32_t, with the maximum value
 // meaning "not an index"...
 inline int32_t
-GetArrayIndexFromId(JSContext* cx, jsid id)
+GetArrayIndexFromId(JSContext* cx, JS::Handle<jsid> id)
 {
   if (MOZ_LIKELY(JSID_IS_INT(id))) {
     return JSID_TO_INT(id);
   }
   if (MOZ_LIKELY(id == s_length_id)) {
     return -1;
   }
   if (MOZ_LIKELY(JSID_IS_ATOM(id))) {
@@ -113,14 +114,14 @@ FillPropertyDescriptor(JSPropertyDescrip
 inline void
 FillPropertyDescriptor(JSPropertyDescriptor* desc, JSObject* obj, JS::Value v, bool readonly)
 {
   desc->value = v;
   FillPropertyDescriptor(desc, obj, readonly);
 }
 
 JSObject*
-EnsureExpandoObject(JSContext* cx, JSObject* obj);
+EnsureExpandoObject(JSContext* cx, JS::Handle<JSObject*> obj);
 
 } // namespace dom
 } // namespace mozilla
 
 #endif /* mozilla_dom_DOMProxyHandler_h */