Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2660783002:
Range request support for parallel download in DownloadRequestCore. (Closed)
DescriptionRange request support for parallel download in DownloadRequestCore.
1. Add range request handling for parallel download, controlled
by DownloadUrlParameters. Currently put the logic in
DownloadRequestCore, if requests logic defers too much, we can pull out
to a new builder class.
Download resumption Range header: Range:100-
Parallel download Range header: Range:100-149
2. Add a unit test for DownloadRequestCore to ensure requests are
correctly built.
BUG=644352
Review-Url: https://codereview.chromium.org/2660783002
Cr-Commit-Position: refs/heads/master@{#449171}
Committed: https://chromium.googlesource.com/chromium/src/+/fed7466d012f7593c11843b2a98d3c4326c54ac9
Patch Set 1 #Patch Set 2 : Add const to test functions. #
Total comments: 4
Patch Set 3 : Strong validator for all range request. #
Total comments: 12
Patch Set 4 : Work on feedbacks. #
Total comments: 4
Patch Set 5 : If-Range and better unit tests. #Patch Set 6 : Remove debug code. #
Total comments: 16
Patch Set 7 : Add 402 handling, and nits. #Patch Set 8 : Remove unused run_loop in unittest. #Patch Set 9 : Fix chrome unit test. #Patch Set 10 : Remove http 402 handling so won't touch extension api for now. #
Total comments: 6
Patch Set 11 : Work on feedbacks. #
Messages
Total messages: 45 (27 generated)
Description was changed from ========== Range request support for parallel download in DownloadRequestCore. 1. Add range request handling for non download resumption. 2. Add a unit test for DownloadRequestCore to ensure requests are correctly built. BUG=644352 ========== to ========== Range request support for parallel download in DownloadRequestCore. 1. Add range request handling for parallel download, controlled by DownloadUrlParameters. Currently put the logic in DownloadRequestCore, if requests logic defers too much, we can pull out to a new builder class. Download resumption Range header: Range:100- Parallel download Range header: Range:100-149 2. Add a unit test for DownloadRequestCore to ensure requests are correctly built. BUG=644352 ==========
xingliu@chromium.org changed reviewers: + asanka@chromium.org, dtrainor@chromium.org, qinmin@chromium.org
Hi, PHTAL, thanks.
https://codereview.chromium.org/2660783002/diff/20001/content/browser/downloa... File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2660783002/diff/20001/content/browser/downloa... content/browser/download/download_request_core.cc:162: DCHECK(params->offset() == 0 || params->length() > 0 || has_etag || I don't think this DCHECK is right. The original DCHECK checks is purely for resumption: if the download is a resumption, then has_etag || has_last_modified is true. Otherwise, params->offset() == 0. When dealing with parallel download, you should have a separate DCHECK: DCHECK(param->length() == DownloadSaveInfo::kLengthUnknown || has_etag || has_last_modified); https://codereview.chromium.org/2660783002/diff/20001/content/browser/downloa... content/browser/download/download_request_core.cc:186: if (params->length() > 0) { you need to add has_etag || has_last_modified condition here. If there is no strong validator, we cannot ensure parallel download jobs are downloading the same content
https://codereview.chromium.org/2660783002/diff/20001/content/browser/downloa... File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2660783002/diff/20001/content/browser/downloa... content/browser/download/download_request_core.cc:162: DCHECK(params->offset() == 0 || params->length() > 0 || has_etag || On 2017/01/31 19:25:56, qinmin wrote: > I don't think this DCHECK is right. > > The original DCHECK checks is purely for resumption: if the download is a > resumption, then has_etag || has_last_modified is true. Otherwise, > params->offset() == 0. > > When dealing with parallel download, you should have a separate DCHECK: > DCHECK(param->length() == DownloadSaveInfo::kLengthUnknown || has_etag || > has_last_modified); Done, this is something I didn't consider that strong validator ensures requests are working on the same file. Merged the two DCHECK into one. https://codereview.chromium.org/2660783002/diff/20001/content/browser/downloa... content/browser/download/download_request_core.cc:186: if (params->length() > 0) { On 2017/01/31 19:25:56, qinmin wrote: > you need to add has_etag || has_last_modified condition here. If there is no > strong validator, we cannot ensure parallel download jobs are downloading the > same content Done.
Is the plan to download the stripes into multiple files? https://codereview.chromium.org/2660783002/diff/40001/content/browser/downloa... File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2660783002/diff/40001/content/browser/downloa... content/browser/download/download_request_core.cc:176: // In accordance with RFC 2616 Section 14.27, use If-Range to specify that This is now RFC 7233 Section 3.2 ( https://tools.ietf.org/html/rfc7233#section-3.2 ). The semantics are still the same. https://codereview.chromium.org/2660783002/diff/40001/content/browser/downloa... content/browser/download/download_request_core.cc:191: request->SetExtraRequestHeaderByName( Also need an If-Range header for this case. Can you fold this logic into the block above and just choose between "Range: bytes=xx-" and "Range: bytes=xx-yy" depending on whether a length is specified? https://codereview.chromium.org/2660783002/diff/40001/content/browser/downloa... content/browser/download/download_request_core.cc:605: if (http_headers.response_code() != net::HTTP_PARTIAL_CONTENT) { For cases where we were attempting a slice, this should be a hard fail.
https://codereview.chromium.org/2660783002/diff/40001/content/public/browser/... File content/public/browser/download_save_info.h (right): https://codereview.chromium.org/2660783002/diff/40001/content/public/browser/... content/public/browser/download_save_info.h:26: static const int kLengthUnknown; I don't feel strongly, but would a name that more represents 'whole file' be better than 'unknown'? https://codereview.chromium.org/2660783002/diff/40001/content/public/browser/... content/public/browser/download_save_info.h:48: // Ask to retrieve segment of the download file when length is greater than 0. Nit, just curious about whether or not 0 is a valid length. https://codereview.chromium.org/2660783002/diff/40001/content/public/browser/... File content/public/browser/download_url_parameters.h (right): https://codereview.chromium.org/2660783002/diff/40001/content/public/browser/... content/public/browser/download_url_parameters.h:174: void set_length(int64_t length) { save_info_.length = length; } Add a comment here along the lines of the one in DownloadSaveInfo IMO.
Thank you all for the review. @asanka, we probably will write to the same file with multiple byte streams. We still need to tweak the file handle code in other CL. https://codereview.chromium.org/2660783002/diff/40001/content/browser/downloa... File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2660783002/diff/40001/content/browser/downloa... content/browser/download/download_request_core.cc:176: // In accordance with RFC 2616 Section 14.27, use If-Range to specify that On 2017/02/01 02:53:36, asanka wrote: > This is now RFC 7233 Section 3.2 ( > https://tools.ietf.org/html/rfc7233#section-3.2 ). The semantics are still the > same. Oh, yes, updated all RFC 2616 to new standards in this file. https://codereview.chromium.org/2660783002/diff/40001/content/browser/downloa... content/browser/download/download_request_core.cc:191: request->SetExtraRequestHeaderByName( On 2017/02/01 02:53:36, asanka wrote: > Also need an If-Range header for this case. > > Can you fold this logic into the block above and just choose between "Range: > bytes=xx-" and "Range: bytes=xx-yy" depending on whether a length is specified? Done. https://codereview.chromium.org/2660783002/diff/40001/content/browser/downloa... content/browser/download/download_request_core.cc:605: if (http_headers.response_code() != net::HTTP_PARTIAL_CONTENT) { On 2017/02/01 02:53:37, asanka wrote: > For cases where we were attempting a slice, this should be a hard fail. Done. This will pair with the If-Range added above. https://codereview.chromium.org/2660783002/diff/40001/content/public/browser/... File content/public/browser/download_save_info.h (right): https://codereview.chromium.org/2660783002/diff/40001/content/public/browser/... content/public/browser/download_save_info.h:26: static const int kLengthUnknown; On 2017/02/01 07:14:36, David Trainor wrote: > I don't feel strongly, but would a name that more represents 'whole file' be > better than 'unknown'? Changed to kLengthFullContent; https://codereview.chromium.org/2660783002/diff/40001/content/public/browser/... content/public/browser/download_save_info.h:48: // Ask to retrieve segment of the download file when length is greater than 0. On 2017/02/01 07:14:36, David Trainor wrote: > Nit, just curious about whether or not 0 is a valid length. Yes, this is something weird, changed the const to 0. When it's 0, it doesn't make sense to send a request or save 0 bytes on disk, so it's probably ok for 0 to represent the whole file. https://codereview.chromium.org/2660783002/diff/40001/content/public/browser/... File content/public/browser/download_url_parameters.h (right): https://codereview.chromium.org/2660783002/diff/40001/content/public/browser/... content/public/browser/download_url_parameters.h:174: void set_length(int64_t length) { save_info_.length = length; } On 2017/02/01 07:14:36, David Trainor wrote: > Add a comment here along the lines of the one in DownloadSaveInfo IMO. Done. Also added some comment for set_offset.
https://codereview.chromium.org/2660783002/diff/60001/content/browser/downloa... File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2660783002/diff/60001/content/browser/downloa... content/browser/download/download_request_core.cc:169: params->length() > DownloadSaveInfo::kLengthFullContent) && This expression requires that kLengthFullContent be 0. Any other value would be nonsensical. Perhaps just check that params->length() != kLengthFullContent ? https://codereview.chromium.org/2660783002/diff/60001/content/browser/downloa... content/browser/download/download_request_core.cc:599: // byte position specified. e.g. "Range:bytes=50-99". Since we specified an If-Range header, we specifically asked the server to send the entire entity if the validators didn't match. We are trying to do two things here: * Assuming we have a prefix of the entity, we can send a request for the remainder. This is Range: bytes=123- . If there's a mismatch in our validators, then the server send the entire resource and we discard the prefix and store the entity. * With-or-without a prefix, we want to fetch some random part of the entity. This is 'Range: bytes=123-345'. If validation fails, then we don't necessarily want to revert to fetching the entire entity. Instead we fail and allow the caller to figure out how they want to handle this. In the case of multiple donwload jobs, you'd now be able to ensure that only the primary job will fetch the entire resource. We really don't want each job to fetch full separate copies :-) So, with apologies, the case of the length-capped fetch should be handled as: Range: bytes=123-235 If-Match: etag or Range: bytes=123-234 If-Unmodified-Since: last-modified This way, the server will fail the request with a HTTP 412 rather than send the entire response body.
The CQ bit was checked by xingliu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2660783002/diff/60001/content/browser/downloa... File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2660783002/diff/60001/content/browser/downloa... content/browser/download/download_request_core.cc:169: params->length() > DownloadSaveInfo::kLengthFullContent) && On 2017/02/01 22:51:17, asanka wrote: > This expression requires that kLengthFullContent be 0. Any other value would be > nonsensical. > > Perhaps just check that params->length() != kLengthFullContent ? Make sense, changed to != check in this file. https://codereview.chromium.org/2660783002/diff/60001/content/browser/downloa... content/browser/download/download_request_core.cc:599: // byte position specified. e.g. "Range:bytes=50-99". On 2017/02/01 22:51:17, asanka wrote: > Since we specified an If-Range header, we specifically asked the server to send > the entire entity if the validators didn't match. > > We are trying to do two things here: > > * Assuming we have a prefix of the entity, we can send a request for the > remainder. This is Range: bytes=123- . If there's a mismatch in our validators, > then the server send the entire resource and we discard the prefix and store the > entity. > > * With-or-without a prefix, we want to fetch some random part of the entity. > This is 'Range: bytes=123-345'. If validation fails, then we don't necessarily > want to revert to fetching the entire entity. Instead we fail and allow the > caller to figure out how they want to handle this. > > In the case of multiple donwload jobs, you'd now be able to ensure that only the > primary job will fetch the entire resource. We really don't want each job to > fetch full separate copies :-) > > So, with apologies, the case of the length-capped fetch should be handled as: > > Range: bytes=123-235 > If-Match: etag > > or > > Range: bytes=123-234 > If-Unmodified-Since: last-modified > > This way, the server will fail the request with a HTTP 412 rather than send the > entire response body. Thanks for looking into RFC details here. Yeah, I'm thinking of If-Range is a bit weird for a slice. It can work but need to drop the request somewhere when the server gives 200 response for a slice like "Range:bytes=x-y". Changed to send If-Match and If-Unmodified-Since together. In RFC, these two fields can stack together.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LG so far. This should be good to go after adding the handling for HTTP 412. The others are nits. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core.cc:200: request->SetExtraRequestHeaderByName("If-Match", params->etag(), true); Could you add "If-Match"/"If-Unmodified-Since" to HttpRequestHeaders? I can stamp that change along with this one. Then you can use the new constants here and in tests. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core.cc:564: switch (http_headers.response_code()) { We should now handle HTTP 412 here (net::HTTP_PRECONDITION_FAILED) which could conceivably map to the obsoleted PRECONDITION_FAILED interrupt reason (https://cs.chromium.org/chromium/src/content/public/browser/download_interrup...). https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core.cc:618: // byte position specified. e.g. "Range:bytes=50-99". Nit: This is a result of the If-Match/If-Unmodified-Since + Range headers. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... File content/browser/download/download_request_core_unittest.cc (right): https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core_unittest.cc:23: ~DownloadRequestCoreTest() override = default; Nit: you should be able to leave these out and achieve the same effect. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core_unittest.cc:33: const std::string& expected_header_value) const { Nit/suggestion: Introduce a way to verify that the header doesn't exist at all rather than existing with an empty value. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core_unittest.cc:40: void CreateRequestOnIOThread(DownloadUrlParameters* params) { Since you are using TestBrowserThreadBundle without requesting real threads, you should be able to call DownloadRequestCore::CreateRequestOnIOThread directly without doing a post task. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core_unittest.cc:86: BuildDownloadParameters("something.com"); Nit: Use example.com for examples and tests. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core_unittest.cc:139: } Add a test case with both an etag and a last-modified header? Should only affect the length capped test cases.
The CQ bit was checked by xingliu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by xingliu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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_...) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
Hi, I tried to add PRECONDITION_FAILED back in interrupt reason, but it needs to touch the extension api, can we do it later in a separate patch? The enum in extensions::api::downloads is tied to download interrupt reason. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core.cc:200: request->SetExtraRequestHeaderByName("If-Match", params->etag(), true); On 2017/02/03 22:16:22, asanka wrote: > Could you add "If-Match"/"If-Unmodified-Since" to HttpRequestHeaders? I can > stamp that change along with this one. > > Then you can use the new constants here and in tests. Done. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core.cc:564: switch (http_headers.response_code()) { On 2017/02/03 22:16:23, asanka wrote: > We should now handle HTTP 412 here (net::HTTP_PRECONDITION_FAILED) which could > conceivably map to the obsoleted PRECONDITION_FAILED interrupt reason > (https://cs.chromium.org/chromium/src/content/public/browser/download_interrup...). > Can we do it later in a separate patch? Since it needs to touch extension api, where extensions::api::downloads is tied to download_interrupt_reasons.h. I'll put a TODO here. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core.cc:618: // byte position specified. e.g. "Range:bytes=50-99". On 2017/02/03 22:16:22, asanka wrote: > Nit: This is a result of the If-Match/If-Unmodified-Since + Range headers. Done. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... File content/browser/download/download_request_core_unittest.cc (right): https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core_unittest.cc:23: ~DownloadRequestCoreTest() override = default; On 2017/02/03 22:16:23, asanka wrote: > Nit: you should be able to leave these out and achieve the same effect. Done. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core_unittest.cc:33: const std::string& expected_header_value) const { On 2017/02/03 22:16:23, asanka wrote: > Nit/suggestion: Introduce a way to verify that the header doesn't exist at all > rather than existing with an empty value. Done. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core_unittest.cc:40: void CreateRequestOnIOThread(DownloadUrlParameters* params) { On 2017/02/03 22:16:23, asanka wrote: > Since you are using TestBrowserThreadBundle without requesting real threads, you > should be able to call DownloadRequestCore::CreateRequestOnIOThread directly > without doing a post task. This is awesome, thanks. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core_unittest.cc:86: BuildDownloadParameters("something.com"); On 2017/02/03 22:16:23, asanka wrote: > Nit: Use http://example.com for examples and tests. Done. https://codereview.chromium.org/2660783002/diff/100001/content/browser/downlo... content/browser/download/download_request_core_unittest.cc:139: } On 2017/02/03 22:16:23, asanka wrote: > Add a test case with both an etag and a last-modified header? > > Should only affect the length capped test cases. Done.
The CQ bit was checked by xingliu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Ping, please help take another look. Thanks. For http 402, since eventually it will modify some extension api, added a TODO in download_request_core.cc and probably we can do it later.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
xingliu@chromium.org changed reviewers: + alexmos@chromium.org
alexmos@chromium.org: Hi, Alex, please help review content/public/browser/* in this CL.
lgtm Ultimately the PRECONDITION_FAILED interrupt reason would only be used when dealing with sharded requests. Since that logic doesn't exist yet, deferring the interrupt reason should be OK.
content/public/browser LGTM with nits https://codereview.chromium.org/2660783002/diff/180001/content/public/browser... File content/public/browser/download_save_info.h (right): https://codereview.chromium.org/2660783002/diff/180001/content/public/browser... content/public/browser/download_save_info.h:26: static const int kLengthFullContent; Up to you, but I think you could also just assign this to 0 here. Would it make sense to make it int64_t to match the |length| type? Also, might want to mention what this is used for in this comment as well. https://codereview.chromium.org/2660783002/diff/180001/content/public/browser... content/public/browser/download_save_info.h:47: // The length of the bytes from |offset|. Set to |kLengthFullContent| by nit: The number of bytes to download from |offset| (just trying to rephrase for a bit more clarity) https://codereview.chromium.org/2660783002/diff/180001/content/public/browser... content/public/browser/download_save_info.h:50: // Request the whole file when length is |kLengthFullContent|. nit: not the "whole file", but the rest of the file starting from |offset|, right? https://codereview.chromium.org/2660783002/diff/180001/content/public/browser... File content/public/browser/download_url_parameters.h (right): https://codereview.chromium.org/2660783002/diff/180001/content/public/browser... content/public/browser/download_url_parameters.h:172: // Also can use |set_length| to specify the last byte position, or the range nit: can remove "Also can" and just start with "Use"
lgtm
lgtm % nits https://codereview.chromium.org/2660783002/diff/180001/content/browser/downlo... File content/browser/download/download_request_core.cc (right): https://codereview.chromium.org/2660783002/diff/180001/content/browser/downlo... content/browser/download/download_request_core.cc:200: if (has_etag) nit: use{} for if statement that spans multiple lines. https://codereview.chromium.org/2660783002/diff/180001/content/browser/downlo... content/browser/download/download_request_core.cc:205: // "If-Match" presents. ditto
The CQ bit was checked by xingliu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by xingliu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from asanka@chromium.org, dtrainor@chromium.org, qinmin@chromium.org, alexmos@chromium.org Link to the patchset: https://codereview.chromium.org/2660783002/#ps200001 (title: "Work on feedbacks.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 200001, "attempt_start_ts": 1486601962481750,
"parent_rev": "3f8eca6742cd15d75532587c37ff4c37f8ecde05", "commit_rev":
"fed7466d012f7593c11843b2a98d3c4326c54ac9"}
Message was sent while issue was closed.
Description was changed from ========== Range request support for parallel download in DownloadRequestCore. 1. Add range request handling for parallel download, controlled by DownloadUrlParameters. Currently put the logic in DownloadRequestCore, if requests logic defers too much, we can pull out to a new builder class. Download resumption Range header: Range:100- Parallel download Range header: Range:100-149 2. Add a unit test for DownloadRequestCore to ensure requests are correctly built. BUG=644352 ========== to ========== Range request support for parallel download in DownloadRequestCore. 1. Add range request handling for parallel download, controlled by DownloadUrlParameters. Currently put the logic in DownloadRequestCore, if requests logic defers too much, we can pull out to a new builder class. Download resumption Range header: Range:100- Parallel download Range header: Range:100-149 2. Add a unit test for DownloadRequestCore to ensure requests are correctly built. BUG=644352 Review-Url: https://codereview.chromium.org/2660783002 Cr-Commit-Position: refs/heads/master@{#449171} Committed: https://chromium.googlesource.com/chromium/src/+/fed7466d012f7593c11843b2a98d... ==========
Message was sent while issue was closed.
Committed patchset #11 (id:200001) as https://chromium.googlesource.com/chromium/src/+/fed7466d012f7593c11843b2a98d... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
