Bug 1215072 - throw in case dictionary initialization fails in canvas.getContext, r=baku

--HG--
extra : rebase_source : 006bfed7cfec46e6150ee15135de08941283dbb7

dom/canvas/CanvasRenderingContext2D.cpp

@@ -1606,7 +1606,9 @@ CanvasRenderingContext2D::SetIsIPC(bool isIPC)
 }
 
 NS_IMETHODIMP
-CanvasRenderingContext2D::SetContextOptions(JSContext* aCx, JS::Handle<JS::Value> aOptions)
+CanvasRenderingContext2D::SetContextOptions(JSContext* aCx,
+                                            JS::Handle<JS::Value> aOptions,
+                                            ErrorResult& aRvForDictionaryInit)
 {
   if (aOptions.isNullOrUndefined()) {
     return NS_OK;
@@ -1616,7 +1618,10 @@ CanvasRenderingContext2D::SetContextOptions(JSContext* aCx, JS::Handle<JS::Value
   MOZ_ASSERT(!mTarget);
 
   ContextAttributes2D attributes;
-  NS_ENSURE_TRUE(attributes.Init(aCx, aOptions), NS_ERROR_UNEXPECTED);
+  if (!attributes.Init(aCx, aOptions)) {
+    aRvForDictionaryInit.Throw(NS_ERROR_UNEXPECTED);
+    return NS_ERROR_UNEXPECTED;
+  }
 
   if (Preferences::GetBool("gfx.canvas.willReadFrequently.enable", false)) {
     // Use software when there is going to be a lot of readback

dom/canvas/CanvasRenderingContext2D.h

@@ -465,7 +465,9 @@ public:
   // this rect is in canvas device space
   void Redraw(const mozilla::gfx::Rect &r);
   NS_IMETHOD Redraw(const gfxRect &r) override { Redraw(ToRect(r)); return NS_OK; }
-  NS_IMETHOD SetContextOptions(JSContext* aCx, JS::Handle<JS::Value> aOptions) override;
+  NS_IMETHOD SetContextOptions(JSContext* aCx,
+                               JS::Handle<JS::Value> aOptions,
+                               ErrorResult& aRvForDictionaryInit) override;
 
   /**
    * An abstract base class to be implemented by callers wanting to be notified

dom/canvas/CanvasRenderingContextHelper.cpp

@@ -173,9 +173,11 @@ CanvasRenderingContextHelper::GetContext(JSContext* aCx,
     mCurrentContext = context.forget();
     mCurrentContextType = contextType;
 
-    aRv = UpdateContext(aCx, aContextOptions);
+    nsresult rv = UpdateContext(aCx, aContextOptions, aRv);
-    if (aRv.Failed()) {
+    if (NS_FAILED(rv)) {
-      aRv = NS_OK; // See bug 645792
+      // See bug 645792 and bug 1215072.
+      // We want to throw only if dictionary initialization fails,
+      // so only in case aRv has been set to some error value.
       return nullptr;
     }
   } else {
@@ -190,7 +192,8 @@ CanvasRenderingContextHelper::GetContext(JSContext* aCx,
 
 nsresult
 CanvasRenderingContextHelper::UpdateContext(JSContext* aCx,
-                                            JS::Handle<JS::Value> aNewContextOptions)
+                                            JS::Handle<JS::Value> aNewContextOptions,
+                                            ErrorResult& aRvForDictionaryInit)
 {
   if (!mCurrentContext)
     return NS_OK;
@@ -205,7 +208,8 @@ CanvasRenderingContextHelper::UpdateContext(JSContext* aCx,
     return rv;
   }
 
-  rv = currentContext->SetContextOptions(aCx, aNewContextOptions);
+  rv = currentContext->SetContextOptions(aCx, aNewContextOptions,
+                                         aRvForDictionaryInit);
   if (NS_FAILED(rv)) {
     mCurrentContext = nullptr;
     return rv;

dom/canvas/CanvasRenderingContextHelper.h

@@ -44,7 +44,8 @@ public:
 
 protected:
   virtual nsresult UpdateContext(JSContext* aCx,
-                                 JS::Handle<JS::Value> aNewContextOptions);
+                                 JS::Handle<JS::Value> aNewContextOptions,
+                                 ErrorResult& aRvForDictionaryInit);
 
   virtual nsresult ParseParams(JSContext* aCx,
                                const nsAString& aType,

dom/canvas/OffscreenCanvas.h

@@ -158,7 +158,8 @@ private:
   void CanvasAttrChanged()
   {
     mAttrDirty = true;
-    UpdateContext(nullptr, JS::NullHandleValue);
+    ErrorResult dummy;
+    UpdateContext(nullptr, JS::NullHandleValue, dummy);
   }
 
   bool mAttrDirty;

dom/canvas/WebGLContext.cpp

@@ -347,13 +347,17 @@ WebGLContext::OnMemoryPressure()
 //
 
 NS_IMETHODIMP
-WebGLContext::SetContextOptions(JSContext* cx, JS::Handle<JS::Value> options)
+WebGLContext::SetContextOptions(JSContext* cx, JS::Handle<JS::Value> options,
+                                ErrorResult& aRvForDictionaryInit)
 {
     if (options.isNullOrUndefined() && mOptionsFrozen)
         return NS_OK;
 
     WebGLContextAttributes attributes;
-    NS_ENSURE_TRUE(attributes.Init(cx, options), NS_ERROR_UNEXPECTED);
+    if (!attributes.Init(cx, options)) {
+      aRvForDictionaryInit.Throw(NS_ERROR_UNEXPECTED);
+      return NS_ERROR_UNEXPECTED;
+    }
 
     WebGLContextOptions newOpts;
 

dom/canvas/WebGLContext.h

@@ -249,7 +249,8 @@ public:
     NS_IMETHOD SetIsOpaque(bool) override { return NS_OK; };
     bool GetIsOpaque() override { return false; }
     NS_IMETHOD SetContextOptions(JSContext* cx,
-                                 JS::Handle<JS::Value> options) override;
+                                 JS::Handle<JS::Value> options,
+                                 ErrorResult& aRvForDictionaryInit) override;
 
     NS_IMETHOD SetIsIPC(bool) override {
         return NS_ERROR_NOT_IMPLEMENTED;

dom/canvas/nsICanvasRenderingContextInternal.h

@@ -149,7 +149,11 @@ public:
   // Redraw the dirty rectangle of this canvas.
   NS_IMETHOD Redraw(const gfxRect &dirty) = 0;
 
-  NS_IMETHOD SetContextOptions(JSContext* cx, JS::Handle<JS::Value> options) { return NS_OK; }
+  NS_IMETHOD SetContextOptions(JSContext* cx, JS::Handle<JS::Value> options,
+                               mozilla::ErrorResult& aRvForDictionaryInit)
+  {
+    return NS_OK;
+  }
 
   // return true and fills in the bounding rect if elementis a child and has a hit region.
   virtual bool GetHitRegionRect(mozilla::dom::Element* element, nsRect& rect) { return false; }

dom/canvas/test/mochitest.ini

@@ -225,6 +225,7 @@ disabled = bug 407107
 [test_bug866575.html]
 skip-if = (toolkit == 'gonk' && debug) #bug 1045153
 [test_bug902651.html]
+[test_bug1215072.html]
 [test_canvas.html]
 skip-if = (toolkit == 'gonk' && debug) || (toolkit == 'android' && processor == 'x86') || (android_version == '18' && debug) #debug-only crash; bug 933541 #x86 only bug 913662 #android 4.3 debug bug 1143317
 [test_canvas_focusring.html]

dom/canvas/test/test_bug1215072.html

@@ -0,0 +1,41 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1215072
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1215072</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for Bug 1215072 **/
+  try {
+    document.createElement("canvas")
+            .getContext("webgl", { get stencil() { throw "bah (webgl)"; } });
+    ok(false);
+  } catch(ex) {
+    is(ex, "bah (webgl)", "Should have thrown an exception.");
+  }
+
+  try {
+    document.createElement("canvas")
+            .getContext("2d", { get alpha() {throw "bah (2d)"; } });
+    ok(false);
+  } catch(ex) {
+    is(ex, "bah (2d)", "Should have thrown an exception.");
+  }
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1215072">Mozilla Bug 1215072</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>

dom/html/HTMLCanvasElement.cpp

@@ -449,7 +449,8 @@ HTMLCanvasElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
       aNameSpaceID == kNameSpaceID_None &&
       (aName == nsGkAtoms::width || aName == nsGkAtoms::height || aName == nsGkAtoms::moz_opaque))
   {
-    rv = UpdateContext(nullptr, JS::NullHandleValue);
+    ErrorResult dummy;
+    rv = UpdateContext(nullptr, JS::NullHandleValue, dummy);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -465,7 +466,8 @@ HTMLCanvasElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName,
       aNameSpaceID == kNameSpaceID_None &&
       (aName == nsGkAtoms::width || aName == nsGkAtoms::height || aName == nsGkAtoms::moz_opaque))
   {
-    rv = UpdateContext(nullptr, JS::NullHandleValue);
+    ErrorResult dummy;
+    rv = UpdateContext(nullptr, JS::NullHandleValue, dummy);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   return rv;
@@ -911,7 +913,8 @@ HTMLCanvasElement::MozGetIPCContext(const nsAString& aContextId,
     mCurrentContext->SetIsIPC(true);
     mCurrentContextType = contextType;
 
-    nsresult rv = UpdateContext(nullptr, JS::NullHandleValue);
+    ErrorResult dummy;
+    nsresult rv = UpdateContext(nullptr, JS::NullHandleValue, dummy);
     NS_ENSURE_SUCCESS(rv, rv);
   } else {
     // We already have a context of some type.