AppCacheUpdateJob allows data reduction proxy bypass redirects

AppCacheUpdateJob allows data reduction proxy bypass redirects

The AppCacheUpdateJob::URLFetcher cancels redirects in general.
This change allows redirects that are generated by the data
reduction proxy bypass mechanism. This is a workaround until
crbug.com/429505 is resolved.

BUG=429011
Committed: https://crrev.com/b4072eae8e50f4e0c54525241d50be6ef73a96d3
Cr-Commit-Position: refs/heads/master@{#302558}

[bengr, 2014-11-02 18:34:05]

bengr@chromium.org changed reviewers:
+ michaeln@chromium.org

[bengr, 2014-11-02 18:34:05]

This is a workaround that I hope to revert as soon as http://crbug.com/429505 is
fixed (which should be before the M40 branch).

[michaeln, 2014-11-03 21:37:20]

https://codereview.chromium.org/695193002/diff/1/content/browser/appcache/app...
File content/browser/appcache/appcache_update_job.cc (right):

https://codereview.chromium.org/695193002/diff/1/content/browser/appcache/app...
content/browser/appcache/appcache_update_job.cc:163: if ((request->load_flags()
& net::LOAD_BYPASS_PROXY) &&
Is the 'redirect' to the same url in this case?

[bengr, 2014-11-03 22:58:24]

https://codereview.chromium.org/695193002/diff/1/content/browser/appcache/app...
File content/browser/appcache/appcache_update_job.cc (right):

https://codereview.chromium.org/695193002/diff/1/content/browser/appcache/app...
content/browser/appcache/appcache_update_job.cc:163: if ((request->load_flags()
& net::LOAD_BYPASS_PROXY) &&
On 2014/11/03 21:37:20, michaeln wrote:
> Is the 'redirect' to the same url in this case?

Yes

[michaeln1, 2014-11-03 23:11:11]

michaeln@google.com changed reviewers:
+ michaeln@google.com

[michaeln1, 2014-11-03 23:11:12]

lgtm as a stopgap for now

https://codereview.chromium.org/695193002/diff/1/content/browser/appcache/app...
File content/browser/appcache/appcache_update_job.cc (right):

https://codereview.chromium.org/695193002/diff/1/content/browser/appcache/app...
content/browser/appcache/appcache_update_job.cc:28:
proxy_server.Equals(net::HostPortPair("proxy-dev.googlezip.net", 80))) {
i hope this isn't here for long

https://codereview.chromium.org/695193002/diff/1/content/browser/appcache/app...
content/browser/appcache/appcache_update_job.cc:163: if ((request->load_flags()
& net::LOAD_BYPASS_PROXY) &&
On 2014/11/03 22:58:24, bengr wrote:
> On 2014/11/03 21:37:20, michaeln wrote:
> > Is the 'redirect' to the same url in this case?
> 
> Yes

Can you add that info to the comment and it'd be great to
DCHECK_EQ(original_url(), url()).

[bengr, 2014-11-03 23:48:32]

https://codereview.chromium.org/695193002/diff/1/content/browser/appcache/app...
File content/browser/appcache/appcache_update_job.cc (right):

https://codereview.chromium.org/695193002/diff/1/content/browser/appcache/app...
content/browser/appcache/appcache_update_job.cc:28:
proxy_server.Equals(net::HostPortPair("proxy-dev.googlezip.net", 80))) {
On 2014/11/03 23:11:12, michaeln1 wrote:
> i hope this isn't here for long

Acknowledged.

https://codereview.chromium.org/695193002/diff/1/content/browser/appcache/app...
content/browser/appcache/appcache_update_job.cc:163: if ((request->load_flags()
& net::LOAD_BYPASS_PROXY) &&
On 2014/11/03 23:11:12, michaeln1 wrote:
> On 2014/11/03 22:58:24, bengr wrote:
> > On 2014/11/03 21:37:20, michaeln wrote:
> > > Is the 'redirect' to the same url in this case?
> > 
> > Yes
> 
> Can you add that info to the comment and it'd be great to
> DCHECK_EQ(original_url(), url()).

Done.

[michaeln, 2014-11-03 23:51:31]

lgtm @chromium too

https://codereview.chromium.org/695193002/diff/20001/content/browser/appcache...
File content/browser/appcache/appcache_update_job.cc (right):

https://codereview.chromium.org/695193002/diff/20001/content/browser/appcache...
content/browser/appcache/appcache_update_job.cc:31: return false;
this could be simplified as

return x || y ||z;

[bengr, 2014-11-04 01:09:05]

The CQ bit was checked by bengr@chromium.org

[bengr, 2014-11-04 01:09:30]

The CQ bit was unchecked by bengr@chromium.org

[bengr, 2014-11-04 01:12:03]

https://codereview.chromium.org/695193002/diff/20001/content/browser/appcache...
File content/browser/appcache/appcache_update_job.cc (right):

https://codereview.chromium.org/695193002/diff/20001/content/browser/appcache...
content/browser/appcache/appcache_update_job.cc:31: return false;
On 2014/11/03 23:51:31, michaeln wrote:
> this could be simplified as
> 
> return x || y ||z;

Done.

[bengr, 2014-11-04 01:59:25]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-11-04 02:01:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/695193002/40001

[commit-bot: I haz the power, 2014-11-04 02:50:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-04 02:51:24]

Patchset 3 (id:??) landed as
https://crrev.com/b4072eae8e50f4e0c54525241d50be6ef73a96d3
Cr-Commit-Position: refs/heads/master@{#302558}