Bug 1205923: Make VectorImage::GetWidth/GetHeight set outparam to 0 (not -1) on failure, to accomodate callers that don't check error codes. r=seth

2015-09-18 15:33:43 -07:00 · 2015-09-18 15:33:43 -07:00 · 3efe8e4b74
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@ -497,14 +497,23 @@ NS_IMETHODIMP
 VectorImage::GetWidth(int32_t* aWidth)
 {
  if (mError || !mIsFullyLoaded) {
-    *aWidth = -1;
-  } else {
-    SVGSVGElement* rootElem = mSVGDocumentWrapper->GetRootSVGElem();
-    MOZ_ASSERT(rootElem, "Should have a root SVG elem, since we finished "
-                         "loading without errors");
-    *aWidth = rootElem->GetIntrinsicWidth();
+    // XXXdholbert Technically we should leave outparam untouched when we
+    // fail. But since many callers don't check for failure, we set it to 0 on
+    // failure, for sane/predictable results.
+    *aWidth = 0;
+    return NS_ERROR_FAILURE;
  }
-  return *aWidth >= 0 ? NS_OK : NS_ERROR_FAILURE;
+
+  SVGSVGElement* rootElem = mSVGDocumentWrapper->GetRootSVGElem();
+  MOZ_ASSERT(rootElem, "Should have a root SVG elem, since we finished "
+             "loading without errors");
+  int32_t rootElemWidth = rootElem->GetIntrinsicWidth();
+  if (rootElemWidth < 0) {
+    *aWidth = 0;
+    return NS_ERROR_FAILURE;
+  }
+  *aWidth = rootElemWidth;
+  return NS_OK;
 }

 //******************************************************************************
@ -561,14 +570,23 @@ NS_IMETHODIMP
 VectorImage::GetHeight(int32_t* aHeight)
 {
  if (mError || !mIsFullyLoaded) {
-    *aHeight = -1;
-  } else {
-    SVGSVGElement* rootElem = mSVGDocumentWrapper->GetRootSVGElem();
-    MOZ_ASSERT(rootElem, "Should have a root SVG elem, since we finished "
-                         "loading without errors");
-    *aHeight = rootElem->GetIntrinsicHeight();
+    // XXXdholbert Technically we should leave outparam untouched when we
+    // fail. But since many callers don't check for failure, we set it to 0 on
+    // failure, for sane/predictable results.
+    *aHeight = 0;
+    return NS_ERROR_FAILURE;
  }
-  return *aHeight >= 0 ? NS_OK : NS_ERROR_FAILURE;
+
+  SVGSVGElement* rootElem = mSVGDocumentWrapper->GetRootSVGElem();
+  MOZ_ASSERT(rootElem, "Should have a root SVG elem, since we finished "
+             "loading without errors");
+  int32_t rootElemHeight = rootElem->GetIntrinsicHeight();
+  if (rootElemHeight < 0) {
+    *aHeight = 0;
+    return NS_ERROR_FAILURE;
+  }
+  *aHeight = rootElemHeight;
+  return NS_OK;
 }

 //******************************************************************************
--- a/image/test/crashtests/1205923-1.html
+++ b/image/test/crashtests/1205923-1.html
@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<body>
+</body>
+<script>
+  function createImage(loadHandler) {
+    var newImage = new Image;
+    newImage.id = "thepreviewimage";
+    newImage.setAttribute("src", "unsized-svg.svg");
+    if (loadHandler) {
+      newImage.onload = loadHandler;
+    }
+
+    // Query width & height, and display them in document:
+    physWidth = newImage.width;
+    physHeight = newImage.height;
+    document.documentElement.innerHTML +=
+      physWidth + " x " + physHeight + "<br>";
+  }
+
+  function part2() {
+    // Load image again:
+    createImage();
+
+    // End the crashtest.
+    document.documentElement.removeAttribute("class");
+  }
+
+  function startTest() {
+    // Trigger image load, and call part2() when it's loaded:
+    createImage(part2);
+  }
+
+  startTest();
+</script>
+</html>
--- a/image/test/crashtests/crashtests.list
+++ b/image/test/crashtests/crashtests.list
@ -53,6 +53,8 @@ load invalid-disposal-method-1.gif
 load invalid-disposal-method-2.gif
 load invalid-disposal-method-3.gif

+load 1205923-1.html
+
 # Ensure we handle ICO directory entries which specify the wrong size for the
 # contained resource.
 load invalid_ico_height.ico
--- a/image/test/crashtests/unsized-svg.svg
+++ b/image/test/crashtests/unsized-svg.svg
@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg"></svg>
				`@ -0,0 +1 @@`
				`<svg xmlns="http://www.w3.org/2000/svg"></svg>`