Make the download's response headers available in the DownloadItem

Make the download's response headers available in the DownloadItem

Background Fetch will give this developer back to the developer, and use
it to determine whether the request was "ok". That information will also
be used to determine which Service Worker event to invoke.

The response headers will not be stored in the download database, so
requires the download to either have been started or resumed in the
current session.

BUG=709489
Review-Url: https://codereview.chromium.org/2809953002
Cr-Commit-Position: refs/heads/master@{#463982}
Committed: https://chromium.googlesource.com/chromium/src/+/87a42b7af9035be518adf40266e4a3b1463186fd

[Peter Beverloo, 2017-04-11 15:09:05]

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

[commit-bot: I haz the power, 2017-04-11 15:09:21]

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

[Peter Beverloo, 2017-04-11 15:10:23]

peter@chromium.org changed reviewers:
+ dtrainor@chromium.org, shaktisahu@chromium.org

[Peter Beverloo, 2017-04-11 15:10:24]

+dtrainor PTAL
+shaktisahu FYI

I'll do headers separately since there's some trickery there -- I'm thinking I
may want to special case CORS to utilize the HttpResponseHeaders parsing
abilities, where the rest can just be passed through to the developer.

https://codereview.chromium.org/2809953002/diff/1/content/browser/download/do...
File content/browser/download/download_item_impl.cc (right):

https://codereview.chromium.org/2809953002/diff/1/content/browser/download/do...
content/browser/download/download_item_impl.cc:1043: if (content_disposition_ !=
new_create_info.content_disposition)
It feels to me like the response code should be part of this, but it might
trigger for downloads resumed after a browser restart which don't have a
response_code_ since we don't store it. WDYT?

[commit-bot: I haz the power, 2017-04-11 15:35:44]

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

[commit-bot: I haz the power, 2017-04-11 15:35:45]

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

[Peter Beverloo, 2017-04-11 16:30:09]

Description was changed from

==========
Make the download's response code available in the DownloadItem

Background Fetch will give this developer back to the developer, and use
it to determine whether the request was "ok". That information will also
be used to determine which Service Worker event to invoke.

The response code will not be stored in the download database, so
requires the download to either have been started or resumed in the
current session.

BUG=709489
==========

to

==========
Make the download's response headers available in the DownloadItem

Background Fetch will give this developer back to the developer, and use
it to determine whether the request was "ok". That information will also
be used to determine which Service Worker event to invoke.

The response headers will not be stored in the download database, so
requires the download to either have been started or resumed in the
current session.

BUG=709489
==========

[Peter Beverloo, 2017-04-11 16:30:17]

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

[commit-bot: I haz the power, 2017-04-11 16:30:36]

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-11 18:12:25]

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

[commit-bot: I haz the power, 2017-04-11 18:12:26]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2017-04-11 22:06:40]

lgtm % comment nit

https://codereview.chromium.org/2809953002/diff/20001/content/public/browser/...
File content/public/browser/download_item.h (right):

https://codereview.chromium.org/2809953002/diff/20001/content/public/browser/...
content/public/browser/download_item.h:255: virtual const scoped_refptr<const
net::HttpResponseHeaders>&
Maybe mention that this isn't persisted, so it will contain the headers for the
current response, if any, in this session.  Not necessarily the original headers
for the response.

[Peter Beverloo, 2017-04-11 22:22:27]

peter@chromium.org changed reviewers:
+ nick@chromium.org

[Peter Beverloo, 2017-04-11 22:22:27]

+nick for //content/public changes

Thanks David!

https://codereview.chromium.org/2809953002/diff/20001/content/public/browser/...
File content/public/browser/download_item.h (right):

https://codereview.chromium.org/2809953002/diff/20001/content/public/browser/...
content/public/browser/download_item.h:255: virtual const scoped_refptr<const
net::HttpResponseHeaders>&
On 2017/04/11 22:06:40, David Trainor-ping if over 24h wrote:
> Maybe mention that this isn't persisted, so it will contain the headers for
the
> current response, if any, in this session.  Not necessarily the original
headers
> for the response.

Done.

[Peter Beverloo, 2017-04-11 22:22:37]

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

[commit-bot: I haz the power, 2017-04-11 22:23:22]

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-11 23:27:13]

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

[commit-bot: I haz the power, 2017-04-11 23:27:14]

Dry run: This issue passed the CQ dry run.

[Peter Beverloo, 2017-04-12 00:36:54]

peter@chromium.org changed reviewers:
+ kinuko@chromium.org
- nick@chromium.org

[Peter Beverloo, 2017-04-12 00:36:54]

nick@ is OOO, +kinuko maybe?

[kinuko, 2017-04-12 02:09:12]

lgtm

[Peter Beverloo, 2017-04-12 10:05:13]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2017-04-12 10:05:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2809953002/#ps40001
(title: "comment")

[commit-bot: I haz the power, 2017-04-12 10:05:38]

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-12 10:11:02]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491991513466210,
"parent_rev": "0e2585ff6d2ab84636f89e40fb60b2e50263d534", "commit_rev":
"87a42b7af9035be518adf40266e4a3b1463186fd"}

[commit-bot: I haz the power, 2017-04-12 10:11:55]

Description was changed from

==========
Make the download's response headers available in the DownloadItem

Background Fetch will give this developer back to the developer, and use
it to determine whether the request was "ok". That information will also
be used to determine which Service Worker event to invoke.

The response headers will not be stored in the download database, so
requires the download to either have been started or resumed in the
current session.

BUG=709489
==========

to

==========
Make the download's response headers available in the DownloadItem

Background Fetch will give this developer back to the developer, and use
it to determine whether the request was "ok". That information will also
be used to determine which Service Worker event to invoke.

The response headers will not be stored in the download database, so
requires the download to either have been started or resumed in the
current session.

BUG=709489

Review-Url: https://codereview.chromium.org/2809953002
Cr-Commit-Position: refs/heads/master@{#463982}
Committed:
https://chromium.googlesource.com/chromium/src/+/87a42b7af9035be518adf40266e4...
==========

[commit-bot: I haz the power, 2017-04-12 10:11:56]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/87a42b7af9035be518adf40266e4...