New issue
Advanced search Search tips

Issue 617361 link

Starred by 2 users
Status: Fixed
Owner:
jinsuk...@chromium.org
Closed: Jun 2016
Cc:
qin...@chromium.org
jinsuk...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 598880
issue 617313



Sign in to add a comment

Android: Move DownloadControllerAndroid(Impl) from content to chrome/

Project Member Reported by siev...@chromium.org, Jun 4 2016 
This is mostly delegating and JNI glue specific to the Chrome for Android implementation.

Turns out that the only non-public dependency in DownloadControllerAndroidImpl is  ResourceDispatcherHostImpl::Get()->GetURLRequest() for cookies (the other includes in that file can all be trivially changed to use the content public base class API for each class).

Min said cookies are no longer needed as of M51 since we don't use the Android DownloadManager anymore. He will go ahead and remove that dependency.

Then moving all of this out of the content-layer will remove the need for the ContentViewDownloadDelegate interface and also the DownloadNotificationService and pretty much everythin moves up a layer and can be more self contained.

Comment 1 by siev...@chromium.org, Jun 4 2016

Blocking: 617313

Comment 2 by siev...@chromium.org, Jun 6 2016

Blocking: 598880

Comment 3 by jinsuk...@chromium.org, Jun 6 2016

Owner: jinsuk...@chromium.org

Comment 4 by jinsuk...@chromium.org, Jun 10 2016

Status: Started (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 21 2016 

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

commit 3da95ee0a9da46764cc912e3b36f29dd171eab40
Author: jinsukkim <jinsukkim@chromium.org>
Date: Tue Jun 21 22:34:46 2016

Move DownloadControllerAndroid from content/ to chrome/

This change lets embedder to decide how to handle downloads,
and makes content layer more self-contained. Accompanying changes are:

- Now WebContents holds the reference to download delegate via WebContentsUserData.
- ContentViewDownloadDelegate interface was removed
- DownloadControlerAndroid -> DownloadControllerBase
- DownloadControlerAndroidImpl -> DownloadController to
  have the name matched with the Java class

BUG= 598880 , 617313 ,  617361 

Review-Url: https://codereview.chromium.org/2014803002
Cr-Commit-Position: refs/heads/master@{#401127}

[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.h
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java
[rename] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadController.java
[rename] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadInfo.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadItem.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotifier.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntry.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSnackbarController.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUmaStatsEntry.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/OMADownloadHandler.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/java_sources.gni
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/javatests/src/org/chromium/chrome/browser/download/ChromeDownloadDelegateTest.java
[rename] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadInfoTest.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTestBase.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/android/javatests/src/org/chromium/chrome/browser/download/SystemDownloadNotifierTest.java
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/download/chrome_download_delegate.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/download/chrome_download_delegate.h
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.cc
[add] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/download/download_controller.cc
[add] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/download/download_controller.h
[add] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/download/download_controller_base.cc
[add] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/download/download_controller_base.h
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/download/download_manager_service.cc
[add] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/download/mock_download_controller.cc
[rename] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/download/mock_download_controller.h
[delete] https://crrev.com/3344627c11223d70ecd7338cf819743676044cf8/chrome/browser/android/download/mock_download_controller_android.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/intercept_download_resource_throttle.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/android/intercept_download_resource_throttle.h
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/download/download_resource_throttle.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/download/download_resource_throttle_unittest.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/download/download_ui_controller.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/favicon/content_favicon_driver_browsertest.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.h
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/browser/ui/android/context_menu_helper.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/chrome/chrome_browser.gypi
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/content/browser/android/browser_jni_registrar.cc
[delete] https://crrev.com/3344627c11223d70ecd7338cf819743676044cf8/content/browser/android/deferred_download_observer.cc
[delete] https://crrev.com/3344627c11223d70ecd7338cf819743676044cf8/content/browser/android/deferred_download_observer.h
[delete] https://crrev.com/3344627c11223d70ecd7338cf819743676044cf8/content/browser/android/download_controller_android_impl.h
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/content/content_browser.gypi
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/content/content_jni.gypi
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/content/public/android/BUILD.gn
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[delete] https://crrev.com/3344627c11223d70ecd7338cf819743676044cf8/content/public/android/java/src/org/chromium/content/browser/ContentViewDownloadDelegate.java
[delete] https://crrev.com/3344627c11223d70ecd7338cf819743676044cf8/content/public/browser/android/download_controller_android.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/content/public/browser/resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/3da95ee0a9da46764cc912e3b36f29dd171eab40/content/public/browser/resource_dispatcher_host_delegate.h

Comment 6 by jinsuk...@chromium.org, Jun 23 2016

Status: Fixed (was: Started)
►

Sign in to add a comment