Issue 3491 - Fix cache handling for blockable items tooltip

This fixes three issues:
1) We were using the wrong load context all along. We got the load context from the window, yet the tooltip will always load images from default cache (i.e. not private browsing cache).
2) Checking isNew in onCacheEntryAvailable isn`t sufficient to recognize whether an entry exists. If a cache entry doesn`t exist isNew will be false and status will be an error code.
3) The fallback code for Firefox 31 and below is no longer needed.
For reference, the point of this code is preventing the tooltip from triggering network requests (privacy). So the image should only be shown if the request can be satisfied from cache.

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

https://codereview.adblockplus.org/29333327/diff/29333328/chrome/content/ui/s...
File chrome/content/ui/sidebar.js (right):

https://codereview.adblockplus.org/29333327/diff/29333328/chrome/content/ui/s...
chrome/content/ui/sidebar.js:353: cacheStorage =
Services.cache2.diskCacheStorage(LoadContextInfo.default, false);
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]

https://codereview.adblockplus.org/29333327/diff/29333328/chrome/content/ui/s...
File chrome/content/ui/sidebar.js (right):

https://codereview.adblockplus.org/29333327/diff/29333328/chrome/content/ui/s...
chrome/content/ui/sidebar.js:353: cacheStorage =
Services.cache2.diskCacheStorage(LoadContextInfo.default, false);
On 2016/01/08 13:43:49, Thomas Greiner wrote:
> 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).

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]

LGTM

https://codereview.adblockplus.org/29333327/diff/29333328/chrome/content/ui/s...
File chrome/content/ui/sidebar.js (right):

https://codereview.adblockplus.org/29333327/diff/29333328/chrome/content/ui/s...
chrome/content/ui/sidebar.js:353: cacheStorage =
Services.cache2.diskCacheStorage(LoadContextInfo.default, false);
On 2016/01/08 13:51:33, Wladimir Palant wrote:
> On 2016/01/08 13:43:49, Thomas Greiner wrote:
> > 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).
> 
> 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).

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.