New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 617313 link

Starred by 3 users
Status: Fixed
Owner:
jinsuk...@chromium.org
Closed: Jan 14
Cc:
tedc...@chromium.org
boliu@chromium.org
yfried...@chromium.org
█ ingem...@opera.com
changwan@chromium.org
ajit...@samsung.com
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocked on:
issue 598880
issue 617324
issue 617361


Sign in to add a comment

Android: Clean up Android specific content API

Project Member Reported by siev...@chromium.org, Jun 3 2016 
Master bug for cleaning up some of the content layer bits on Android, esp. those that involve Java and JNI.

The general idea here is to better separate
- calls into the content layer from the embedder vs.
- Android specific implementation details that are invoked from the content layer

Some of the existing code is a bit mixed. While we move out code from ContentViewCore ( crbug.com/598880 ), it makes sense to also revisit this a bit where feasible, for example clean up interfaces that go across boundaries, revisit what should be native and what should be Java at the interface level, and look at JNI and object ownership patterns.

This also involves continuing the work of fleshing out the 'content_public' Java package and enforcing this correctly with checkdeps. For that, private .java files should probably move from content/public to somewhere else in content/.

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

Owner: siev...@chromium.org
Status: Started (was: Untriaged)

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

Blockedon: 617324

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

Blockedon: 617361

Comment 4 by ajit...@samsung.com, Jun 6 2016

Cc: ajit...@samsung.com
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 siev...@chromium.org, Jun 23 2016

Labels: OS-Android
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 2 2016 

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

commit dbed3229058fc046539cd20cc546a740179d1fc4
Author: sievers <sievers@chromium.org>
Date: Tue Aug 02 17:29:58 2016

Android: Don't require RegisterNatives if there are none

Nowadays the calls up to Java are lazily resolved, so
RegisterNatives() actually does again what the name implies.

Allow not calling RegisterNatives() if there are no native methods
by marking it with the 'unused' attribute.

In follow-up patches all references will be removed so that the
generator can stop emiting the NOP functions where there are no
native methods.

This makes it more straightforward to design one-way JNI interfaces.

BUG= 603936 ,  617313 ,  617324 

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

[modify] https://crrev.com/dbed3229058fc046539cd20cc546a740179d1fc4/base/android/jni_generator/golden_sample_for_tests_jni.h
[modify] https://crrev.com/dbed3229058fc046539cd20cc546a740179d1fc4/base/android/jni_generator/jni_generator.py
[modify] https://crrev.com/dbed3229058fc046539cd20cc546a740179d1fc4/base/android/jni_generator/testCalledByNatives.golden
[modify] https://crrev.com/dbed3229058fc046539cd20cc546a740179d1fc4/base/android/jni_generator/testConstantsFromJavaP.golden
[modify] https://crrev.com/dbed3229058fc046539cd20cc546a740179d1fc4/base/android/jni_generator/testFromJavaP.golden
[modify] https://crrev.com/dbed3229058fc046539cd20cc546a740179d1fc4/base/android/jni_generator/testFromJavaPGenerics.golden
[modify] https://crrev.com/dbed3229058fc046539cd20cc546a740179d1fc4/base/android/jni_generator/testMultipleJNIAdditionalImport.golden
[modify] https://crrev.com/dbed3229058fc046539cd20cc546a740179d1fc4/base/android/jni_generator/testNativeExportsOnlyOption.golden
[modify] https://crrev.com/dbed3229058fc046539cd20cc546a740179d1fc4/base/android/jni_generator/testSingleJNIAdditionalImport.golden

Comment 8 by ingem...@opera.com, Sep 12 2016

Cc: ingem...@opera.com

Comment 9 by siev...@chromium.org, Oct 5 2016

Owner: boliu@chromium.org

Comment 10 by benhenry@google.com, Jan 11 

You started fixing this bug over two years ago. Are you still working on it?

Comment 11 by jinsuk...@chromium.org, Jan 14

Cc: -aelias@chromium.org -jinsuk...@chromium.org -yfried...@chromium.org jyfriedman@chromium.org boliu@chromium.org
Owner: jinsuk...@chromium.org
Status: Fixed (was: Started)
Let me mark this bug closed. Many of the changes made to fulfill this issue were logged in  Issue 598880  which was already fixed.

Comment 12 by jinsuk...@chromium.org, Jan 14

Cc: -jyfriedman@chromium.org yfried...@chromium.org
►

Sign in to add a comment