Deactivate cache entry when caching is stopped.

Deactivate cache entry when caching is stopped.

The method HttpCache::Transaction::StopCaching is called when resource download begins. The method deactivates http caching mechanism for the downloaded resource (see mode_ = NONE;).

However, the deactivation is not complete: the resource is still kept in http cache structures.

This causes all requests to this resource (e.g. second download request) to fail, as the resource is still registered as active cache entry (and the request just waits for the cache element to become available, which will never happen because once download finishes, the cache entry is removed).

This fix makes the cache entry inactive, thus making another request for this resource possible.

[joleksy, 2014-02-26 09:24:47]

@reviewer: does this patch look OK to you?

[rvargas (doing something else), 2014-02-27 03:37:41]

Could you open a bug report?

The problem with this patch is that it blindly deletes the entry from the cache.
For instance, the cache may already hold the whole resource, then a download
starts and the download manager decides to stop caching... that should not
delete the entry.

I'm not really happy with not having restrictions on the use of this method
(StopCaching), which prevents us from doing the right thing right away (as the
comment states)... which would be calling DoneWithEntry().

[joleksy, 2014-02-27 08:05:56]

On 2014/02/27 03:37:41, rvargas wrote:
> Could you open a bug report?
> 
> The problem with this patch is that it blindly deletes the entry from the
cache.
> For instance, the cache may already hold the whole resource, then a download
> starts and the download manager decides to stop caching... that should not
> delete the entry.
> 
> I'm not really happy with not having restrictions on the use of this method
> (StopCaching), which prevents us from doing the right thing right away (as the
> comment states)... which would be calling DoneWithEntry().

Bug report opened:

https://code.google.com/p/chromium/issues/detail?id=347458

[joleksy, 2015-02-23 08:16:49]

On 2014/02/27 08:05:56, joleksy wrote:
> On 2014/02/27 03:37:41, rvargas wrote:
> > Could you open a bug report?
> > 
> > The problem with this patch is that it blindly deletes the entry from the
> cache.
> > For instance, the cache may already hold the whole resource, then a download
> > starts and the download manager decides to stop caching... that should not
> > delete the entry.
> > 
> > I'm not really happy with not having restrictions on the use of this method
> > (StopCaching), which prevents us from doing the right thing right away (as
the
> > comment states)... which would be calling DoneWithEntry().
> 
> Bug report opened:
> 
> https://code.google.com/p/chromium/issues/detail?id=347458

Any input on this one? Should it be fixed in some different way?

[rvargas (doing something else), 2015-02-24 02:02:58]

On 2015/02/23 08:16:49, joleksy wrote:
> On 2014/02/27 08:05:56, joleksy wrote:
> > On 2014/02/27 03:37:41, rvargas wrote:
> > > Could you open a bug report?
> > > 
> > > The problem with this patch is that it blindly deletes the entry from the
> > cache.
> > > For instance, the cache may already hold the whole resource, then a
download
> > > starts and the download manager decides to stop caching... that should not
> > > delete the entry.
> > > 
> > > I'm not really happy with not having restrictions on the use of this
method
> > > (StopCaching), which prevents us from doing the right thing right away (as
> the
> > > comment states)... which would be calling DoneWithEntry().
> > 
> > Bug report opened:
> > 
> > https://code.google.com/p/chromium/issues/detail?id=347458
> 
> Any input on this one? Should it be fixed in some different way?

In the big picture, deleting an entry when we could keep it is not that big of a
deal (unless we go into a mode where we do that all the time, which I'm afraid
could happen).

There is no resolution on the bug report, but I would prefer to implement option
1 of comment #8... it is more work, but I think it's cleaner than what we have
today :(.

Makes sense?

[joleksy, 2015-02-24 07:55:13]

On 2015/02/24 02:02:58, rvargas wrote:
> On 2015/02/23 08:16:49, joleksy wrote:
> > On 2014/02/27 08:05:56, joleksy wrote:
> > > On 2014/02/27 03:37:41, rvargas wrote:
> > > > Could you open a bug report?
> > > > 
> > > > The problem with this patch is that it blindly deletes the entry from
the
> > > cache.
> > > > For instance, the cache may already hold the whole resource, then a
> download
> > > > starts and the download manager decides to stop caching... that should
not
> > > > delete the entry.
> > > > 
> > > > I'm not really happy with not having restrictions on the use of this
> method
> > > > (StopCaching), which prevents us from doing the right thing right away
(as
> > the
> > > > comment states)... which would be calling DoneWithEntry().
> > > 
> > > Bug report opened:
> > > 
> > > https://code.google.com/p/chromium/issues/detail?id=347458
> > 
> > Any input on this one? Should it be fixed in some different way?
> 
> In the big picture, deleting an entry when we could keep it is not that big of
a
> deal (unless we go into a mode where we do that all the time, which I'm afraid
> could happen).
> 
> There is no resolution on the bug report, but I would prefer to implement
option
> 1 of comment #8... it is more work, but I think it's cleaner than what we have
> today :(.
> 
> Makes sense?

OK, thanks. I will try to implement the suggested solution.