Issue 3491 - Fix cache handling for blockable items tooltip

chrome/content/ui/sidebar.js

Index: chrome/content/ui/sidebar.js
===================================================================
--- a/chrome/content/ui/sidebar.js
+++ b/chrome/content/ui/sidebar.js
@@ -344,65 +344,38 @@ function fillInTooltip(e) {
   showPreview = showPreview && item.type == "IMAGE";
   showPreview = showPreview && (!filter || filter.disabled || filter instanceof WhitelistFilter);
   E("tooltipPreviewBox").hidden = true;
   if (showPreview)
   {
     if (!cacheStorage)
     {
       let {Services} = Cu.import("resource://gre/modules/Services.jsm", null);
-      // Cache v2 API is enabled by default starting with Gecko 32
-      if (Services.vc.compare(Utils.platformVersion, "32.0a1") >= 0)
-      {
-        let {LoadContextInfo} = Cu.import("resource://gre/modules/LoadContextInfo.jsm", null);
-        let loadContext = content.QueryInterface(Ci.nsIInterfaceRequestor)
-                                 .getInterface(Ci.nsIWebNavigation)
-                                 .QueryInterface(Ci.nsILoadContext);
-        cacheStorage = Services.cache2.diskCacheStorage(LoadContextInfo.fromLoadContext(loadContext, false), false);
-      }
-      else
-        cacheStorage = Services.cache.createSession("HTTP", Ci.nsICache.STORE_ANYWHERE, true);
+      let {LoadContextInfo} = Cu.import("resource://gre/modules/LoadContextInfo.jsm", null);
+      cacheStorage = Services.cache2.diskCacheStorage(LoadContextInfo.default, false);

[Thomas Greiner, 2016/01/08 13:43:49]

I noticed that we intentionally decided not to go with `LoadContextInfo.default`
in #660 to consider private browsing sessions (see
https://issues.adblockplus.org/ticket/660#comment:13).

Is that no longer an issue at this point? Because it seems that we could use
`LoadContextInfo.private` instead (or alternatively `LoadContextInfo.custom()`
to construct our own).

[Wladimir Palant, 2016/01/08 13:51:33]

Please see my explanations in the description of this review - not going with
LoadContextInfo.default was actually the wrong thing to do. The tooltip we are
showing here isn't part of the private browsing session, it rather belongs into
LoadContextInfo.default. So it doesn't matter whether we are in a private
browsing window, if the request isn't in the regular cache then showing the
image in the tooltip will trigger a request (yes, I tested this).

[Thomas Greiner, 2016/01/08 14:12:26]

The review description mentioned that we were using the wrong context, not that
`LoadContextInfo.default` was the right choice after all which is why I wanted
to double-check that.

Anyway, it seems to be appropriate if we don't need to consider private browsing
so fine with me. Thanks for the details on that.

     }
 
     let showTooltipPreview = function ()
     {
       E("tooltipPreview").setAttribute("src", item.location);
       E("tooltipPreviewBox").hidden = false;
     };
     try
     {
-      if (Ci.nsICacheStorage && cacheStorage instanceof Ci.nsICacheStorage)
-      {
-        cacheStorage.asyncOpenURI(Utils.makeURI(item.location), "", Ci.nsICacheStorage.OPEN_READONLY, {
-          onCacheEntryCheck: function (entry, appCache)
-          {
-            return Ci.nsICacheEntryOpenCallback.ENTRY_WANTED;
-          },
-          onCacheEntryAvailable: function (entry, isNew, appCache, status)
-          {
-            if (!isNew)
-              showTooltipPreview();
-          }
-        });
-      }
-      else
-      {
-        cacheStorage.asyncOpenCacheEntry(item.location, Ci.nsICache.ACCESS_READ, {
-          onCacheEntryAvailable: function(descriptor, accessGranted, status)
-          {
-            if (!descriptor)
-              return;
-            descriptor.close();
+      cacheStorage.asyncOpenURI(Utils.makeURI(item.location), "", Ci.nsICacheStorage.OPEN_READONLY, {
+        onCacheEntryCheck: function (entry, appCache)
+        {
+          return Ci.nsICacheEntryOpenCallback.ENTRY_WANTED;
+        },
+        onCacheEntryAvailable: function (entry, isNew, appCache, status)
+        {
+          if (Components.isSuccessCode(status) && !isNew)
             showTooltipPreview();
-          },
-          onCacheEntryDoomed: function(status)
-          {
-          }
-        });
-      }
+        }
+      });
     }
     catch (e)
     {
       Cu.reportError(e);
     }
   }
 }