Bug 865969 part 2. Better rooting in bindings for 'any' arguments. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 03 May 2013 19:29:07 -0400
changeset 141755 d98854b1c00160eaa395dac809e00dde67be71ad
parent 141754 705880d234376d176071f4e62a23399019e608bb
child 141756 e0fabe9f6a89825f45529bd42e560493bc159f03
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)
reviewerssmaug
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 2. Better rooting in bindings for 'any' arguments. r=smaug
content/canvas/src/CanvasRenderingContext2D.cpp
content/canvas/src/CanvasRenderingContext2D.h
content/events/src/nsDOMMessageEvent.h
content/html/content/public/HTMLCanvasElement.h
dom/bindings/BindingDeclarations.h
dom/bindings/Codegen.py
dom/bindings/test/TestBindingHeader.h
--- a/content/canvas/src/CanvasRenderingContext2D.cpp
+++ b/content/canvas/src/CanvasRenderingContext2D.cpp
@@ -1249,43 +1249,44 @@ CanvasRenderingContext2D::GetMozCurrentT
 }
 
 //
 // colors
 //
 
 void
 CanvasRenderingContext2D::SetStyleFromJSValue(JSContext* cx,
-                                              JS::Value& value,
+                                              JS::Handle<JS::Value> value,
                                               Style whichStyle)
 {
   if (value.isString()) {
     nsDependentJSString strokeStyle;
     if (strokeStyle.init(cx, value.toString())) {
       SetStyleFromString(strokeStyle, whichStyle);
     }
     return;
   }
 
   if (value.isObject()) {
     nsCOMPtr<nsISupports> holder;
 
     CanvasGradient* gradient;
+    JS::Rooted<JS::Value> rootedVal(cx, value);
     nsresult rv = xpc_qsUnwrapArg<CanvasGradient>(cx, value, &gradient,
                                                   static_cast<nsISupports**>(getter_AddRefs(holder)),
-                                                  &value);
+                                                  rootedVal.address());
     if (NS_SUCCEEDED(rv)) {
       SetStyleFromGradient(gradient, whichStyle);
       return;
     }
 
     CanvasPattern* pattern;
     rv = xpc_qsUnwrapArg<CanvasPattern>(cx, value, &pattern,
                                         static_cast<nsISupports**>(getter_AddRefs(holder)),
-                                        &value);
+                                        rootedVal.address());
     if (NS_SUCCEEDED(rv)) {
       SetStyleFromPattern(pattern, whichStyle);
       return;
     }
   }
 
   WarnAboutUnexpectedStyle(mCanvasElement);
 }
--- a/content/canvas/src/CanvasRenderingContext2D.h
+++ b/content/canvas/src/CanvasRenderingContext2D.h
@@ -87,24 +87,24 @@ public:
     }
   }
 
   void GetGlobalCompositeOperation(nsAString& op, mozilla::ErrorResult& error);
   void SetGlobalCompositeOperation(const nsAString& op,
                                    mozilla::ErrorResult& error);
   JS::Value GetStrokeStyle(JSContext* cx, mozilla::ErrorResult& error);
 
-  void SetStrokeStyle(JSContext* cx, JS::Value& value)
+  void SetStrokeStyle(JSContext* cx, JS::Handle<JS::Value> value)
   {
     SetStyleFromJSValue(cx, value, STYLE_STROKE);
   }
 
   JS::Value GetFillStyle(JSContext* cx, mozilla::ErrorResult& error);
 
-  void SetFillStyle(JSContext* cx, JS::Value& value)
+  void SetFillStyle(JSContext* cx, JS::Handle<JS::Value> value)
   {
     SetStyleFromJSValue(cx, value, STYLE_FILL);
   }
 
   already_AddRefed<CanvasGradient>
     CreateLinearGradient(double x0, double y0, double x1, double y1);
   already_AddRefed<CanvasGradient>
     CreateRadialGradient(double x0, double y0, double r0, double x1, double y1,
@@ -466,17 +466,18 @@ protected:
   /**
     * Lookup table used to speed up PutImageData().
     */
   static uint8_t (*sPremultiplyTable)[256];
 
   static mozilla::gfx::DrawTarget* sErrorTarget;
 
   // Some helpers.  Doesn't modify a color on failure.
-  void SetStyleFromJSValue(JSContext* cx, JS::Value& value, Style whichStyle);
+  void SetStyleFromJSValue(JSContext* cx, JS::Handle<JS::Value> value,
+			   Style whichStyle);
   void SetStyleFromString(const nsAString& str, Style whichStyle);
 
   void SetStyleFromGradient(CanvasGradient *gradient, Style whichStyle)
   {
     CurrentState().SetGradientStyle(whichStyle, gradient);
   }
 
   void SetStyleFromPattern(CanvasPattern *pattern, Style whichStyle)
--- a/content/events/src/nsDOMMessageEvent.h
+++ b/content/events/src/nsDOMMessageEvent.h
@@ -49,17 +49,17 @@ public:
     nsCOMPtr<nsIDOMWindow> ret = mSource;
     return ret.forget();
   }
 
   void InitMessageEvent(JSContext* aCx,
                         const nsAString& aType,
                         bool aCanBubble,
                         bool aCancelable,
-                        JS::Value& aData,
+                        JS::Value aData,
                         const nsAString& aOrigin,
                         const nsAString& aLastEventId,
                         nsIDOMWindow* aSource,
                         mozilla::ErrorResult& aRv)
   {
     aRv = InitMessageEvent(aType, aCanBubble, aCancelable, aData,
                            aOrigin, aLastEventId, aSource);
   }
--- a/content/html/content/public/HTMLCanvasElement.h
+++ b/content/html/content/public/HTMLCanvasElement.h
@@ -87,27 +87,27 @@ public:
     return GetUnsignedIntAttr(nsGkAtoms::width, DEFAULT_CANVAS_WIDTH);
   }
   void SetWidth(uint32_t aWidth, ErrorResult& aRv)
   {
     SetUnsignedIntAttr(nsGkAtoms::width, aWidth, aRv);
   }
   already_AddRefed<nsISupports>
   GetContext(JSContext* aCx, const nsAString& aContextId,
-             const Optional<JS::Value>& aContextOptions, ErrorResult& aRv)
+             const Optional<LazyRootedValue>& aContextOptions, ErrorResult& aRv)
   {
     JS::Value contextOptions = aContextOptions.WasPassed()
                              ? aContextOptions.Value()
                              : JS::UndefinedValue();
     nsCOMPtr<nsISupports> context;
     aRv = GetContext(aContextId, contextOptions, aCx, getter_AddRefs(context));
     return context.forget();
   }
   void ToDataURL(JSContext* aCx, const nsAString& aType,
-                 const Optional<JS::Value>& aParams, nsAString& aDataURL,
+                 const Optional<LazyRootedValue>& aParams, nsAString& aDataURL,
                  ErrorResult& aRv)
   {
     JS::Value params = aParams.WasPassed()
                      ? aParams.Value()
                      : JS::UndefinedValue();
     aRv = ToDataURL(aType, params, aCx, aDataURL);
   }
   void ToBlob(nsIFileCallback* aCallback, const nsAString& aType,
--- a/dom/bindings/BindingDeclarations.h
+++ b/dom/bindings/BindingDeclarations.h
@@ -510,12 +510,40 @@ public:
 
   JSObject** operator&()
   {
     // Assert if we're empty, on purpose
     return ref().address();
   }
 };
 
+class LazyRootedValue : public Maybe<JS::Rooted<JS::Value> >
+{
+public:
+  operator JS::Value() const
+  {
+    // Assert if we're empty, on purpose
+    return ref();
+  }
+
+  operator JS::Rooted<JS::Value>& ()
+  {
+    // Assert if we're empty, on purpose
+    return ref();
+  }
+
+  operator JS::Handle<JS::Value>()
+  {
+    // Assert if we're empty, on purpose
+    return ref();
+  }
+
+  JS::Value* operator&()
+  {
+    // Assert if we're empty, on purpose
+    return ref().address();
+  }
+};
+
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_BindingDeclarations_h__
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -3217,30 +3217,29 @@ for (uint32_t i = 0; i < length; ++i) {
                 type,
                 "${declName} = nullptr",
                 failureCode)
         return (template, declType, None, isOptional)
 
     if type.isAny():
         assert not isEnforceRange and not isClamp
 
-        if isMember == "Dictionary":
-            declType = "RootedJSValue"
-            templateBody = ("if (!${declName}.SetValue(cx, ${val})) {\n"
-                            "  return false;\n"
-                            "}")
-            nullHandling = "${declName}.SetValue(nullptr, JS::NullValue())"
-        elif isMember and isMember != "Variadic":
-            # Variadic arguments are rooted by being in argv
+        if isMember == "Dictionary" or not isMember:
+            declType = "LazyRootedValue"
+            templateBody = "${declName}.construct(cx, ${val});"
+            nullHandling = "${declName}.construct(cx, JS::NullValue());"
+        elif isMember != "Variadic":
             raise TypeError("Can't handle sequence member 'any'; need to sort "
                             "out rooting issues")
         else:
+            # Variadic arguments are rooted by being in argv
             declType = "JS::Value"
             templateBody = "${declName} = ${val};"
             nullHandling = "${declName} = JS::NullValue()"
+
         templateBody = handleDefaultNull(templateBody, nullHandling)
         return (templateBody, CGGeneric(declType), None, isOptional)
 
     if type.isObject():
         assert not isEnforceRange and not isClamp
         return handleJSObjectType(type, isMember, failureCode)
 
     if type.isDictionary():
@@ -8110,17 +8109,17 @@ class CGNativeMember(ClassMethod):
         # And if we're static, a global
         if self.member.isStatic():
             globalObjectType = "GlobalObject"
             if self.descriptor.workers:
                 globalObjectType = "Worker" + globalObjectType
             args.insert(0, Argument("const %s&" % globalObjectType, "global"))
         return args
 
-    def doGetArgType(self, type, optional, isMember):
+    def doGetArgType(self, type, optional, variadic, isMember):
         """
         The main work of getArgType.  Returns a string type decl, whether this
         is a const ref, as well as whether the type should be wrapped in
         Nullable as needed.
         """
         if type.isArray():
             raise TypeError("Can't handle array arguments yet")
 
@@ -8199,17 +8198,22 @@ class CGNativeMember(ClassMethod):
                     declType = "%s&"
             if type.isCallback():
                 name = type.unroll().identifier.name
             else:
                 name = type.unroll().inner.identifier.name
             return declType % name, False, False
 
         if type.isAny():
-            return "JS::Value", False, False
+            # Don't do the rooting stuff for variadics for now
+            if optional and not variadic:
+                declType = "LazyRootedValue"
+            else:
+                declType = "JS::Value"
+            return declType, False, False
 
         if type.isObject():
             if optional:
                 if type.nullable():
                     declType = "LazyRootedObject"
                 else:
                     declType = "NonNullLazyRootedObject"
             elif type.nullable() or self.jsObjectsArePtr:
@@ -8230,17 +8234,17 @@ class CGNativeMember(ClassMethod):
 
         return builtinNames[type.tag()], False, True
 
     def getArgType(self, type, optional, variadic, isMember):
         """
         Get the type of an argument declaration.  Returns the type CGThing, and
         whether this should be a const ref.
         """
-        (decl, ref, handleNullable) = self.doGetArgType(type, optional,
+        (decl, ref, handleNullable) = self.doGetArgType(type, optional, variadic,
                                                         isMember or variadic)
         decl = CGGeneric(decl)
         if handleNullable and type.nullable():
             decl = CGTemplatedType("Nullable", decl)
             ref = True
         if variadic:
             arrayType = "Sequence" if self.variadicIsSequence else "nsTArray"
             decl = CGTemplatedType(arrayType, decl)
--- a/dom/bindings/test/TestBindingHeader.h
+++ b/dom/bindings/test/TestBindingHeader.h
@@ -135,17 +135,17 @@ public:
   static
   already_AddRefed<TestInterface> Test2(const GlobalObject&,
                                         JSContext*,
                                         const DictForConstructor&,
                                         JS::Value,
                                         JSObject&,
                                         JSObject*,
                                         const Sequence<Dict>&,
-                                        const Optional<JS::Value>&,
+                                        const Optional<LazyRootedValue>&,
                                         const Optional<NonNullLazyRootedObject>&,
                                         const Optional<LazyRootedObject>&,
                                         ErrorResult&);
 
   // Integer types
   int8_t ReadonlyByte();
   int8_t WritableByte();
   void SetWritableByte(int8_t);
@@ -424,17 +424,17 @@ public:
   void PassOptionalNullableTreatAsNullCallbackWithDefaultValue(TestTreatAsNullCallback*);
   void SetTreatAsNullCallback(TestTreatAsNullCallback&);
   already_AddRefed<TestTreatAsNullCallback> TreatAsNullCallback();
   void SetNullableTreatAsNullCallback(TestTreatAsNullCallback*);
   already_AddRefed<TestTreatAsNullCallback> GetNullableTreatAsNullCallback();
 
   // Any types
   void PassAny(JSContext*, JS::Value);
-  void PassOptionalAny(JSContext*, const Optional<JS::Value>&);
+  void PassOptionalAny(JSContext*, const Optional<LazyRootedValue>&);
   void PassAnyDefaultNull(JSContext*, JS::Value);
   JS::Value ReceiveAny(JSContext*);
 
   // object types
   void PassObject(JSContext*, JSObject&);
   void PassNullableObject(JSContext*, JSObject*);
   void PassOptionalObject(JSContext*, const Optional<NonNullLazyRootedObject>&);
   void PassOptionalNullableObject(JSContext*, const Optional<LazyRootedObject>&);