Bug 1205923: Make VectorImage::GetWidth/GetHeight leave outparam untouched on failure, to avoid returning -1 to callers that don't check error codes. r=seth
☠☠ backed out by 3e4168e0a457 ☠ ☠
authorDaniel Holbert <dholbert@cs.stanford.edu>
Fri, 18 Sep 2015 15:33:43 -0700
changeset 295990 757560ab8de1841caa4d7a487f6a9288823fc7fd
parent 295989 909b9b147e71d707f4d0c4cf33822d36926af816
child 295991 0e0f3104478fcb2ed29be54f3e5ee113245378b8
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseth
bugs1205923
milestone43.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 1205923: Make VectorImage::GetWidth/GetHeight leave outparam untouched on failure, to avoid returning -1 to callers that don't check error codes. r=seth
image/VectorImage.cpp
image/test/crashtests/1205923-1.html
image/test/crashtests/crashtests.list
image/test/crashtests/unsized-svg.svg
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -492,24 +492,28 @@ VectorImage::SetAnimationStartTime(const
 //------------------------------------------------------------------------------
 // imgIContainer methods
 
 //******************************************************************************
 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();
+    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");
+  nscoord rootElemWidth = rootElem->GetIntrinsicWidth();
+  if (rootElemWidth < 0) {
+    return NS_ERROR_FAILURE;
+  }
+  *aWidth = rootElemWidth;
+  return NS_OK;
 }
 
 //******************************************************************************
 NS_IMETHODIMP_(void)
 VectorImage::RequestRefresh(const TimeStamp& aTime)
 {
   if (HadRecentRefresh(aTime)) {
     return;
@@ -556,24 +560,28 @@ VectorImage::GetImageSpaceInvalidationRe
   return aRect;
 }
 
 //******************************************************************************
 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();
+    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");
+  nscoord rootElemHeight = rootElem->GetIntrinsicHeight();
+  if (rootElemHeight < 0) {
+    return NS_ERROR_FAILURE;
+  }
+  *aHeight = rootElemHeight;
+  return NS_OK;
 }
 
 //******************************************************************************
 NS_IMETHODIMP
 VectorImage::GetIntrinsicSize(nsSize* aSize)
 {
   if (mError || !mIsFullyLoaded) {
     return NS_ERROR_FAILURE;
new file mode 100644
--- /dev/null
+++ b/image/test/crashtests/1205923-1.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<body>
+</body>
+<script>
+  function createImage() {
+    var newImage = new Image;
+    newImage.id = "thepreviewimage";
+    newImage.setAttribute("src", "unsized-svg.svg");
+    physWidth = newImage.width || 0;
+    physHeight = newImage.height || 0;
+    document.documentElement.innerHTML += newImage.width + " x " + newImage.height + "<br>";
+  }
+  function createImageAndEnd() {
+    createImage();
+    document.documentElement.removeAttribute("class");
+  }
+
+  // Trigger image load:
+  createImage();
+
+  // Asynchronously trigger image load again (hitting cache this time):
+  setTimeout(createImageAndEnd, 0);
+</script>
+</html>
--- a/image/test/crashtests/crashtests.list
+++ b/image/test/crashtests/crashtests.list
@@ -47,8 +47,10 @@ load multiple-png-hassize.ico
 load 856616.gif
 
 skip-if(AddressSanitizer) skip-if(B2G) load 944353.jpg
 
 # Bug 1160801: Ensure that we handle invalid disposal types.
 load invalid-disposal-method-1.gif
 load invalid-disposal-method-2.gif
 load invalid-disposal-method-3.gif
+
+load 1205923-1.html
new file mode 100644
--- /dev/null
+++ b/image/test/crashtests/unsized-svg.svg
@@ -0,0 +1,1 @@
+<svg xmlns="http://www.w3.org/2000/svg"></svg>