Use URLRequestJobManager to retry data reduction proxy bypasses

Use URLRequestJobManager to retry data reduction proxy bypasses

The data reduction proxy retries a bypassed request by creating
a new URLRequestHttpJob for the request and starting it. This
change uses URLRequestJobManager::CreateJob() to create that job.
Previously, the URLRequestJobFactory chain was called to create
the job, an it only would for main frame requests.

BUG=434830
Committed: https://crrev.com/09d0aae07de9e0a68752dde74a3c204429ab24d9
Cr-Commit-Position: refs/heads/master@{#305054}

[bengr, 2014-11-20 00:26:28]

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

[marq (ping after 24h), 2014-11-20 00:29:14]

LGTM but the classes being called aren't ones I'm familiar with so my opinion is
of limited utility.

[bengr, 2014-11-20 00:29:50]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-11-20 00:30:51]

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

[commit-bot: I haz the power, 2014-11-20 00:34:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-20 00:34:36]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/83821)

[bengr, 2014-11-20 01:47:27]

Patchset #2 (id:20001) has been deleted

[bengr, 2014-11-20 02:10:39]

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

[bengr, 2014-11-20 02:10:39]

mmenke: net/*

Also, I know of know way to test this behavior given our current fixture. Could
you suggest a way? Regardless, I'd like to land this ASAP because it is a
critical bug fix and fill in the tests afterwards.

[bengr, 2014-11-20 02:10:49]

Patchset #2 (id:40001) has been deleted

[bengr, 2014-11-20 02:20:41]

On 2014/11/20 02:10:39, bengr wrote:
> mmenke: net/*
> 
> Also, I know of know way to test this behavior given our current fixture.
Could
> you suggest a way? Regardless, I'd like to land this ASAP because it is a
> critical bug fix and fill in the tests afterwards.

I mean "no way"

[mmenke, 2014-11-20 15:48:39]

On 2014/11/20 02:20:41, bengr wrote:
> On 2014/11/20 02:10:39, bengr wrote:
> > mmenke: net/*
> > 
> > Also, I know of know way to test this behavior given our current fixture.
> Could
> > you suggest a way? Regardless, I'd like to land this ASAP because it is a
> > critical bug fix and fill in the tests afterwards.
> 
> I mean "no way"

You can't do it with using interceptors to mock out the original request -
they're hooked up to the URLRequestJobFactories, not the URLRequestJobManager. 
You have to use a real test server.  The embedded test server can serve up files
from a directory (With an optional second file to contain the headers). 
net/url_request/url_request_unittest is one example of this - though they use
the spawned test server, the embedded one works the same way (Embedded one is
more robust, and starts up faster [Slowness of starting spawned one has caused
browser test failures before]).

[mmenke, 2014-11-20 16:41:08]

On 2014/11/20 15:48:39, mmenke wrote:
> On 2014/11/20 02:20:41, bengr wrote:
> > On 2014/11/20 02:10:39, bengr wrote:
> > > mmenke: net/*
> > > 
> > > Also, I know of know way to test this behavior given our current fixture.
> > Could
> > > you suggest a way? Regardless, I'd like to land this ASAP because it is a
> > > critical bug fix and fill in the tests afterwards.
> > 
> > I mean "no way"
> 
> You can't do it with using interceptors to mock out the original request -
> they're hooked up to the URLRequestJobFactories, not the URLRequestJobManager.

> You have to use a real test server.  The embedded test server can serve up
files
> from a directory (With an optional second file to contain the headers). 
> net/url_request/url_request_unittest is one example of this - though they use
> the spawned test server, the embedded one works the same way (Embedded one is
> more robust, and starts up faster [Slowness of starting spawned one has caused
> browser test failures before]).

Oh, and net/ LGTM.  The manager should go away soon, so not too concerned about
exposing more stuff in net/.

[mmenke, 2014-11-20 16:46:09]

Oops...forgot to publish my comment.

https://codereview.chromium.org/740643003/diff/60001/net/url_request/url_requ...
File net/url_request/url_request_job_manager.h (right):

https://codereview.chromium.org/740643003/diff/60001/net/url_request/url_requ...
net/url_request/url_request_job_manager.h:27: class NET_EXPORT
URLRequestJobManager {
include net/base/net_export.h (Or something like that)

[bengr, 2014-11-20 17:30:38]

https://codereview.chromium.org/740643003/diff/60001/net/url_request/url_requ...
File net/url_request/url_request_job_manager.h (right):

https://codereview.chromium.org/740643003/diff/60001/net/url_request/url_requ...
net/url_request/url_request_job_manager.h:27: class NET_EXPORT
URLRequestJobManager {
On 2014/11/20 16:46:09, mmenke wrote:
> include net/base/net_export.h (Or something like that)

Done.

[bengr, 2014-11-20 18:08:24]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-11-20 18:10:06]

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

[commit-bot: I haz the power, 2014-11-20 19:00:13]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2014-11-20 19:01:49]

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