Issue 3491 - Fix cache handling for blockable items tooltip

chrome/content/ui/sidebar.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 331 matching lines @@
 
   var showPreview = Prefs.previewimages && !("tooltip" in item);
   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);
     }
   }
 }
 
 const visual = {
   OTHER: true,
@@ skipping 925 matching lines @@
       return {tooltip: this.itemsDummyTooltip};
   },
 
   invalidateItem: function(item)
   {
     let row = this.data.indexOf(item);
     if (row >= 0)
       this.boxObject.invalidateRow(row);
   }
 }