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

Issue 617750 link

Starred by 2 users
Status: Fixed
Owner:
jinsuk...@chromium.org
Closed: Aug 2016
Cc:
█ hush@chromium.org
jinsuk...@chromium.org
wychen@chromium.org
boliu@chromium.org
█ sgu...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 622864
issue 598880
issue 620172



Sign in to add a comment

Android: Move ContentViewCore.ContentViewAndroidDelegate to embedder

Project Member Reported by siev...@chromium.org, Jun 6 2016 
The content layer shouldn't know so many details about the Android View system and WebView-specifics.

Make this an interface for adding and removing anchor views to be implemented by the embedder.

It also looks like the code might try to detect WebView vs. Chrome based on the View Layout in doSetAnchorViewPosition(), so the code is not even shared.

- Bo was also questioning the scroll adjustment logic offline.
- The support for changing container views is also specific to WebView.
- Furthermore it's questionable that using the old positioning info when moving to a new View is the right thing. Probably the popup would disappear anyway if you went fullscreen. (We tried html select on a tablet - since phone uses a modal dialog - with WebView and it did not work in general i.e. nothing showed up.)

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

Blocking: 598880

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

Cc: hush@chromium.org

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

Blocking: 620172

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

Cc: wychen@chromium.org

Comment 5 by siev...@chromium.org, Jun 23 2016

Labels: OS-Android

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

Blocking: 622864

Comment 7 by jinsuk...@chromium.org, Jun 27 2016

Owner: jinsuk...@chromium.org

Comment 8 by jinsuk...@chromium.org, Jul 1 2016

Status: Started (was: Available)
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 5 2016 

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

commit 9ee8865d84c2156728f1b9f78fb8f8b812e4ba19
Author: jinsukkim <jinsukkim@chromium.org>
Date: Fri Aug 05 06:20:09 2016

Factor out ContentViewAndroidDelegate

Took ContentViewAndroidDelegate out of ContentViewCore
so that embedders come with their own implementation of
ViewAndroidDelegate. This helps content layer
not have to know about Android View/WebView-specific details.

BUG= 617750 

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

[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/android_webview/BUILD.gn
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/android_webview/java/src/org/chromium/android_webview/AwAutofillClient.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/android_webview/java/src/org/chromium/android_webview/AwContents.java
[add] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/android_webview/java/src/org/chromium/android_webview/AwViewAndroidDelegate.java
[add] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsAnchorViewTest.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/android_webview/native/aw_autofill_client.cc
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/android_webview/native/aw_autofill_client.h
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/android_webview/test/BUILD.gn
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillPopupBridge.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chrome/android/java/src/org/chromium/chrome/browser/autofill/PasswordGenerationPopupBridge.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillTest.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chrome/android/javatests/src/org/chromium/chrome/browser/input/SelectPopupOtherContentViewTest.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chrome/browser/ui/android/autofill/autofill_popup_view_android.cc
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chrome/browser/ui/android/autofill/autofill_popup_view_android.h
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chrome/browser/ui/android/autofill/password_generation_popup_view_android.cc
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chrome/browser/ui/android/autofill/password_generation_popup_view_android.h
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWindowAndroid.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/content/public/android/java/src/org/chromium/content/browser/input/SelectPopupDropdown.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/content/public/android/javatests/src/org/chromium/content/browser/ContentViewCoreViewAndroidDelegateTest.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/content/public/browser/android/content_view_core.h
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/content/shell/android/java/src/org/chromium/content_shell/Shell.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/device/power_save_blocker/android/java/src/org/chromium/device/power_save_blocker/PowerSaveBlocker.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/device/power_save_blocker/power_save_blocker_android.cc
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/BUILD.gn
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/DEPS
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/java/src/org/chromium/ui/DropdownPopupWindow.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/java/src/org/chromium/ui/autofill/AutofillPopup.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/java/src/org/chromium/ui/base/WindowAndroid.java
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/ui_android.gyp
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/view_android.cc
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/view_android.h
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/window_android.cc
[modify] https://crrev.com/9ee8865d84c2156728f1b9f78fb8f8b812e4ba19/ui/android/window_android.h
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 11 2016 

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

commit 5ad16b4b8be2fb8d3890a984f043c843f3f536e8
Author: jinsukkim <jinsukkim@chromium.org>
Date: Thu Aug 11 04:03:30 2016

Remove |ContentViewCore.getViewAndroidDelegate()|

Removed the method since it is prone to misuse and now in use for
test classes only. The tests now define their own ViewAndroidDelegate
instance rather than the one defined internally but inaccessible
due to the removal. This does not make difference from the tests'
perspective because it is associated with the same
container view to acquire/remove anchor views.

BUG= 617750 

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

[modify] https://crrev.com/5ad16b4b8be2fb8d3890a984f043c843f3f536e8/android_webview/java/src/org/chromium/android_webview/AwViewAndroidDelegate.java
[modify] https://crrev.com/5ad16b4b8be2fb8d3890a984f043c843f3f536e8/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsAnchorViewTest.java
[modify] https://crrev.com/5ad16b4b8be2fb8d3890a984f043c843f3f536e8/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillTest.java
[modify] https://crrev.com/5ad16b4b8be2fb8d3890a984f043c843f3f536e8/content/public/android/BUILD.gn
[modify] https://crrev.com/5ad16b4b8be2fb8d3890a984f043c843f3f536e8/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[delete] https://crrev.com/3e84af8bf97ec8913818318f32d1accb0dba0ca0/content/public/android/javatests/src/org/chromium/content/browser/ContentViewCoreViewAndroidDelegateTest.java

Comment 11 by siev...@chromium.org, Aug 29 2016

Status: Fixed (was: Started)
►

Sign in to add a comment