Avoid meta-refresh when back-button gets non-sdch content...

Avoid meta-refresh when back-button gets non-sdch content

The first page search from google will not be SDCH encoded, but will
trigger a background download of a dictionary for future use.
..but..
IF the user navigates forward from the search page, and then back, then
Chromium will fetch the content from cache after specifying in the URL
that a dictionary is now available.   This new logic detects such a
situation where non-SDCH content is pulled from the cache, and avoids
the (slower and overly conservative) meta-refresh.

test=see bug for repro cases.  Check about:histogram/SDCH for error codes.
bug=20457
r=kmixter

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=35318

[kmixter1, 2009-12-28 19:29:02]

LGTM

http://codereview.chromium.org/518016/diff/1/2
File net/base/sdch_filter.cc (right):

http://codereview.chromium.org/518016/diff/1/2#newcode154
net/base/sdch_filter.cc:154: // This is where we try very hard to do error
recovery, and make this
Incomplete comment.

http://codereview.chromium.org/518016/diff/1/2#newcode171
net/base/sdch_filter.cc:171: } else if (filter_context().IsCachedContent()
Do we still have the original request headers (those that resulted in this
cached response)?  If so then it seem the ideal solution would be to only add
the SDCH filter if the original request accepted it.  This seems like the best
alternative short of that.

http://codereview.chromium.org/518016/diff/1/3
File net/base/sdch_manager.h (right):

http://codereview.chromium.org/518016/diff/1/3#newcode126
net/base/sdch_manager.h:126: PASS_THROUGH_OLD_CACHED = 79,   // Back button got
pre-SDCH cached content.
Maybe worth commenting that unlike most of the problem codes here, this one is
expected through normal browser use.  As such, I guess this is mostly useful in
manual testing that the code is executing and less for field metrics collection.

[kmixter1, 2009-12-28 23:02:36]

LGTM

http://codereview.chromium.org/518016/diff/1/2
File net/base/sdch_filter.cc (right):

http://codereview.chromium.org/518016/diff/1/2#newcode154
net/base/sdch_filter.cc:154: // This is where we try very hard to do error
recovery, and make this
On 2009/12/28 19:29:02, kmixter1 wrote:
> Incomplete comment.
Sorry - I misread this.  The comment looks fine.

[jar (doing other things), 2009-12-28 23:05:05]

Changes per comments by kmixter.

http://codereview.chromium.org/518016/diff/1/2
File net/base/sdch_filter.cc (right):

http://codereview.chromium.org/518016/diff/1/2#newcode171
net/base/sdch_filter.cc:171: } else if (filter_context().IsCachedContent()
The original request headers are not saved into the cache :-(.  I agree, if we
had the headers, the comparison would be the way to go.  I don't recall all the
reasons, but I recall that there were reasons why the request headers were not
saved (I raised this point with darin and/or ricardo).

On 2009/12/28 19:29:02, kmixter1 wrote:
> Do we still have the original request headers (those that resulted in this
> cached response)?  If so then it seem the ideal solution would be to only add
> the SDCH filter if the original request accepted it.  This seems like the best
> alternative short of that.

http://codereview.chromium.org/518016/diff/1/3
File net/base/sdch_manager.h (right):

http://codereview.chromium.org/518016/diff/1/3#newcode126
net/base/sdch_manager.h:126: PASS_THROUGH_OLD_CACHED = 79,   // Back button got
pre-SDCH cached content.
On 2009/12/28 19:29:02, kmixter1 wrote:
> Maybe worth commenting that unlike most of the problem codes here, this one is
> expected through normal browser use.  As such, I guess this is mostly useful
in
> manual testing that the code is executing and less for field metrics
collection.

Done.