New issue
Advanced search Search tips

Issue 715630 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-Servicification


Sign in to add a comment

Download code should be layered ontop of network service

Project Member Reported by jam@chromium.org, Apr 26 2017

Issue description

Downloads is pretty closely tied to networking right now. We should simplify this. Per various discussions, it would be great if downloads is not part of networking, but is instead layered on it. The network service should only care about fetching requests and giving the response to the caller.
 

Comment 1 by jam@chromium.org, Apr 26 2017

Cc: rdsmith@chromium.org
Labels: -Pri-3 Pri-2
Owner: dtrainor@chromium.org
Status: Assigned (was: Untriaged)
Cc: asanka@chromium.org

Comment 3 by yzshen@chromium.org, May 24 2017

Components: Internals>Network>Service

Comment 4 by jam@chromium.org, Jul 7 2017

Blockedon: 740130

Comment 5 by jam@chromium.org, Jul 20 2017

Blockedon: 705744

Comment 6 by jam@chromium.org, Jul 20 2017

Blockedon: 747130
Cc: dtrainor@chromium.org
Owner: qin...@chromium.org
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71ff85c2f56c6a3934674edcd699583fc810d97f

commit 71ff85c2f56c6a3934674edcd699583fc810d97f
Author: Min Qin <qinmin@chromium.org>
Date: Thu Jul 27 23:11:02 2017

allow DownloadFile to take a DataPipe for input source

This CL allows SourceStream to take a ScopedDataPipeConsumerHandle object as input source
And it hides the internal data source from public
So callers don't need to worry about whether the data is coming from byte stream or a data pipe.
There are still some TODOs for StreamActive() and GetStatus() method, but will do that in separate CL.

BUG= 715630 

Change-Id: Ia10b1b15c4294f65fa5f7125b77eeed3f3fab2c7
Reviewed-on: https://chromium-review.googlesource.com/582197
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490070}
[modify] https://crrev.com/71ff85c2f56c6a3934674edcd699583fc810d97f/content/browser/download/download_file.h
[modify] https://crrev.com/71ff85c2f56c6a3934674edcd699583fc810d97f/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/71ff85c2f56c6a3934674edcd699583fc810d97f/content/browser/download/download_file_impl.h
[modify] https://crrev.com/71ff85c2f56c6a3934674edcd699583fc810d97f/content/browser/download/mock_download_file.cc
[modify] https://crrev.com/71ff85c2f56c6a3934674edcd699583fc810d97f/content/browser/download/mock_download_file.h

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c5ebc2c9ea8c9c52d3b092a064c03e954b5f378

commit 5c5ebc2c9ea8c9c52d3b092a064c03e954b5f378
Author: Min Qin <qinmin@chromium.org>
Date: Wed Aug 09 16:30:24 2017

Move the logic for determining the download complete status into a separate file

This logic will be used by both network service and non-mojo implementations.
Move it to a separate file so it can be reused.

BUG= 715630 

Change-Id: I56c76f4a2bb9b58c42c64102813976b01f858af5
Reviewed-on: https://chromium-review.googlesource.com/587437
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493014}
[modify] https://crrev.com/5c5ebc2c9ea8c9c52d3b092a064c03e954b5f378/content/browser/BUILD.gn
[modify] https://crrev.com/5c5ebc2c9ea8c9c52d3b092a064c03e954b5f378/content/browser/download/download_request_core.cc
[modify] https://crrev.com/5c5ebc2c9ea8c9c52d3b092a064c03e954b5f378/content/browser/download/download_request_core.h
[add] https://crrev.com/5c5ebc2c9ea8c9c52d3b092a064c03e954b5f378/content/browser/download/download_utils.cc
[add] https://crrev.com/5c5ebc2c9ea8c9c52d3b092a064c03e954b5f378/content/browser/download/download_utils.h

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b3ca235bcffd5d402fb2bc5fc604acd353b9b78b

commit b3ca235bcffd5d402fb2bc5fc604acd353b9b78b
Author: Min Qin <qinmin@chromium.org>
Date: Thu Aug 10 23:15:46 2017

Download: Introduce a new method to create ResourceRequest for network service

To launch a ThrottlingUrlLoader for download, we need to pass a ResourceRequest,
instead of a URLRequest.
This CL adds a method to create the ResourceRequest.

BUG= 715630 

Change-Id: Ife84d10c391c9d129c16218648e1917c6302d174
Reviewed-on: https://chromium-review.googlesource.com/602601
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493593}
[modify] https://crrev.com/b3ca235bcffd5d402fb2bc5fc604acd353b9b78b/content/browser/download/download_request_core.cc
[modify] https://crrev.com/b3ca235bcffd5d402fb2bc5fc604acd353b9b78b/content/browser/download/download_request_core.h
[modify] https://crrev.com/b3ca235bcffd5d402fb2bc5fc604acd353b9b78b/content/browser/download/download_utils.cc
[modify] https://crrev.com/b3ca235bcffd5d402fb2bc5fc604acd353b9b78b/content/browser/download/download_utils.h

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0bc51825251036eb4182fffeb9209c9eb5fccd7f

commit 0bc51825251036eb4182fffeb9209c9eb5fccd7f
Author: Min Qin <qinmin@chromium.org>
Date: Sat Aug 12 00:53:18 2017

Introduce DownloadResponseHandler class

This class will handle responses for both navigation triggered downloads and context menu downloads.
It is responsible for passing the mojo DataPipe and completion status to DownloadFile.
The Delegate class will be notified once data is ready to be consumed.

BUG= 715630 

Change-Id: I6d839454d5601098aa98550fd6541e17dd02e9e0
Reviewed-on: https://chromium-review.googlesource.com/608746
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493913}
[modify] https://crrev.com/0bc51825251036eb4182fffeb9209c9eb5fccd7f/content/browser/BUILD.gn
[modify] https://crrev.com/0bc51825251036eb4182fffeb9209c9eb5fccd7f/content/browser/download/download_request_core.cc
[modify] https://crrev.com/0bc51825251036eb4182fffeb9209c9eb5fccd7f/content/browser/download/download_request_core.h
[add] https://crrev.com/0bc51825251036eb4182fffeb9209c9eb5fccd7f/content/browser/download/download_response_handler.cc
[add] https://crrev.com/0bc51825251036eb4182fffeb9209c9eb5fccd7f/content/browser/download/download_response_handler.h
[modify] https://crrev.com/0bc51825251036eb4182fffeb9209c9eb5fccd7f/content/browser/download/download_utils.cc
[modify] https://crrev.com/0bc51825251036eb4182fffeb9209c9eb5fccd7f/content/browser/download/download_utils.h

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc0ba42ad570f704b6d04a3ebbc0c2332dc74632

commit dc0ba42ad570f704b6d04a3ebbc0c2332dc74632
Author: Min Qin <qinmin@chromium.org>
Date: Tue Aug 15 23:38:46 2017

Pass response completion status to DownloadFile

Data pipe doesn't support sending a status code at completion/abort.
Adding an extra call to send the completion status
see  http://crbug.com/748240  for more information.

BUG= 715630 

Change-Id: Ibb911a9d6841dc56fbdd62646dc868fa504271ca
Reviewed-on: https://chromium-review.googlesource.com/592515
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494604}
[modify] https://crrev.com/dc0ba42ad570f704b6d04a3ebbc0c2332dc74632/content/browser/download/download_file.h
[modify] https://crrev.com/dc0ba42ad570f704b6d04a3ebbc0c2332dc74632/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/dc0ba42ad570f704b6d04a3ebbc0c2332dc74632/content/browser/download/download_file_impl.h
[modify] https://crrev.com/dc0ba42ad570f704b6d04a3ebbc0c2332dc74632/content/browser/download/mock_download_file.h

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a22dbb3ae4b9cc1535a251015155add87b3da1b7

commit a22dbb3ae4b9cc1535a251015155add87b3da1b7
Author: Min Qin <qinmin@chromium.org>
Date: Wed Aug 23 23:00:21 2017

Introduce ResourceDownloader for url downloading with network service

Both the ResourceDownloader and the UrlDownloader now inherits UrlDownloadHandler,
so DownloadManagerImpl/DownloadWorker can reuse most of the current code for both implementations.
The ResourceDownloader returns a data pipe when download starts,
DownloadManagerImpl/DownloadWorker can use the data pipe to initialize DownloadFile.

Bug= 715630 

Change-Id: Iff558ae1acc7145677a3d517c84e6600dd4e386f
Bug: 
Reviewed-on: https://chromium-review.googlesource.com/621772
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496847}
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/BUILD.gn
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/download_manager_impl.h
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/download_response_handler.cc
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/download_response_handler.h
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/download_utils.cc
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/download_worker.cc
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/download_worker.h
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/parallel_download_job_unittest.cc
[add] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/resource_downloader.cc
[add] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/resource_downloader.h
[add] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/url_download_handler.cc
[add] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/url_download_handler.h
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/url_downloader.cc
[modify] https://crrev.com/a22dbb3ae4b9cc1535a251015155add87b3da1b7/content/browser/download/url_downloader.h

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd

commit ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Aug 24 03:13:11 2017

Revert "Introduce ResourceDownloader for url downloading with network service"

This reverts commit a22dbb3ae4b9cc1535a251015155add87b3da1b7.

Reason for revert: causes failures in network service bot: https://build.chromium.org/p/chromium.fyi/builders/Mojo%20Linux/builds/4530

Please use the linux_mojo trybot to repro.

Original change's description:
> Introduce ResourceDownloader for url downloading with network service
> 
> Both the ResourceDownloader and the UrlDownloader now inherits UrlDownloadHandler,
> so DownloadManagerImpl/DownloadWorker can reuse most of the current code for both implementations.
> The ResourceDownloader returns a data pipe when download starts,
> DownloadManagerImpl/DownloadWorker can use the data pipe to initialize DownloadFile.
> 
> Bug= 715630 
> 
> Change-Id: Iff558ae1acc7145677a3d517c84e6600dd4e386f
> Bug: 
> Reviewed-on: https://chromium-review.googlesource.com/621772
> Commit-Queue: Min Qin <qinmin@chromium.org>
> Reviewed-by: Bo <boliu@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#496847}

TBR=boliu@chromium.org,dtrainor@chromium.org,qinmin@chromium.org

Change-Id: Ia431a23f934de377038c2dbea089047dc03ad9bc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/631436
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496937}
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/BUILD.gn
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/download/download_manager_impl.h
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/download/download_response_handler.cc
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/download/download_response_handler.h
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/download/download_utils.cc
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/download/download_worker.cc
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/download/download_worker.h
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/download/parallel_download_job_unittest.cc
[delete] https://crrev.com/7ac6016772d9154bab80d0d602082a7516fc49b5/content/browser/download/resource_downloader.cc
[delete] https://crrev.com/7ac6016772d9154bab80d0d602082a7516fc49b5/content/browser/download/resource_downloader.h
[delete] https://crrev.com/7ac6016772d9154bab80d0d602082a7516fc49b5/content/browser/download/url_download_handler.cc
[delete] https://crrev.com/7ac6016772d9154bab80d0d602082a7516fc49b5/content/browser/download/url_download_handler.h
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/download/url_downloader.cc
[modify] https://crrev.com/ffb9e08dfbc20e5ecd29d7eaacb7c97a7b27d8bd/content/browser/download/url_downloader.h

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9

commit 3cbdb8db9d4f2b379724365596e05d8ce56a4bc9
Author: Min Qin <qinmin@chromium.org>
Date: Fri Aug 25 06:23:09 2017

Reland: Introduce ResourceDownloader for url downloading with network service

The previous CL: https://chromium-review.googlesource.com/c/chromium/src/+/621772 was reverted,
due to broken test on network service bot.
However, those test shouldn't pass in the first place.
The test passes because they are not using network service.
Instead, they are using the UrlRequest codepath.
This CL is clone of the original CL, but it removed the if statement to create ResourceDownloader
in DownloadManagerImpl. I added a TODO instead.
Will enable the ResourceDownload creation when the new code path fully works,
so network service bot will be happy.

Both the ResourceDownloader and the UrlDownloader now inherits UrlDownloadHandler,
so DownloadManagerImpl/DownloadWorker can reuse most of the current code for both implementations.
The ResourceDownloader returns a data pipe when download starts,
DownloadManagerImpl/DownloadWorker can use the data pipe to initialize DownloadFile.

Bug= 715630 

Change-Id: Ia4cdfee066e0c8236b941437b58393bdeaaaa5fc
Reviewed-on: https://chromium-review.googlesource.com/631636
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497336}
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/BUILD.gn
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/download_manager_impl.h
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/download_response_handler.cc
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/download_response_handler.h
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/download_utils.cc
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/download_worker.cc
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/download_worker.h
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/parallel_download_job_unittest.cc
[add] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/resource_downloader.cc
[add] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/resource_downloader.h
[add] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/url_download_handler.cc
[add] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/url_download_handler.h
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/url_downloader.cc
[modify] https://crrev.com/3cbdb8db9d4f2b379724365596e05d8ce56a4bc9/content/browser/download/url_downloader.h

Project Member

Comment 16 by bugdroid1@chromium.org, Sep 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e4d82ab633d6dd28d13e273a7857fa020b045d4

commit 0e4d82ab633d6dd28d13e273a7857fa020b045d4
Author: Min Qin <qinmin@chromium.org>
Date: Fri Sep 01 20:40:31 2017

Making ResourceDownloader to work for network service

This CL makes the ResourceDownload to send a ResourceRequest,
and consumes the mojo data pipe when it becomes available.
It reuses a lot of the UrlDownloader code path, so we won't have
a lot of maintainence issues whether network service is enabled.
The basic context menu and background download functionality should work now.

BUG= 715630 

Change-Id: I18946a28324370d60d1b9e4bb49f4d8f597bc070
Reviewed-on: https://chromium-review.googlesource.com/641912
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499283}
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/BUILD.gn
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_file.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_file_factory.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_file_factory.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_file_impl.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_interrupt_reasons_impl.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_interrupt_reasons_impl.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_job.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_job.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_manager_impl.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_manager_impl_unittest.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_resource_handler.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_response_handler.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_response_handler.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_worker.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/download_worker.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/mock_download_file.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/mock_download_file.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/parallel_download_job_unittest.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/resource_downloader.h
[delete] https://crrev.com/f5f64803582b166ac7ffb10efd70c3e2e2e5b1b0/content/browser/download/url_download_handler.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/url_download_handler.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/browser/download/url_downloader.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/public/browser/download_manager.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/public/browser/download_manager.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/public/common/BUILD.gn
[add] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/public/common/download_stream.mojom
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/public/test/mock_download_manager.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/public/test/mock_download_manager.h
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/content/public/test/test_file_error_injector.cc
[modify] https://crrev.com/0e4d82ab633d6dd28d13e273a7857fa020b045d4/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Project Member

Comment 17 by bugdroid1@chromium.org, Sep 13 2017

Project Member

Comment 18 by bugdroid1@chromium.org, Sep 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/37db510220601abd49bcceba7c0bbe62256b5f8f

commit 37db510220601abd49bcceba7c0bbe62256b5f8f
Author: Min Qin <qinmin@chromium.org>
Date: Wed Sep 13 21:21:25 2017

Pass correct is_download value to NavigationRequest

Currently the is_download value is set to false.
We should be able to determine the download value based on response.
However, this CL doesn't address whether plugin can also handle the response.
That can be done in separate CLs.

BUG= 715630 

Change-Id: I668c115e2854ddbe59f078cecde04bd7216069cd
Reviewed-on: https://chromium-review.googlesource.com/661891
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501752}
[modify] https://crrev.com/37db510220601abd49bcceba7c0bbe62256b5f8f/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/37db510220601abd49bcceba7c0bbe62256b5f8f/content/browser/loader/navigation_url_loader_network_service.h
[modify] https://crrev.com/37db510220601abd49bcceba7c0bbe62256b5f8f/content/browser/loader/navigation_url_loader_unittest.cc
[modify] https://crrev.com/37db510220601abd49bcceba7c0bbe62256b5f8f/content/test/test_navigation_url_loader_delegate.cc
[modify] https://crrev.com/37db510220601abd49bcceba7c0bbe62256b5f8f/content/test/test_navigation_url_loader_delegate.h
[modify] https://crrev.com/37db510220601abd49bcceba7c0bbe62256b5f8f/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter
[modify] https://crrev.com/37db510220601abd49bcceba7c0bbe62256b5f8f/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 19 by bugdroid1@chromium.org, Sep 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/43aa323b6fc671614558a52f1b2f5d16530b2246

commit 43aa323b6fc671614558a52f1b2f5d16530b2246
Author: Min Qin <qinmin@chromium.org>
Date: Wed Sep 20 21:19:36 2017

Fix Cross-Origin download suggested name

On redirection, the suggested name for download should get cleared if it is cross origin.

BUG= 715630 

Change-Id: Ib9f3c7cc2b021b5b1c1eebe47ab2a56a42dc1f18
Reviewed-on: https://chromium-review.googlesource.com/675572
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503250}
[modify] https://crrev.com/43aa323b6fc671614558a52f1b2f5d16530b2246/content/browser/download/download_response_handler.cc
[modify] https://crrev.com/43aa323b6fc671614558a52f1b2f5d16530b2246/content/browser/download/download_response_handler.h
[modify] https://crrev.com/43aa323b6fc671614558a52f1b2f5d16530b2246/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/43aa323b6fc671614558a52f1b2f5d16530b2246/content/browser/download/resource_downloader.h

Project Member

Comment 20 by bugdroid1@chromium.org, Sep 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4eb4ec43a3380d17c5bd1c139918d567b5ef35f7

commit 4eb4ec43a3380d17c5bd1c139918d567b5ef35f7
Author: Min Qin <qinmin@chromium.org>
Date: Fri Sep 22 20:13:44 2017

Network service:Check whether a URL can be requested before sending request

For network service, we should do the same as we are currently doing

BUG= 715630 

Change-Id: I08039c17eda4cb8bbe780a9f4a3bdc64ebde735f
Reviewed-on: https://chromium-review.googlesource.com/678921
Reviewed-by: Xing Liu <xingliu@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503834}
[modify] https://crrev.com/4eb4ec43a3380d17c5bd1c139918d567b5ef35f7/content/browser/download/download_manager_impl.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Sep 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40b721739d4e72d004df05079ce00c5aa99f1ed5

commit 40b721739d4e72d004df05079ce00c5aa99f1ed5
Author: Min Qin <qinmin@chromium.org>
Date: Wed Sep 27 00:19:17 2017

Network service: Make content initiated download to work

This CL implements the funtionality for download to intercept the navigation response.
In order for this to work, DownloadManager will pass a callback to NavigationRequest.
The callback will finally be posted and executed on the IO thread,
so that URLLoader and the forwarding URLLoaderClient can be transfered at the same time.
Navigation will be canceled during the process.
ResourceDownload will own the URLLoader, and DownloadResponseHandler will serve
as the new URLLoaderClient

Bug= 715630 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I9c44b715eeee6ef0df7e75f0c2d76612ad69a4e0
Reviewed-on: https://chromium-review.googlesource.com/669039
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504536}
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/download/download_manager_impl.h
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/download/download_response_handler.cc
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/download/download_response_handler.h
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/download/resource_downloader.h
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/frame_host/navigation_request.h
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/loader/navigation_url_loader.h
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/loader/navigation_url_loader_impl.h
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/browser/loader/navigation_url_loader_network_service.h
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/common/throttling_url_loader.h
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/test/test_navigation_url_loader.cc
[modify] https://crrev.com/40b721739d4e72d004df05079ce00c5aa99f1ed5/content/test/test_navigation_url_loader.h

Project Member

Comment 22 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99b2c57efe8991c06fdea35467c8091e805b07f4

commit 99b2c57efe8991c06fdea35467c8091e805b07f4
Author: Min Qin <qinmin@chromium.org>
Date: Thu Sep 28 05:04:37 2017

Fix Blob url download for network service

For blob URL download, it needs to hold onto the BlobDataHandle before the download completes.
This solves the case that a blob url is immediately revoked after download requests starts.
However, ThrottlingURLLoader::CreateLoaderAndStart() doesn't handle this case.
This is because it calls into the non static CreateLoaderAndStart() method in BlobURLLoaderFactory.
That method searches for the blob url-id mapping after the download request started.
And the mapping may have already been revoked in that situation
To solve this, we should use the static BlobURLLoaderFactory::CreateLoaderAndStart() method

BUG= 715630 

Change-Id: I96d7b70719df3243026ce2f58f1491a285f3c36e
Reviewed-on: https://chromium-review.googlesource.com/677778
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504910}
[modify] https://crrev.com/99b2c57efe8991c06fdea35467c8091e805b07f4/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/99b2c57efe8991c06fdea35467c8091e805b07f4/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/99b2c57efe8991c06fdea35467c8091e805b07f4/content/browser/download/resource_downloader.h

Project Member

Comment 23 by bugdroid1@chromium.org, Oct 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32c60fdc1d049e8b78bc59c0770eda996d976944

commit 32c60fdc1d049e8b78bc59c0770eda996d976944
Author: Min Qin <qinmin@chromium.org>
Date: Mon Oct 02 23:38:23 2017

Fix an issue that EmbeddedTestServer may not send headers that can be parsed correctly

HttpUtils is looking for "\n\n" and "\n\r\n" as the end of the header.
So if the headers are already ending with "\n\n", we shouldn't append "\r\n" to it.
Otherwise, the appended "\r\n" will be treated as the body

BUG= 715630 

Change-Id: Ia087f3ec7f7644c088635f375782841f2c6a32d0
Reviewed-on: https://chromium-review.googlesource.com/693323
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505856}
[modify] https://crrev.com/32c60fdc1d049e8b78bc59c0770eda996d976944/net/test/embedded_test_server/http_response.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1895b7464a705aba45a3be734dcac21ad1aefb73

commit 1895b7464a705aba45a3be734dcac21ad1aefb73
Author: Min Qin <qinmin@chromium.org>
Date: Tue Oct 03 04:46:55 2017

stop using URLRequestMockHTTPJob from DownloadBrowsertests

URLRequestInterceptor doesn't work with network service.
And EmbeddedTestServer has the same functionality as URLRequestMockHTTPJob.
We can switch to EmbeddedTestServer without impacting existing tests.

BUG= 715630 

Change-Id: I771dd360aaf274c92bb9df2d3e71b216705ec5cd
Reviewed-on: https://chromium-review.googlesource.com/693398
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505944}
[modify] https://crrev.com/1895b7464a705aba45a3be734dcac21ad1aefb73/content/browser/download/download_browsertest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73328f25dc6eaa63fa407e1e2263341f652787ae

commit 73328f25dc6eaa63fa407e1e2263341f652787ae
Author: Paul Jensen <pauljensen@chromium.org>
Date: Tue Oct 03 15:02:13 2017

[Cronet] Adjust tests after crrev.com/505856

Embedded HTTP test server is sending one less byte.

BUG= 715630 

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: I91d09a39727d74b62adf242b96b04a170fd687bd
Reviewed-on: https://chromium-review.googlesource.com/697545
Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506038}
[modify] https://crrev.com/73328f25dc6eaa63fa407e1e2263341f652787ae/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java

Project Member

Comment 26 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/606840c453e5069d97132f81fbaafd9eea7971c6

commit 606840c453e5069d97132f81fbaafd9eea7971c6
Author: Min Qin <qinmin@chromium.org>
Date: Thu Oct 12 05:07:16 2017

Rewrite some download browsertests to support network service

URLRequestInterceptor doesn't work with network service.
This change rewrites sine tests by using EmbeddedTestServer to return slow download responses.
The rewritten tests should have the same behavior as the original ones.
This should work with or without network service flag enabled.

BUG= 715630 

Change-Id: Ia2aad959cf7f33b6c3e66787819d25ac586433f1
Reviewed-on: https://chromium-review.googlesource.com/691040
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508258}
[modify] https://crrev.com/606840c453e5069d97132f81fbaafd9eea7971c6/content/browser/download/download_browsertest.cc
[add] https://crrev.com/606840c453e5069d97132f81fbaafd9eea7971c6/content/browser/download/slow_download_http_response.cc
[add] https://crrev.com/606840c453e5069d97132f81fbaafd9eea7971c6/content/browser/download/slow_download_http_response.h
[modify] https://crrev.com/606840c453e5069d97132f81fbaafd9eea7971c6/content/test/BUILD.gn

Project Member

Comment 27 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ed77c0ce9787eca013520687cc5ee123ae2e7bc

commit 2ed77c0ce9787eca013520687cc5ee123ae2e7bc
Author: Min Qin <qinmin@chromium.org>
Date: Tue Oct 17 02:45:17 2017

Mark download as main frame request

Download should always be treated as main frame request

BUG= 715630 

Change-Id: If1403ba99b30d98387de850d191876ad6bfabdbe
Reviewed-on: https://chromium-review.googlesource.com/721922
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509242}
[modify] https://crrev.com/2ed77c0ce9787eca013520687cc5ee123ae2e7bc/content/browser/download/download_utils.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c60045ca9986b04f0469f9ab4733ae0c8c7c2d9a

commit c60045ca9986b04f0469f9ab4733ae0c8c7c2d9a
Author: Min Qin <qinmin@chromium.org>
Date: Tue Oct 17 03:02:22 2017

Allow interrupted download to resume if network service is enabled

This CL adds the logic to resume an interrupted download with network service
A helper method is introduced so we can reuse some code.

BUG= 715630 

Change-Id: I5b3ab183e4db42ad3e9d864cd9e9634552cf90cc
Reviewed-on: https://chromium-review.googlesource.com/721576
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509251}
[modify] https://crrev.com/c60045ca9986b04f0469f9ab4733ae0c8c7c2d9a/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/c60045ca9986b04f0469f9ab4733ae0c8c7c2d9a/content/browser/download/download_manager_impl.h

Project Member

Comment 29 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38bb9881513858f32fe287c0ed0eb4f3a5d5a953

commit 38bb9881513858f32fe287c0ed0eb4f3a5d5a953
Author: Reilly Grant <reillyg@chromium.org>
Date: Tue Oct 17 20:10:12 2017

Disable failing content_browsertests with Network Service

These tests recently started to fail with the Network Service enabled.

TBR=qinmin@chromium.org
NOTRY=true

Bug:  715630 
Change-Id: Ic36c83567eda25423987d7b63d7bff1f10e7990d
Reviewed-on: https://chromium-review.googlesource.com/723673
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509499}
[modify] https://crrev.com/38bb9881513858f32fe287c0ed0eb4f3a5d5a953/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Project Member

Comment 30 by bugdroid1@chromium.org, Oct 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d17a27748b313da6233b4d8c6e8baa72ca8c2bd

commit 9d17a27748b313da6233b4d8c6e8baa72ca8c2bd
Author: Min Qin <qinmin@chromium.org>
Date: Thu Oct 19 17:54:33 2017

Network service: support pause/resume a download

Without network service, pausing a download is handled by DownloadRequestCore.
It defers reading the response until download is resumed.
With network service enabled, we can pause download by stopping reading from the data pipe.

BUG= 715630 

Change-Id: I231d273828a82a401a9cd3d2773fd75a3166cfbd
Reviewed-on: https://chromium-review.googlesource.com/726302
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510129}
[modify] https://crrev.com/9d17a27748b313da6233b4d8c6e8baa72ca8c2bd/content/browser/download/download_file.h
[modify] https://crrev.com/9d17a27748b313da6233b4d8c6e8baa72ca8c2bd/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/9d17a27748b313da6233b4d8c6e8baa72ca8c2bd/content/browser/download/download_file_impl.h
[modify] https://crrev.com/9d17a27748b313da6233b4d8c6e8baa72ca8c2bd/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/9d17a27748b313da6233b4d8c6e8baa72ca8c2bd/content/browser/download/download_job.cc
[modify] https://crrev.com/9d17a27748b313da6233b4d8c6e8baa72ca8c2bd/content/browser/download/mock_download_file.h

Project Member

Comment 31 by bugdroid1@chromium.org, Oct 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6bbcd694cd21e075da7bb719ce11f26c248acb6e

commit 6bbcd694cd21e075da7bb719ce11f26c248acb6e
Author: Min Qin <qinmin@chromium.org>
Date: Fri Oct 20 23:28:13 2017

Rewrite download browsertests that rely on TestDownloadRequestHandler

TestDownloadRequestHandler uses UrlRequestInterceptor to intercept request and inject errors.
This is not supported when network service is enabled.
This CL rewrotes all the browsertests relying on TestDownloadRequestHandler.
The EmbeddedTestServer will now return customizable http responses for those tests.
Because EmbeddedTestServer doesn't support error injection during,
this CL changes DownloadFileImpl to allow tests to inject errors into the DownloadFile directly.
All the tests behavior are kept the same before and after the CL.

BUG= 715630 

Change-Id: I782f492144ecd1d46b830700051263cdeede1be1
Reviewed-on: https://chromium-review.googlesource.com/722201
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510605}
[modify] https://crrev.com/6bbcd694cd21e075da7bb719ce11f26c248acb6e/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/6bbcd694cd21e075da7bb719ce11f26c248acb6e/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/6bbcd694cd21e075da7bb719ce11f26c248acb6e/content/browser/download/download_file_impl.h
[add] https://crrev.com/6bbcd694cd21e075da7bb719ce11f26c248acb6e/content/browser/download/test_download_http_response.cc
[add] https://crrev.com/6bbcd694cd21e075da7bb719ce11f26c248acb6e/content/browser/download/test_download_http_response.h
[modify] https://crrev.com/6bbcd694cd21e075da7bb719ce11f26c248acb6e/content/test/BUILD.gn

Project Member

Comment 32 by bugdroid1@chromium.org, Oct 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a03c827ec9eb1f0800d337377a2f6f1ee4663481

commit a03c827ec9eb1f0800d337377a2f6f1ee4663481
Author: Min Qin <qinmin@chromium.org>
Date: Wed Oct 25 18:58:30 2017

NetworkService: Enable all download browsertests that are passing

Lots of tests should pass after https://chromium-review.googlesource.com/c/chromium/src/+/722201
Enable them on the bot.

TBR=dtrainor@chromium.org
BUG= 715630 

Change-Id: I1017b96f0a72153c7d449ec4b6a1fa42d187d5a5
Reviewed-on: https://chromium-review.googlesource.com/734213
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511539}
[modify] https://crrev.com/a03c827ec9eb1f0800d337377a2f6f1ee4663481/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Project Member

Comment 33 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/83d0787d588c9c83233a916b8ac815898601b73c

commit 83d0787d588c9c83233a916b8ac815898601b73c
Author: Min Qin <qinmin@chromium.org>
Date: Thu Oct 26 23:22:41 2017

Network Service: Pass url chain to download

The url chain is used for download protection purpose.
Because redirect happens prior to the interception, so we need to pass the chain from navigation to download.

BUG= 715630 

Change-Id: I523d38d38ff7b1ad9f6f45d38be3b1c7eed6b98a
Reviewed-on: https://chromium-review.googlesource.com/736309
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512007}
[modify] https://crrev.com/83d0787d588c9c83233a916b8ac815898601b73c/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/83d0787d588c9c83233a916b8ac815898601b73c/content/browser/download/download_response_handler.cc
[modify] https://crrev.com/83d0787d588c9c83233a916b8ac815898601b73c/content/browser/download/download_response_handler.h
[modify] https://crrev.com/83d0787d588c9c83233a916b8ac815898601b73c/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/83d0787d588c9c83233a916b8ac815898601b73c/content/browser/download/resource_downloader.h
[modify] https://crrev.com/83d0787d588c9c83233a916b8ac815898601b73c/content/browser/loader/navigation_url_loader.h
[modify] https://crrev.com/83d0787d588c9c83233a916b8ac815898601b73c/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/83d0787d588c9c83233a916b8ac815898601b73c/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Project Member

Comment 34 by bugdroid1@chromium.org, Nov 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fbc6a26be69f18c87380789f5b19d3530bfd5dda

commit fbc6a26be69f18c87380789f5b19d3530bfd5dda
Author: Xing Liu <xingliu@chromium.org>
Date: Wed Nov 01 19:08:45 2017

Use embedded test server in chrome download_browsertest.

This CL converts more tests in chrome download_browsertest to use
embedded test server.

Also moves test_download_http_respons and slow_download_http_response
from content/browser/download to content/public/test to use them in
chrome layer download_browsertest.

Bug:  715630 
Change-Id: Iae780fb16a3cd0e9f16fb82244491452b246002a
Reviewed-on: https://chromium-review.googlesource.com/745728
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513206}
[modify] https://crrev.com/fbc6a26be69f18c87380789f5b19d3530bfd5dda/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/fbc6a26be69f18c87380789f5b19d3530bfd5dda/content/browser/download/download_browsertest.cc
[rename] https://crrev.com/fbc6a26be69f18c87380789f5b19d3530bfd5dda/content/public/test/slow_download_http_response.cc
[rename] https://crrev.com/fbc6a26be69f18c87380789f5b19d3530bfd5dda/content/public/test/slow_download_http_response.h
[rename] https://crrev.com/fbc6a26be69f18c87380789f5b19d3530bfd5dda/content/public/test/test_download_http_response.cc
[rename] https://crrev.com/fbc6a26be69f18c87380789f5b19d3530bfd5dda/content/public/test/test_download_http_response.h
[modify] https://crrev.com/fbc6a26be69f18c87380789f5b19d3530bfd5dda/content/test/BUILD.gn
[modify] https://crrev.com/fbc6a26be69f18c87380789f5b19d3530bfd5dda/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 35 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/34fe8f396f17b029d8fae8bd2e383a3f379be853

commit 34fe8f396f17b029d8fae8bd2e383a3f379be853
Author: Xing Liu <xingliu@chromium.org>
Date: Thu Nov 02 17:17:13 2017

Enable safe browsering download test for network servicification.

This CL enables a couple of safe browsering test cases.

After this merged, the only test left is
DownloadResourceThrottleCancels, because the new code didn't implement
the new throttle yet.


Bug:  715630 
Change-Id: I843b700d03074fa3aa5fd48a8cb2b5d5163215e1
Reviewed-on: https://chromium-review.googlesource.com/749726
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513526}
[modify] https://crrev.com/34fe8f396f17b029d8fae8bd2e383a3f379be853/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/34fe8f396f17b029d8fae8bd2e383a3f379be853/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 36 by bugdroid1@chromium.org, Nov 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e12ee453ae01d09933a0b024ca85054c3c8941f1

commit e12ee453ae01d09933a0b024ca85054c3c8941f1
Author: Xing Liu <xingliu@chromium.org>
Date: Fri Nov 03 17:52:01 2017

Enable two more download browsertests for network servicification.

Converts two more test to use embedded test server and enable them in
network service bot.

Also add some logic in TestFileErrorInjector, and refactor one class
in content/download_browsertests.cc to content/public/test/.

For DownloadHistoryCheck, previously we inject error from network layer,
now we inject error from TestFileErrorInjector in download file.

TBR=phajdan.jr@chromium.org

Bug:  715630 
Change-Id: I2339e4fd5ee49707ad7443882428e93c61a6e357
Reviewed-on: https://chromium-review.googlesource.com/749110
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513832}
[modify] https://crrev.com/e12ee453ae01d09933a0b024ca85054c3c8941f1/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/e12ee453ae01d09933a0b024ca85054c3c8941f1/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/e12ee453ae01d09933a0b024ca85054c3c8941f1/content/public/test/test_download_http_response.cc
[modify] https://crrev.com/e12ee453ae01d09933a0b024ca85054c3c8941f1/content/public/test/test_download_http_response.h
[modify] https://crrev.com/e12ee453ae01d09933a0b024ca85054c3c8941f1/content/public/test/test_file_error_injector.cc
[modify] https://crrev.com/e12ee453ae01d09933a0b024ca85054c3c8941f1/content/public/test/test_file_error_injector.h
[modify] https://crrev.com/e12ee453ae01d09933a0b024ca85054c3c8941f1/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 37 by bugdroid1@chromium.org, Nov 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a388657bd998723974fa1675cabb77e9ec2d8b77

commit a388657bd998723974fa1675cabb77e9ec2d8b77
Author: Xing Liu <xingliu@chromium.org>
Date: Fri Nov 03 23:58:39 2017

Converts more chrome download browsertest to use embedded test server.

This CL converts more tests to use embedded test server, and enable
them in network service bot.

Test cases in this CL are easy ones that can be directly converted
without changing error injection logic.

Bug:  715630 
Change-Id: I1036f96bcd0c69539a13c6f27892a719f4aa3751
Reviewed-on: https://chromium-review.googlesource.com/744644
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513972}
[modify] https://crrev.com/a388657bd998723974fa1675cabb77e9ec2d8b77/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/a388657bd998723974fa1675cabb77e9ec2d8b77/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 38 by bugdroid1@chromium.org, Nov 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8dcdec523ea5d34d5686f334b4b1325b4c9c80c

commit f8dcdec523ea5d34d5686f334b4b1325b4c9c80c
Author: Min Qin <qinmin@chromium.org>
Date: Sat Nov 04 15:17:27 2017

NetworkService: Populate fetch_error_body from DownloadUrlParameters to DownloadCreateInfo

DownloadResponseHandler takes the fetch_error_body value from DownloadUrlParameters.
However, it doesn't populate it to the DownloadCreateInfo.
And the DownloadItem will be created without setting fetch_error_body to true.

BUG= 715630 ,  769993 

Change-Id: Ia763b55ac363ad32cbf766755b49faa70b0647d2
Reviewed-on: https://chromium-review.googlesource.com/754331
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514051}
[modify] https://crrev.com/f8dcdec523ea5d34d5686f334b4b1325b4c9c80c/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/f8dcdec523ea5d34d5686f334b4b1325b4c9c80c/content/browser/download/download_response_handler.cc
[modify] https://crrev.com/f8dcdec523ea5d34d5686f334b4b1325b4c9c80c/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
Project Member

Comment 40 by bugdroid1@chromium.org, Nov 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/697ab2676c85ca4295a2ce38b1ef901a7653729e

commit 697ab2676c85ca4295a2ce38b1ef901a7653729e
Author: Xing Liu <xingliu@chromium.org>
Date: Thu Nov 09 19:15:15 2017

Clean up download browser tests.

This CL converts a few more tests to use embedded test server. Most of
them are not listed in the mojo filter but also not working correctly.


Change-Id: I02a9e0962a5b3efc61a3fbc05eb9df618c4fc1a7

Bug:  779121 ,  715630 
Change-Id: I02a9e0962a5b3efc61a3fbc05eb9df618c4fc1a7
Reviewed-on: https://chromium-review.googlesource.com/755839
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515233}
[modify] https://crrev.com/697ab2676c85ca4295a2ce38b1ef901a7653729e/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/697ab2676c85ca4295a2ce38b1ef901a7653729e/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 41 by bugdroid1@chromium.org, Nov 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa9ca62415a1963411ce10790190f532aaf91177

commit aa9ca62415a1963411ce10790190f532aaf91177
Author: Min Qin <qinmin@chromium.org>
Date: Thu Nov 09 22:27:07 2017

Network service: Interrupt download resumption if we got a redirect

When resuming a download, we should not expect a redirection as resumption
always uses the last url in the url chain.
So if we get a redirect, we should treat it as an interruption

BUG= 715630 

Change-Id: Id4795a89076fc30dfbc18c5a8ea97a9a8fdbec7b
Reviewed-on: https://chromium-review.googlesource.com/754288
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515321}
[modify] https://crrev.com/aa9ca62415a1963411ce10790190f532aaf91177/content/browser/download/download_response_handler.cc
[modify] https://crrev.com/aa9ca62415a1963411ce10790190f532aaf91177/content/browser/download/download_response_handler.h
[modify] https://crrev.com/aa9ca62415a1963411ce10790190f532aaf91177/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Blockedon: 783466
Project Member

Comment 43 by bugdroid1@chromium.org, Nov 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/098549fcb1bf89794e862d5bb7c071efabacc771

commit 098549fcb1bf89794e862d5bb7c071efabacc771
Author: Xing Liu <xingliu@chromium.org>
Date: Sat Nov 11 03:55:27 2017

Fix an issue in download with network service.

Download code uses frame_tree_node_id to find WebContents when browser
side navigation is enabled. The WebContents is used in UI layer code
to close the tab when the request is a download.


Bug:  715630 ,  781565 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ib9490a64692027e3b16c70b77b54aceebcfcb9b7
Reviewed-on: https://chromium-review.googlesource.com/761269
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515816}
[modify] https://crrev.com/098549fcb1bf89794e862d5bb7c071efabacc771/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/098549fcb1bf89794e862d5bb7c071efabacc771/content/browser/download/download_manager_impl.h
[modify] https://crrev.com/098549fcb1bf89794e862d5bb7c071efabacc771/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/098549fcb1bf89794e862d5bb7c071efabacc771/content/browser/download/resource_downloader.h
[modify] https://crrev.com/098549fcb1bf89794e862d5bb7c071efabacc771/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/098549fcb1bf89794e862d5bb7c071efabacc771/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 44 by bugdroid1@chromium.org, Nov 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f598d6acae1f05a650ed8325a0ab4e19d56482da

commit f598d6acae1f05a650ed8325a0ab4e19d56482da
Author: John Abd-El-Malek <jam@chromium.org>
Date: Mon Nov 13 02:57:11 2017

Revert "Fix an issue in download with network service."

This reverts commit 098549fcb1bf89794e862d5bb7c071efabacc771.

Reason for revert: Causing a bunch of test failures, see original commit's replies for list

Original change's description:
> Fix an issue in download with network service.
> 
> Download code uses frame_tree_node_id to find WebContents when browser
> side navigation is enabled. The WebContents is used in UI layer code
> to close the tab when the request is a download.
> 
> 
> Bug:  715630 ,  781565 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> Change-Id: Ib9490a64692027e3b16c70b77b54aceebcfcb9b7
> Reviewed-on: https://chromium-review.googlesource.com/761269
> Commit-Queue: Xing Liu <xingliu@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Min Qin <qinmin@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#515816}

TBR=jam@chromium.org,dtrainor@chromium.org,qinmin@chromium.org,xingliu@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  715630 ,  781565 
Change-Id: I5c969d11f411c5742dec2762017609db351d7a28
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Reviewed-on: https://chromium-review.googlesource.com/765388
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515879}
[modify] https://crrev.com/f598d6acae1f05a650ed8325a0ab4e19d56482da/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/f598d6acae1f05a650ed8325a0ab4e19d56482da/content/browser/download/download_manager_impl.h
[modify] https://crrev.com/f598d6acae1f05a650ed8325a0ab4e19d56482da/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/f598d6acae1f05a650ed8325a0ab4e19d56482da/content/browser/download/resource_downloader.h
[modify] https://crrev.com/f598d6acae1f05a650ed8325a0ab4e19d56482da/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/f598d6acae1f05a650ed8325a0ab4e19d56482da/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 45 by bugdroid1@chromium.org, Nov 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ab0e167df56d1354140dc43558c61e9db67d12f

commit 0ab0e167df56d1354140dc43558c61e9db67d12f
Author: Min Qin <qinmin@chromium.org>
Date: Tue Nov 14 21:57:19 2017

Network Service: Add a navigation throttle to intercept OMA DRM download

OMA(Open Mobile Alliance) DRM download(https://en.wikipedia.org/wiki/OMA_DRM) requires
the device to encrypt the downloaded media content with device keys.
So that the downloaded media file cannot be copied and played on another device.
Chrome doesn't have the capability to encrypt the content,
so the content has to be downloaded through Android DownloadManager.
This CL intercepts the download before it is intercepted by content::DownloadManagerImpl,
and passes the download URL to Android DownloadManager.

BUG= 715630 

Change-Id: I93adff6d275cc2feeacc3b4e8d7f2cf93b7f216f
Reviewed-on: https://chromium-review.googlesource.com/759326
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516444}
[modify] https://crrev.com/0ab0e167df56d1354140dc43558c61e9db67d12f/chrome/browser/BUILD.gn
[modify] https://crrev.com/0ab0e167df56d1354140dc43558c61e9db67d12f/chrome/browser/android/download/download_controller.cc
[modify] https://crrev.com/0ab0e167df56d1354140dc43558c61e9db67d12f/chrome/browser/android/download/download_controller_base.cc
[modify] https://crrev.com/0ab0e167df56d1354140dc43558c61e9db67d12f/chrome/browser/android/download/download_controller_base.h
[add] https://crrev.com/0ab0e167df56d1354140dc43558c61e9db67d12f/chrome/browser/android/download/intercept_oma_download_navigation_throttle.cc
[add] https://crrev.com/0ab0e167df56d1354140dc43558c61e9db67d12f/chrome/browser/android/download/intercept_oma_download_navigation_throttle.h
[modify] https://crrev.com/0ab0e167df56d1354140dc43558c61e9db67d12f/chrome/browser/chrome_content_browser_client.cc

Project Member

Comment 46 by bugdroid1@chromium.org, Nov 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18a8e089607563e923056b2c5d839e3c45c9dc62

commit 18a8e089607563e923056b2c5d839e3c45c9dc62
Author: Xing Liu <xingliu@chromium.org>
Date: Wed Nov 15 06:16:13 2017

[Reland] Fix an issue in download with network service.

This CL relands
https://chromium-review.googlesource.com/c/chromium/src/+/761269,
which is reverted in
https://chromium-review.googlesource.com/c/chromium/src/+/765388.

Also SBNavigationObserverBrowserTest.DownloadViaHTML5FileApiWithoutUserGesture
and SBNavigationObserverBrowserTest.DownloadViaHTML5FileApiWithUserGesture is
broken because of the reverted patch.
See https://chromium-review.googlesource.com/c/chromium/src/+/765229.

The code path from RenderFrameMessageFilter::DownloadUrl still needs
the render_frame_id to find the WebContents. In this case, the download
is not intercepted from navigation but from renderer, mostly triggered from javascript.

Bug:  715630 ,  781565 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo;master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I414c87f54d3f03a3060a38a9de402e02197ca445
Reviewed-on: https://chromium-review.googlesource.com/767852
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516595}
[modify] https://crrev.com/18a8e089607563e923056b2c5d839e3c45c9dc62/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/18a8e089607563e923056b2c5d839e3c45c9dc62/content/browser/download/download_manager_impl.h
[modify] https://crrev.com/18a8e089607563e923056b2c5d839e3c45c9dc62/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/18a8e089607563e923056b2c5d839e3c45c9dc62/content/browser/download/resource_downloader.h
[modify] https://crrev.com/18a8e089607563e923056b2c5d839e3c45c9dc62/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/18a8e089607563e923056b2c5d839e3c45c9dc62/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 47 by bugdroid1@chromium.org, Nov 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d5ec7d1e5b842420620d10b94eb8392cdaec5cf2

commit d5ec7d1e5b842420620d10b94eb8392cdaec5cf2
Author: Min Qin <qinmin@chromium.org>
Date: Mon Nov 20 23:27:52 2017

Switch to use EmbeddedTestServer for DragDownloadFileTest

URLRequestMockHTTPJob doesn't work with network service
This CL migrates the test to use EmbeddedTestServer.
So that the test can pass with/wihout network service enabled

BUG= 715630 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I4d380ebbe78f83ca59a921c8c1ff18fc5a598f16
Reviewed-on: https://chromium-review.googlesource.com/776055
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517986}
[modify] https://crrev.com/d5ec7d1e5b842420620d10b94eb8392cdaec5cf2/content/browser/download/drag_download_file_browsertest.cc
[modify] https://crrev.com/d5ec7d1e5b842420620d10b94eb8392cdaec5cf2/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Project Member

Comment 48 by bugdroid1@chromium.org, Nov 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1700169565263f14c7b0f2d1d9c1c6745a54dcf8

commit 1700169565263f14c7b0f2d1d9c1c6745a54dcf8
Author: John Abd-El-Malek <jam@chromium.org>
Date: Mon Nov 27 18:16:42 2017

Add a comment about why ResourceDownloader calls into BlobURLLoaderFactory.

ALso remove unused code to get the blob factory from URLLoaderFactoryGetter which isn't used.

BUG= 715630 

Change-Id: I1239399158b7c8cb04501fd00fa7e0079fec660f
Reviewed-on: https://chromium-review.googlesource.com/790810
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519340}
[modify] https://crrev.com/1700169565263f14c7b0f2d1d9c1c6745a54dcf8/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/1700169565263f14c7b0f2d1d9c1c6745a54dcf8/content/browser/download/resource_downloader.h

Project Member

Comment 49 by bugdroid1@chromium.org, Nov 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c3c19465bb078ed8c172d1ec56d7b9dbfd533551

commit c3c19465bb078ed8c172d1ec56d7b9dbfd533551
Author: Min Qin <qinmin@chromium.org>
Date: Tue Nov 28 00:02:57 2017

Network Service: All DownloadManager to throttle navigation interception

This CL adds the throttling logic for navigation interception with Network service enabled.
This matches the same logic used by DownloadResourceThrottle.
It will prompt user for storage permission, and limiting the number of download requests per tab.

BUG= 715630 

Change-Id: I88ad091972739e204ebf9b9720589bc10779eed4
Reviewed-on: https://chromium-review.googlesource.com/773483
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519463}
[modify] https://crrev.com/c3c19465bb078ed8c172d1ec56d7b9dbfd533551/chrome/browser/download/chrome_download_manager_delegate.cc
[modify] https://crrev.com/c3c19465bb078ed8c172d1ec56d7b9dbfd533551/chrome/browser/download/chrome_download_manager_delegate.h
[modify] https://crrev.com/c3c19465bb078ed8c172d1ec56d7b9dbfd533551/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/c3c19465bb078ed8c172d1ec56d7b9dbfd533551/content/browser/download/download_manager_impl.h
[modify] https://crrev.com/c3c19465bb078ed8c172d1ec56d7b9dbfd533551/content/browser/download/download_response_handler.cc
[modify] https://crrev.com/c3c19465bb078ed8c172d1ec56d7b9dbfd533551/content/browser/download/download_response_handler.h
[modify] https://crrev.com/c3c19465bb078ed8c172d1ec56d7b9dbfd533551/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/c3c19465bb078ed8c172d1ec56d7b9dbfd533551/content/browser/download/resource_downloader.h
[modify] https://crrev.com/c3c19465bb078ed8c172d1ec56d7b9dbfd533551/content/public/browser/download_manager_delegate.cc
[modify] https://crrev.com/c3c19465bb078ed8c172d1ec56d7b9dbfd533551/content/public/browser/download_manager_delegate.h

Project Member

Comment 50 by bugdroid1@chromium.org, Nov 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/181073ed08c38347bcf5b54b06fa60abde22e4db

commit 181073ed08c38347bcf5b54b06fa60abde22e4db
Author: Min Qin <qinmin@chromium.org>
Date: Thu Nov 30 20:15:26 2017

Enable DownloadTest.DownloadResourceThrottleCancels on mojo_linux bot

The test should pass now after https://chromium-review.googlesource.com/c/chromium/src/+/773483

BUG= 715630 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I1850aaf490b1920e9e2bc9766a6e291d0ba50b0f
Reviewed-on: https://chromium-review.googlesource.com/795037
Reviewed-by: Xing Liu <xingliu@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520658}
[modify] https://crrev.com/181073ed08c38347bcf5b54b06fa60abde22e4db/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/181073ed08c38347bcf5b54b06fa60abde22e4db/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 51 by bugdroid1@chromium.org, Dec 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75ed6df4d4024d1c6b73791101239907e0bf30a6

commit 75ed6df4d4024d1c6b73791101239907e0bf30a6
Author: Min Qin <qinmin@chromium.org>
Date: Fri Dec 01 20:39:15 2017

Rewrite MHTML download logic using ContentBrowserClient

The current logic uses ResourceDispatcherHostDelegate.
This doesn't work when network service is enabled.
This CL rewrites the logic using ContentBrowserClient,
so test can work with and without network service enabled

BUG= 715630 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I109a7f3fafe6a7d81a98ef39f2b828e7e0551cf0
Reviewed-on: https://chromium-review.googlesource.com/801370
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521047}
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.h
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/content/browser/loader/mime_sniffing_resource_handler.cc
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/content/public/browser/content_browser_client.h
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/content/public/browser/resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/content/public/browser/resource_dispatcher_host_delegate.h
[modify] https://crrev.com/75ed6df4d4024d1c6b73791101239907e0bf30a6/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Project Member

Comment 52 by bugdroid1@chromium.org, Dec 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2508f44b334e5dcf508ea57d1ed749dba5476c38

commit 2508f44b334e5dcf508ea57d1ed749dba5476c38
Author: Min Qin <qinmin@chromium.org>
Date: Tue Dec 05 19:27:58 2017

Network Service:Make network service process a privileged process

On Android, regular utility process is sandboxed,
and won't be able to access network.
This CL parses the sandbox type from the commandline,
and make the process a privileged process if the sandbox type is network.

BUG= 715630 

Change-Id: I5ed3edcf45de13aba838f7bbf3e0faf5e39568fb
Reviewed-on: https://chromium-review.googlesource.com/807152
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521781}
[modify] https://crrev.com/2508f44b334e5dcf508ea57d1ed749dba5476c38/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelper.java
[modify] https://crrev.com/2508f44b334e5dcf508ea57d1ed749dba5476c38/content/public/android/java/src/org/chromium/content/common/ContentSwitches.java
[modify] https://crrev.com/2508f44b334e5dcf508ea57d1ed749dba5476c38/net/android/java/src/org/chromium/net/X509Util.java
[modify] https://crrev.com/2508f44b334e5dcf508ea57d1ed749dba5476c38/net/cert/x509_util_android.cc

Project Member

Comment 53 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/30a78a1064e663e472771f4304ddbdae71a7e874

commit 30a78a1064e663e472771f4304ddbdae71a7e874
Author: Min Qin <qinmin@chromium.org>
Date: Wed Dec 06 01:29:13 2017

Network service: Moving user script download logic to ContentBrowserClient

This logic is currently implemented in ResourceDisaptcherHostDelegate,
which doesn't work with network service.
This CL moves the logic to ContentBrowserClient,
so the function can be invoked by both legacy and network service path.
A new DownloadTest is added to test this.

BUG= 715630 

Change-Id: I488858e16ef7d54e82405432b0d4efc9c2ca31eb
Reviewed-on: https://chromium-review.googlesource.com/806737
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521938}
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.h
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/content/browser/loader/mime_sniffing_resource_handler.cc
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/content/browser/loader/mime_sniffing_resource_handler_unittest.cc
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/content/browser/loader/mojo_async_resource_handler_unittest.cc
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/content/browser/loader/navigation_url_loader_network_service.h
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/content/public/browser/content_browser_client.h
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/content/public/browser/resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/30a78a1064e663e472771f4304ddbdae71a7e874/content/public/browser/resource_dispatcher_host_delegate.h

Project Member

Comment 54 by bugdroid1@chromium.org, Jan 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2755ff066c6d300b91c80aea9dea40b4cf2671be

commit 2755ff066c6d300b91c80aea9dea40b4cf2671be
Author: Min Qin <qinmin@chromium.org>
Date: Sat Jan 20 01:38:40 2018

Remove unused FileSystemContext from ResourceDownloader

FileSystemContext is no longer used in BlobURLLoaderFactory, remove it

BUG= 715630 

Change-Id: I6431b74883f750bf296c38f0cf260bcdede711f0
Reviewed-on: https://chromium-review.googlesource.com/876957
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530724}
[modify] https://crrev.com/2755ff066c6d300b91c80aea9dea40b4cf2671be/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/2755ff066c6d300b91c80aea9dea40b4cf2671be/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/2755ff066c6d300b91c80aea9dea40b4cf2671be/content/browser/download/resource_downloader.h

Cc: -rdsmith@chromium.org

Comment 56 by jam@chromium.org, May 16 2018

Blockedon: 843631

Comment 57 by dxie@chromium.org, May 18 2018

Labels: OS-Android

Comment 58 by dxie@chromium.org, May 29 2018

Labels: Hotlist-KnownIssue
An out-of-process network service is not ready to ship to production on android. Android currently assumes the only unsandboxed child process is GPU, and needs some work before supporting arbitrary unsandboxed utility processes.

I can't revert the existing behavior though because some browser tests became dependent on it. So just beware.

And talk to me if anyone want to work on making arbitrary unsandboxed utility processes work on android
Thanks Bo for the comment above, I've filed bug 872343 to track this specifically
Project Member

Comment 61 by bugdroid1@chromium.org, Aug 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9db74cc6b993f882d2e3ddf43370fbaed7d0e478

commit 9db74cc6b993f882d2e3ddf43370fbaed7d0e478
Author: John Abd-El-Malek <jam@chromium.org>
Date: Wed Aug 08 19:25:21 2018

Fix user gesture flag not being set on user-initiated downloads with network service enabled.

Bug:  776589 ,  715630 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I5a9bb9c857e3ec68df8c8ef1133cb418071f1e41
Reviewed-on: https://chromium-review.googlesource.com/1167553
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581659}
[modify] https://crrev.com/9db74cc6b993f882d2e3ddf43370fbaed7d0e478/chrome/browser/policy/policy_browsertest.cc
[modify] https://crrev.com/9db74cc6b993f882d2e3ddf43370fbaed7d0e478/components/download/internal/common/resource_downloader.cc
[modify] https://crrev.com/9db74cc6b993f882d2e3ddf43370fbaed7d0e478/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/9db74cc6b993f882d2e3ddf43370fbaed7d0e478/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Can we mark this as fixed now?
Status: Fixed (was: Assigned)
Done

Sign in to add a comment