Make use of network delegate explicit in drive unit tests using TestURLRequest.

Make use of network delegate explicit in drive unit tests using TestURLRequest.

The older interface assumes an implicit relationship between a URLRequestContext
and its delegate.  This interface has been deprecated in favor of making the
network delegate explicit.

BUG=146587
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192977

[tedv, 2013-03-26 03:48:40]

hshi: PTAL.  Let me know if you have questions but it should be pretty
straight-forward.

[hshi1, 2013-03-26 03:50:14]

https://codereview.chromium.org/13079008/diff/1/chrome/browser/chromeos/drive...
File chrome/browser/chromeos/drive/drive_url_request_job_unittest.cc (right):

https://codereview.chromium.org/13079008/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/drive_url_request_job_unittest.cc:49:
delegate_.get(), url_request_context_.get(), network_delegate_.get());
If they do not fit one line, then it is recommended that you list each item on a
separate line, left-aligned.

https://codereview.chromium.org/13079008/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/drive_url_request_job_unittest.cc:66:
delegate_.get(), url_request_context_.get(), network_delegate_.get());
Same as above.

[tedv, 2013-03-26 04:03:29]

Fixed style nits as requested.

[hshi1, 2013-03-26 04:19:19]

LGTM and thanks!

[commit-bot: I haz the power, 2013-03-26 13:24:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedv@chromium.org/13079008/3002

[commit-bot: I haz the power, 2013-03-26 14:22:20]

Retried try job too often on linux_chromeos for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[tedv, 2013-04-04 03:45:32]

Having dug into this code, the unit test failures are generated by the
TestNetworkDelegate internal state machine detecting an invalid state. In
particular, it detects that URLRequestJob using the delegate hasn't called
NotifyBeforeURLRequest() before calling NotifyCompleted(). The standard
URLRequestJob::Start() function calls NotifyBeforeURLRequest() when provided
with a NetworkDelegate, but the DriveURLRequestJob::Start() method does not.

My major question, though, is why the DriveURLRequestJob even wants a network
delegate in the first place.  The class seems to override most of the
URLRequestJob flow control, and only passes its provided network delegate (if
any) onto the parent URLRequestJob class.  I'm hesitant to break it out without
confirmation though, as the unit tests seem sparse right now.

To summarize, it seems the options are one of:

A) DriveURLRequestJob does not need a network delegate, so it should provide a
NULL delegate to URLRequestJob and the unit tests should use NULL as well.
B) DriveURLRequestJob needs a network delegate, so its Start() method should at
the very least call NetworkDelegate::NotifyBeforeURLRequest().
C) DriveURLRequestJob needs a network delegate, but it's really not appropriate
to call NotifyBeforeURLRequest(), so the TestNetworkDelegate class should get
updated to know this is okay.

[tedv, 2013-04-07 20:13:24]

So I've opted for the least risky option of creating the URLRequestContext with
no network delegate (which is functionally identical to the current
implementation), with a comment in the code about why using the
TestNetworkDelegate causes test failures. It's a good thing to fix, but not in
this patch.

[commit-bot: I haz the power, 2013-04-07 21:50:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedv@chromium.org/13079008/30001

[commit-bot: I haz the power, 2013-04-07 22:04:51]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-04-08 13:01:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedv@chromium.org/13079008/30001

[commit-bot: I haz the power, 2013-04-08 13:05:30]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-04-09 00:57:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedv@chromium.org/13079008/30001

[commit-bot: I haz the power, 2013-04-09 00:57:50]

Change committed as 192977