Bug 1169680 - Don't merge image cache entries for blob URIs with different refs. r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 05 Jun 2015 01:52:06 -0700
changeset 247347 4f873b18b4bfa16f7678316719f82461440f8880
parent 247346 f644f58217890f083f0235c0deef1ed66e8719db
child 247348 a8d18c6736cc6ef9a1f6dd933a04ef10c4130410
push id28864
push userkwierso@gmail.com
push dateFri, 05 Jun 2015 21:49:37 +0000
treeherdermozilla-central@97a39c939c51 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1169680
milestone41.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 1169680 - Don't merge image cache entries for blob URIs with different refs. r=tn
image/ImageCacheKey.cpp
image/ImageURL.h
image/test/reftest/blob/blob-uri-with-ref-param-notref.html
image/test/reftest/blob/blob-uri-with-ref-param.html
image/test/reftest/blob/image.png
image/test/reftest/blob/reftest.list
image/test/reftest/reftest.list
--- a/image/ImageCacheKey.cpp
+++ b/image/ImageCacheKey.cpp
@@ -81,18 +81,20 @@ ImageCacheKey::ImageCacheKey(ImageCacheK
   , mHash(aOther.mHash)
   , mIsChrome(aOther.mIsChrome)
 { }
 
 bool
 ImageCacheKey::operator==(const ImageCacheKey& aOther) const
 {
   if (mBlobSerial || aOther.mBlobSerial) {
-    // If at least one of us has a blob serial, just compare those.
-    return mBlobSerial == aOther.mBlobSerial;
+    // If at least one of us has a blob serial, just compare the blob serial and
+    // the ref portion of the URIs.
+    return mBlobSerial == aOther.mBlobSerial &&
+           mURI->HasSameRef(*aOther.mURI);
   }
 
   // For non-blob URIs, compare the URIs.
   return *mURI == *aOther.mURI;
 }
 
 const char*
 ImageCacheKey::Spec() const
@@ -104,18 +106,23 @@ ImageCacheKey::Spec() const
 ImageCacheKey::ComputeHash(ImageURL* aURI,
                            const Maybe<uint64_t>& aBlobSerial)
 {
   // Since we frequently call Hash() several times in a row on the same
   // ImageCacheKey, as an optimization we compute our hash once and store it.
 
   if (aBlobSerial) {
     // For blob URIs, we hash the serial number of the underlying blob, so that
-    // different blob URIs which point to the same blob share a cache entry.
-    return HashGeneric(*aBlobSerial);
+    // different blob URIs which point to the same blob share a cache entry. We
+    // also include the ref portion of the URI to support -moz-samplesize and
+    // -moz-resolution, which require us to create different Image objects even
+    // if the source data is the same.
+    nsAutoCString ref;
+    aURI->GetRef(ref);
+    return HashGeneric(*aBlobSerial, HashString(ref));
   }
 
   // For non-blob URIs, we hash the URI spec.
   nsAutoCString spec;
   aURI->GetSpec(spec);
   return HashString(spec);
 }
 
--- a/image/ImageURL.h
+++ b/image/ImageURL.h
@@ -96,16 +96,21 @@ public:
 
   bool operator==(const ImageURL& aOther) const
   {
     // Note that we don't need to consider mScheme and mRef, because they're
     // already represented in mSpec.
     return mSpec == aOther.mSpec;
   }
 
+  bool HasSameRef(const ImageURL& aOther) const
+  {
+    return mRef == aOther.mRef;
+  }
+
 private:
   // Since this is a basic storage class, no duplication of spec parsing is
   // included in the functionality. Instead, the class depends upon the
   // parsing implementation in the nsIURI class used in object construction.
   // This means each field is stored separately, but since only a few are
   // required, this small memory tradeoff for threadsafe usage should be ok.
   nsAutoCString mSpec;
   nsAutoCString mScheme;
new file mode 100644
--- /dev/null
+++ b/image/test/reftest/blob/blob-uri-with-ref-param-notref.html
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+
+<html class="reftest-wait">
+
+<body>
+  <img id="test">
+</body>
+
+<script>
+  var image = new Image;
+
+  image.onload = function() {
+    // Create a canvas.
+    var canvas = document.createElement('canvas');
+    canvas.width = 100;
+    canvas.height = 100;
+
+    // Draw the image into the canvas.
+    var ctx = canvas.getContext('2d');
+    ctx.drawImage(image, 0, 0);
+
+    // Convert the image into a blob URI and use it as #test's src.
+    canvas.toBlob(function(blob) {
+      var uri = window.URL.createObjectURL(blob);
+      uri += '#-moz-samplesize=8';
+      document.getElementById('test').src = uri;
+
+      // Take the snapshot.
+      document.documentElement.removeAttribute('class');
+    }, 'image/jpeg', 0.99);
+  }
+
+  // Start loading the image.
+  image.src = 'image.png';
+</script>
+
+</html>
new file mode 100644
--- /dev/null
+++ b/image/test/reftest/blob/blob-uri-with-ref-param.html
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+
+<html class="reftest-wait">
+
+<body>
+  <img id="test">
+</body>
+
+<script>
+  var image = new Image;
+
+  image.onload = function() {
+    // Create a canvas.
+    var canvas = document.createElement('canvas');
+    canvas.width = 100;
+    canvas.height = 100;
+
+    // Draw the image into the canvas.
+    var ctx = canvas.getContext('2d');
+    ctx.drawImage(image, 0, 0);
+
+    // Convert the image into a blob URI and use it as #test's src.
+    canvas.toBlob(function(blob) {
+      var uri = window.URL.createObjectURL(blob);
+      document.getElementById('test').src = uri;
+
+      // Take the snapshot.
+      document.documentElement.removeAttribute('class');
+    }, 'image/jpeg', 0.99);
+  }
+
+  // Start loading the image.
+  image.src = 'image.png';
+</script>
+
+</html>
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..d7d87adce09ac7655ab4de9ebbb4bc7791bdd739
GIT binary patch
literal 840
zc%17D@N?(olHy`uVBq!ia0vp^DIm<j1SJ1AFfjuuj>O~;A0Q19Vr=PJ{0vA-r9s3P
zU>X=;fDy#D>9i67GEaNDIEGZ*dV4ol=#T=BYhcCi`^B-2xe5=Br{9k{`B612T_#J@
zKXUiHtlE^=Jy&-zoL;wi=3SjN8Vi;dJl^PL?5iT(W_mee+AW4PTYZ#II&J)xDCNBD
z^in5tv59MPXD(7+rV}I>U8?e0VcCKX&AC&OKYOV^)o_$tTE!h0v~pe1l<-x1HI`1x
z((%2yG>S_k=$ldNl)Vw6VP_irMYWwzI-QEM$bFH2c+19L(fcNBs=nAZ&rBuky@<+c
z>&PRo@^^TIU6HkUE!r*j=atK(s}psSQaQv=CLL(r$}Yxy+H9MLST2WHg!<ZDf9`9j
zaO-SPU;q*an7DN~K*WZ;qpSYBNr?Wc7`a6;B0;wOXpgy1`6O5Armeza$*;2$Ca!sB
zrV>-1?7B(f!{iMwj(nF>i981s0os4s@&V^{?xr{VzIv|GZg<m{@?Nig_^LX}N%NM$
zx7U8F3Tj2Q|Eo^&n0fm{m40+%*WNovgL;|&d99ze$mHFn>7J!`tzsTujq7-}=L~C5
z-|S0^_g0y|x66F6=70F_d&z5Se>N}idh$}k+Fj>`?A1K!TVbmTJT7Lfkv-98`7dkX
zt(EMnebsh9IkB&>;JO@}WYYFu%-`AH<+d&TV{7smDe@VC*a(Ux!5AF*??1^W0eLsl
zQXE4-$tL3P+ncBPS``GC177_9Z>*PBlw8wupg}k7&a4Z`hAt}0C+%84Y3=TthDCX)
zH>U(1VGEOA!5@C~Q*%^d#mx}Q1y`>X1k~mx%zVSK!KX~1Lrs}4=5nmYq$jLZq0=u0
z7|dn7HQ8hRs{;ag-b+^1CTi;l9F+QFDm__ZO#-WSg!e|4=|5wVg!UhrRJFHG;;pBM
z=9}{q>Yu)Beak5Dv&qgye^Xu{mvwv7KFM7@%;mGo#91D(UFxW?J38az<0l_yK9o;7
o=2rJ~v4(x#ql2;97ma^Q-xqcY-1^F6D=4*ly85}Sb4q9e00<^u0{{R3
new file mode 100644
--- /dev/null
+++ b/image/test/reftest/blob/reftest.list
@@ -0,0 +1,7 @@
+# Blob URI tests 
+
+# Test that blob URIs don't get merged if they have different ref params.
+# (We run the test twice to check both cached and non-cached cases.)
+default-preferences pref(image.mozsamplesize.enabled,true)
+!= blob-uri-with-ref-param.html blob-uri-with-ref-param-notref.html
+!= blob-uri-with-ref-param.html blob-uri-with-ref-param-notref.html
--- a/image/test/reftest/reftest.list
+++ b/image/test/reftest/reftest.list
@@ -41,10 +41,13 @@ include icon/win/reftest.list
 include generic/reftest.list
 
 # Color management test
 include color-management/reftest.list
 
 # Downscaling tests
 include downscaling/reftest.list
 
+# Blob URI tests
+include blob/reftest.list
+
 # Lossless encoders
 skip-if(Android||B2G) include encoders-lossless/reftest.list # bug 783621