Remove URLRequestJob::GetResponseCode implementations

Remove URLRequestJob::GetResponseCode implementations
from streams and fileapi code.

URLRequestJob itself now has a default implementation, which gets
the response code from the headers returned by GetResponseInfo.
We eventually want to remove this method, since it's redundant,
and has weird behavior for just when it is and is not safe to
call.

BUG=688481
Review-Url: https://codereview.chromium.org/2822593003
Cr-Commit-Position: refs/heads/master@{#465588}
Committed: https://chromium.googlesource.com/chromium/src/+/f13168e3f095f995d58ac9baf810a71531fd10bf

[mmenke, 2017-04-14 21:38:02]

Description was changed from

==========
Remove URLRequestJob::GetResponseCode implementations
from streams and fileapi code.

We want to remove this method, in favor of just getting the
value from the result from GetResponseInfo().  This ensures
consistency between headers and response code, and removes
redundant code in a bunch of subclasses.

BUG=688481
==========

to

==========
Remove URLRequestJob::GetResponseCode implementations
from streams and fileapi code.

URLRequestJob itself now has a default implementation, which gets
the response code from the headers returned by GetResponseInfo.
We eventually want to remove this method, since it's redundant,
and has weird behavior for just when it is and is not safe to
call.

BUG=688481
==========

[mmenke, 2017-04-14 21:38:13]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-14 21:38:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-14 21:47:32]

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

[commit-bot: I haz the power, 2017-04-14 21:47:33]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2017-04-17 16:53:14]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 16:53:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 17:02:42]

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

[commit-bot: I haz the power, 2017-04-17 17:02:43]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2017-04-17 17:17:37]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 17:17:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2017-04-17 18:00:32]

Patchset #2 (id:20001) has been deleted

[mmenke, 2017-04-17 18:03:28]

mmenke@chromium.org changed reviewers:
+ nhiroki@chromium.org, tyoshino@chromium.org

[mmenke, 2017-04-17 18:03:29]

[tyoshino]:  Please review content/browser/streams.
[nhiroki]: Please review storage/browser/fileapi.

Per description, the data returned by GetResponseCode() is redundant with
GetResponseInfo(), so I'm planning on removing it.  GetResponseInfo() is heavier
weight, but a higher layer caches the GetResponseInfo, anyways, so this
shouldn't reduce performance.

[commit-bot: I haz the power, 2017-04-17 18:14:03]

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

[commit-bot: I haz the power, 2017-04-17 18:14:04]

Dry run: This issue passed the CQ dry run.

[tyoshino (SeeGerritForStatus), 2017-04-18 05:06:55]

https://codereview.chromium.org/2822593003/diff/40001/content/browser/streams...
File content/browser/streams/stream_url_request_job.cc (left):

https://codereview.chromium.org/2822593003/diff/40001/content/browser/streams...
content/browser/streams/stream_url_request_job.cc:147: 
OK, but it looks URLRequestJob looks at URLRequest::response_info_ which
StreamURLRequestJob doesn't update. What's the new recommended way to update
URLRequest::response_info_ that maybe we should do in
StreamURLRequestJob::HeadersCompleted?

[mmenke, 2017-04-18 06:43:30]

On 2017/04/18 05:06:55, tyoshino wrote:
>
https://codereview.chromium.org/2822593003/diff/40001/content/browser/streams...
> File content/browser/streams/stream_url_request_job.cc (left):
> 
>
https://codereview.chromium.org/2822593003/diff/40001/content/browser/streams...
> content/browser/streams/stream_url_request_job.cc:147: 
> OK, but it looks URLRequestJob looks at URLRequest::response_info_ which
> StreamURLRequestJob doesn't update. What's the new recommended way to update
> URLRequest::response_info_ that maybe we should do in
> StreamURLRequestJob::HeadersCompleted?

The URLRequestJob updates it itself, as needed.  It calls
StreamURLRequestJob::GetResponseInfo itself.  Pull-based, not push-based.

[tyoshino (SeeGerritForStatus), 2017-04-18 06:46:29]

On 2017/04/18 06:43:30, mmenke wrote:
> On 2017/04/18 05:06:55, tyoshino wrote:
> >
>
https://codereview.chromium.org/2822593003/diff/40001/content/browser/streams...
> > File content/browser/streams/stream_url_request_job.cc (left):
> > 
> >
>
https://codereview.chromium.org/2822593003/diff/40001/content/browser/streams...
> > content/browser/streams/stream_url_request_job.cc:147: 
> > OK, but it looks URLRequestJob looks at URLRequest::response_info_ which
> > StreamURLRequestJob doesn't update. What's the new recommended way to update
> > URLRequest::response_info_ that maybe we should do in
> > StreamURLRequestJob::HeadersCompleted?
> 
> The URLRequestJob updates it itself, as needed.  It calls
> StreamURLRequestJob::GetResponseInfo itself.  Pull-based, not push-based.

Oh, sorry! I wasn't aware of that.

lgtm

[mmenke, 2017-04-18 12:26:46]

On 2017/04/18 06:46:29, tyoshino wrote:
> On 2017/04/18 06:43:30, mmenke wrote:
> > On 2017/04/18 05:06:55, tyoshino wrote:
> > >
> >
>
https://codereview.chromium.org/2822593003/diff/40001/content/browser/streams...
> > > File content/browser/streams/stream_url_request_job.cc (left):
> > > 
> > >
> >
>
https://codereview.chromium.org/2822593003/diff/40001/content/browser/streams...
> > > content/browser/streams/stream_url_request_job.cc:147: 
> > > OK, but it looks URLRequestJob looks at URLRequest::response_info_ which
> > > StreamURLRequestJob doesn't update. What's the new recommended way to
update
> > > URLRequest::response_info_ that maybe we should do in
> > > StreamURLRequestJob::HeadersCompleted?
> > 
> > The URLRequestJob updates it itself, as needed.  It calls
> > StreamURLRequestJob::GetResponseInfo itself.  Pull-based, not push-based.
> 
> Oh, sorry! I wasn't aware of that.
> 
> lgtm

Not a problem!  Thanks for the dilligence!

[nhiroki, 2017-04-19 09:55:07]

Sorry for my delayed response. LGTM

[mmenke, 2017-04-19 13:08:59]

On 2017/04/19 09:55:07, nhiroki wrote:
> Sorry for my delayed response. LGTM

Not a problem, no rush on this on.

Thank you both.

[mmenke, 2017-04-19 13:09:03]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2017-04-19 13:09:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-19 14:14:06]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492607343889330,
"parent_rev": "db0751b8db4b09ffba0d7907dcb93311b2c596ec", "commit_rev":
"f13168e3f095f995d58ac9baf810a71531fd10bf"}

[commit-bot: I haz the power, 2017-04-19 14:14:14]

Description was changed from

==========
Remove URLRequestJob::GetResponseCode implementations
from streams and fileapi code.

URLRequestJob itself now has a default implementation, which gets
the response code from the headers returned by GetResponseInfo.
We eventually want to remove this method, since it's redundant,
and has weird behavior for just when it is and is not safe to
call.

BUG=688481
==========

to

==========
Remove URLRequestJob::GetResponseCode implementations
from streams and fileapi code.

URLRequestJob itself now has a default implementation, which gets
the response code from the headers returned by GetResponseInfo.
We eventually want to remove this method, since it's redundant,
and has weird behavior for just when it is and is not safe to
call.

BUG=688481

Review-Url: https://codereview.chromium.org/2822593003
Cr-Commit-Position: refs/heads/master@{#465588}
Committed:
https://chromium.googlesource.com/chromium/src/+/f13168e3f095f995d58ac9baf810...
==========

[commit-bot: I haz the power, 2017-04-19 14:14:15]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/f13168e3f095f995d58ac9baf810...