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

Issue 807155 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task

Blocked on:
issue 810720



Sign in to add a comment

Delete ContentView.java

Project Member Reported by jinsuk...@chromium.org, Jan 30 2018

Issue description

Java class ContentView is instantiated per WebContent, and is used by Content shell and Chrome for following purposes:

1) receives view-related events from Android framework through its parent, and forwards it to content (or ui/ i.e. EventForwarder) layer
2) acts as an Android View which various popup views anchor themselves on (a.k.a 'container view')
3) handles |onScrollChanged| from content layer, in response to |scrollBy| or |scrollTo| given before. Similar processing for a few other events by implementing interface |InternalAccessDelegate|
4) implements interface |SmartClipProvider|
5) Manages its own size info in case it's different from that of the parent view.
 
What content layer cares about is what it receives at ContentViewCore/EventForwarder. That makes ContentView arguably an embedder-specific implementation for handling all the actions above (note that WebView doesn't use it). This bug is to discuss how to replace what it has been doing in alternative approaches, and eventually delete the class (or move it out of content/). My initial thought for each use case is:

1) ContentView's parent (CompositorViewHolder) in Chrome should be able to choose the right CVC/EventForwarder from |WebContents| object, call their methods directly.

2) Ditto. Popups for a certain content set CompositorViewHolder for their container view. A caveat then is whenever active content changes, the popups for the old contents should be detached (not just hidden) from the parent view since the parent will be shared by all the contents - not clear if this is already handled.

3) From content layer's pov, it can still interact with whatever is passed to it as InternalAccessDelegate. Maybe CompositorViewHolder here again?

4) SmartClipProvider, from the comment ("Please make sure implementation of them is somewhere in the view hierarchy."), can be anywhere in the view hierarchy for the active content. The implementation may not be shared among multiple contents, so this rules out |CompositorViewHolder|. It may call for another view between content and embedder..?

5) Only required for contextual search. CompositorViewHolder is adjusting the physical backing size with the size info for contextual search (overlay panel). https://bugs.chromium.org/p/chromium/issues/detail?id=609332#c7 hints that the size difference (narrow width overlay panel) may be gone. Needs to keep track of the discussion. If it stays, the entire logic should be able to move to Chrome.



 

Comment 1 by boliu@chromium.org, Jan 30 2018

A lot of this is proposing to lift something that's per-webcontents to something that's per-window. That's a lot sketchy without actual detail of how that works.

> 1) ContentView's parent (CompositorViewHolder) in Chrome should be able to choose the right CVC/EventForwarder from |WebContents| object, call their methods directly.

That seems kinda of dangerous if it gets things wrong. Is there nothing that needs things from background webcontents? Also do we rely on ContentView for actual input routing right now?
> Is there nothing that needs things from background webcontents? Also do we rely on ContentView for actual input routing right now?

Good questions. I don't have answers for them now. What comes to mind about background webcontents is, maybe they would get |onScrollChanged| if their contents offset gets updated while in the background. But will it be a problem if the parent view handles it instead? From content's standpoint, it wouldn't matter as long as the offset gets reset when the active contents gets switched.

The least we can do would be to keep a per-content Android view but let the embedder defines and provides it to content, by having Chrome/Content shell come up with what's necessary and implement them separately.



Cc: donnd@chromium.org
Would appreciate input from donnd@ regarding 5)

Comment 4 by donnd@google.com, Jan 31 2018

Cc: twelling...@chromium.org
If CS is the only client that needs the size info then we should simply everything by removing it.  IIRC we don't need to support the narrow Overlay so this should be easy, and if we run into snags we can always close the overlay upon rotation.  Jinsuk, let me know if I can help!
The narrow panel should still be in use on tablets and non-Chrome Home devices. The new bottom sheet design is still in-progress, so it's not clear whether we'll need a narrow panel on phones long term.
Cc: mdjones@chromium.org
Thanks Don & Teresa for the feedback. Then I guess the the size info needs to be kept for now.

Also cc'ed mdjones@ for your thoughts, particuarly on 2) as there will be some gotchas lurking around..
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 3 2018

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

commit 1a37f6b6246e6ea38cea88fee9f28b6e92e548e7
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Sat Feb 03 00:57:53 2018

Make interface SmartClipProvider public

In preparation of moving the class ContentView out of content,
this CL makes the interface SmartClipProvider implemented by
ContentView public, and updates dependencies accordingly. With
this change, ContentView doesn't use non-public class any more.
Also found that there's no embedder using the public interface
SmartClipCallback. So deleted it.

Bug: 807155
Change-Id: Icac5956ccab217095d2216d65176e2e0d1665184
Reviewed-on: https://chromium-review.googlesource.com/894967
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534221}
[modify] https://crrev.com/1a37f6b6246e6ea38cea88fee9f28b6e92e548e7/android_webview/glue/java/DEPS
[modify] https://crrev.com/1a37f6b6246e6ea38cea88fee9f28b6e92e548e7/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/1a37f6b6246e6ea38cea88fee9f28b6e92e548e7/android_webview/java/DEPS
[modify] https://crrev.com/1a37f6b6246e6ea38cea88fee9f28b6e92e548e7/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/1a37f6b6246e6ea38cea88fee9f28b6e92e548e7/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/1a37f6b6246e6ea38cea88fee9f28b6e92e548e7/content/public/android/BUILD.gn
[modify] https://crrev.com/1a37f6b6246e6ea38cea88fee9f28b6e92e548e7/content/public/android/java/src/org/chromium/content/browser/ContentView.java
[modify] https://crrev.com/1a37f6b6246e6ea38cea88fee9f28b6e92e548e7/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
[delete] https://crrev.com/a5ad53267054dbf7229c254442798f41bee5c535/content/public/android/java/src/org/chromium/content_public/browser/SmartClipCallback.java
[rename] https://crrev.com/1a37f6b6246e6ea38cea88fee9f28b6e92e548e7/content/public/android/java/src/org/chromium/content_public/browser/SmartClipProvider.java

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 6 2018

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

commit d16b7adb4d804a4dd7ba4315cc3c323655f6d944
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Tue Feb 06 23:22:08 2018

CVC.isSelectPopupVisibleForTesting

This CL replaces |getSelectPopupForTesting| with the one that simply
returns its visibility (showing or hidden), which is enough for
testing purposes. It helps CVC not return/expose non-public class
when moved to content_public https://crrev.com/c/900884

Bug: 807155
Change-Id: I8dbdace17ace9068ee7807dfa1abf7ac85f4c357
Reviewed-on: https://chromium-review.googlesource.com/902982
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534829}
[modify] https://crrev.com/d16b7adb4d804a4dd7ba4315cc3c323655f6d944/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
[modify] https://crrev.com/d16b7adb4d804a4dd7ba4315cc3c323655f6d944/chrome/android/javatests/src/org/chromium/chrome/browser/input/SelectPopupOtherContentViewTest.java
[modify] https://crrev.com/d16b7adb4d804a4dd7ba4315cc3c323655f6d944/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/d16b7adb4d804a4dd7ba4315cc3c323655f6d944/content/public/android/java/src/org/chromium/content/browser/ContentViewCoreImpl.java
[modify] https://crrev.com/d16b7adb4d804a4dd7ba4315cc3c323655f6d944/content/public/android/javatests/src/org/chromium/content/browser/input/SelectPopupTest.java
[modify] https://crrev.com/d16b7adb4d804a4dd7ba4315cc3c323655f6d944/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewCore.java

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 7 2018

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

commit a441ab2a2de6290940ca8b8f776e25aa5297fc4a
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Wed Feb 07 05:41:56 2018

Move ContentViewCore to content_public

ContentViewCore is a public interface being used by
embedder, now relocatable to content_public. DEPS files are
updated as well.

Most of changes come from updates their import path accordingly.

TBR=sky@chromium.org,mthiesse@chromium.org

Bug: 807155
Change-Id: I0a564579e3b0e16b5210a7d0384896cbef231d23
Reviewed-on: https://chromium-review.googlesource.com/900884
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534922}
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/java/DEPS
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/java/src/org/chromium/android_webview/AwAutofillClient.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/java/src/org/chromium/android_webview/PopupTouchHandleDrawable.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/javatests/DEPS
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/javatests/src/org/chromium/android_webview/test/ContentViewMiscTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/javatests/src/org/chromium/android_webview/test/PlatformMediaCodecTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/javatests/src/org/chromium/android_webview/test/VisualStateTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/javatests/src/org/chromium/android_webview/test/WebViewWebVrTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/test/shell/DEPS
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/DEPS
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayContentDelegate.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanel.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContentViewDelegate.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManager.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerHost.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/eventfilter/OverlayPanelEventFilter.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ContextMenuItemDelegate.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/FullscreenHtmlApiHandler.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/infobar/PermissionUpdateInfoBarDelegate.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/modaldialog/TabModalPresenter.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/tab/TabObserver.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsDelegateAndroid.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelUtils.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/DEPS
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/ContentViewFocusTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/FullscreenActivityTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/OSKOverscrollTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/SelectFileDialogTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/TabsTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillKeyboardAccessoryTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupWithKeyboardTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelEventFilterTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/compositor/layouts/MockLayoutHost.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/fullscreen/FullscreenManagerTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/input/SelectPopupOtherContentViewTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestTestRule.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/TestFramework.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellControllerInputTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellNavigationTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellNavigationTestWebXr.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/TransitionUtils.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/VrTransitionUtils.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/XrTransitionUtils.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/sync_shell/javatests/DEPS
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/SyncTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsService.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsSurfaceHelper.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/BUILD.gn
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/java/src/org/chromium/content/browser/ContentView.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/java/src/org/chromium/content/browser/ContentViewCoreImpl.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/java/src/org/chromium/content/browser/input/SelectPopupDialog.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/java/src/org/chromium/content/browser/input/SelectPopupDropdown.java
[rename] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/java/src/org/chromium/content_public/browser/ContentViewCore.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/javatests/src/org/chromium/content/browser/ContentViewCoreSelectionTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/javatests/src/org/chromium/content/browser/ContentViewPopupZoomerTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/javatests/src/org/chromium/content/browser/ContentViewScrollingTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/javatests/src/org/chromium/content/browser/GestureDetectorResetTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/javatests/src/org/chromium/content/browser/NavigationTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/javatests/src/org/chromium/content/browser/accessibility/WebContentsAccessibilityTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/javatests/src/org/chromium/content/browser/input/SelectPopupTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/android/javatests/src/org/chromium/content/browser/input/TextSuggestionMenuTest.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/DOMUtils.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestCallbackHelperContainer.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewCore.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/shell/android/java/src/org/chromium/content_shell/Shell.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/shell/android/java/src/org/chromium/content_shell/ShellManager.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellActivityTestRule.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/shell/android/linker_test_apk/src/org/chromium/chromium_linker_test_apk/ChromiumLinkerTestActivity.java
[modify] https://crrev.com/a441ab2a2de6290940ca8b8f776e25aa5297fc4a/content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellActivity.java

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 8 2018

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

commit 19368f425059e9745499c6db63615d46d25235aa
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Thu Feb 08 02:21:18 2018

Make overlay panel size init logic simpler

Overlay panel initialization done through CompositorViewHolder
(LayoutManagerHost) can be made simpler, since its view/physical
backing size is set twice for narrow panel (first with the size
of parent view, and then updated to its actual, narrow size).

This CL gets the second init logic in OverlayPanelContent to work
for both (full/narrow panel) cases, deleting the init logic through:

OverlayContentDelegate.onContentViewCreated ->
OverlayPanelContentViewDelegate.setOverlayPanelContentViewCore ->
LayoutManagerHost.onOverlayPanelContentViewCoreAdded

With this CL, the overlay panel doesn't rely on its ContentView
(which is planned to be refactored) to determine backing/view
size in content layer any more.

Bug: 807155
Change-Id: I0858313249d9994268125b8b4cef56ec8774924b
Reviewed-on: https://chromium-review.googlesource.com/899892
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Donn Denman <donnd@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535263}
[modify] https://crrev.com/19368f425059e9745499c6db63615d46d25235aa/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java
[modify] https://crrev.com/19368f425059e9745499c6db63615d46d25235aa/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayContentDelegate.java
[modify] https://crrev.com/19368f425059e9745499c6db63615d46d25235aa/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanel.java
[modify] https://crrev.com/19368f425059e9745499c6db63615d46d25235aa/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
[modify] https://crrev.com/19368f425059e9745499c6db63615d46d25235aa/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContentViewDelegate.java
[modify] https://crrev.com/19368f425059e9745499c6db63615d46d25235aa/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManager.java
[modify] https://crrev.com/19368f425059e9745499c6db63615d46d25235aa/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerHost.java
[modify] https://crrev.com/19368f425059e9745499c6db63615d46d25235aa/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
[modify] https://crrev.com/19368f425059e9745499c6db63615d46d25235aa/chrome/android/javatests/src/org/chromium/chrome/browser/compositor/layouts/MockLayoutHost.java

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 9 2018

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

commit d1eed6b6c9554c5dc9b0902eac6703a36ba79eac
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Fri Feb 09 05:30:58 2018

Move ContentView to components/

ContentView is an Android View object passed by embedders
to content layer to provide connection to Android view hierarchy.
Essentially it does not belong to content. This CL defines
a new component 'content_view' and moves the class into it
so that the embedders (chrome/content shell, and chromecast)
can use it.

Bug: 807155
Change-Id: I24a190be0cb02cb3528140ae7738dc88fc3fd189
Reviewed-on: https://chromium-review.googlesource.com/905249
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Cait Phillips <caitkp@chromium.org>
Reviewed-by: Stephen Lanham <slan@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535652}
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/chrome/android/BUILD.gn
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/chrome/android/java/DEPS
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/chrome/android/javatests/DEPS
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/chrome/android/javatests/src/org/chromium/chrome/browser/input/SelectPopupOtherContentViewTest.java
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/chromecast/browser/android/BUILD.gn
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/chromecast/browser/android/DEPS
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsService.java
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsSurfaceHelper.java
[add] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/components/content_view/BUILD.gn
[add] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/components/content_view/DEPS
[add] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/components/content_view/OWNERS
[add] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/components/content_view/README
[rename] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/components/content_view/java/src/org/chromium/components/content_view/ContentView.java
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/content/public/android/BUILD.gn
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/content/shell/DEPS
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/content/shell/android/BUILD.gn
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/content/shell/android/java/src/org/chromium/content_shell/Shell.java
[modify] https://crrev.com/d1eed6b6c9554c5dc9b0902eac6703a36ba79eac/content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellActivityTestRule.java

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 9 2018

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

commit 3978107b5a40d808750a76166fea4b1ec3453575
Author: Tao Bai <michaelbai@chromium.org>
Date: Fri Feb 09 21:03:23 2018

Revert "Move ContentView to components/"

This reverts commit d1eed6b6c9554c5dc9b0902eac6703a36ba79eac.

Reason for revert: Break downstream bots

https://uberchromegw.corp.google.com/i/internal.client.clank/builders/arm-unpublished-builder/builds/8987

https://uberchromegw.corp.google.com/i/internal.client.clank/builders/arm-unpublished-builder-rel/builds/9960

See crbug.com/810720

Original change's description:
> Move ContentView to components/
> 
> ContentView is an Android View object passed by embedders
> to content layer to provide connection to Android view hierarchy.
> Essentially it does not belong to content. This CL defines
> a new component 'content_view' and moves the class into it
> so that the embedders (chrome/content shell, and chromecast)
> can use it.
> 
> Bug: 807155
> Change-Id: I24a190be0cb02cb3528140ae7738dc88fc3fd189
> Reviewed-on: https://chromium-review.googlesource.com/905249
> Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
> Reviewed-by: Bo <boliu@chromium.org>
> Reviewed-by: Cait Phillips <caitkp@chromium.org>
> Reviewed-by: Stephen Lanham <slan@chromium.org>
> Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#535652}

TBR=dgozman@chromium.org,caitkp@chromium.org,boliu@chromium.org,tedchoc@chromium.org,slan@chromium.org,jinsukkim@chromium.org

Change-Id: I2f55d49074ac3059f301edd3e070eef91335aea7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 807155
Reviewed-on: https://chromium-review.googlesource.com/912093
Reviewed-by: Tao Bai <michaelbai@chromium.org>
Commit-Queue: Tao Bai <michaelbai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535824}
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/chrome/android/BUILD.gn
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/chrome/android/java/DEPS
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/chrome/android/javatests/DEPS
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/chrome/android/javatests/src/org/chromium/chrome/browser/input/SelectPopupOtherContentViewTest.java
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/chromecast/browser/android/BUILD.gn
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/chromecast/browser/android/DEPS
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsService.java
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsSurfaceHelper.java
[delete] https://crrev.com/d0bdc3d320199055a00952199f82626b3869016b/components/content_view/BUILD.gn
[delete] https://crrev.com/d0bdc3d320199055a00952199f82626b3869016b/components/content_view/DEPS
[delete] https://crrev.com/d0bdc3d320199055a00952199f82626b3869016b/components/content_view/OWNERS
[delete] https://crrev.com/d0bdc3d320199055a00952199f82626b3869016b/components/content_view/README
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/content/public/android/BUILD.gn
[rename] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/content/public/android/java/src/org/chromium/content/browser/ContentView.java
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/content/shell/DEPS
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/content/shell/android/BUILD.gn
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/content/shell/android/java/src/org/chromium/content_shell/Shell.java
[modify] https://crrev.com/3978107b5a40d808750a76166fea4b1ec3453575/content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellActivityTestRule.java

Status: Assigned (was: Untriaged)
Blockedon: 810720
Project Member

Comment 15 by bugdroid1@chromium.org, Feb 22 2018

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

commit 71c0c57f52d2b1b4238360610edff6116ee9a9f2
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Thu Feb 22 01:00:20 2018

Reland "Move ContentView to components/"

Reverted at https://crrev.com/c/912093 but now the multidex
issue was resolved by https://crrev.com/c/911050.

TBRing since no change has been made to the original CL
https://crrev.com/c/905249.

TBR=boliu@chromium.org,caitkp@chromium.org,slan@chromium.org,dgozman@chromium.org,tedchoc@chromium.org

Bug: 807155
Change-Id: I3913529fdfa0b4b6540f67dca0a574f2ffe26866
Reviewed-on: https://chromium-review.googlesource.com/928100
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538285}
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/chrome/android/BUILD.gn
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/chrome/android/java/DEPS
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/chrome/android/javatests/DEPS
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/chrome/android/javatests/src/org/chromium/chrome/browser/input/SelectPopupOtherContentViewTest.java
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/chromecast/browser/android/BUILD.gn
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/chromecast/browser/android/DEPS
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsService.java
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsSurfaceHelper.java
[add] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/components/content_view/BUILD.gn
[add] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/components/content_view/DEPS
[add] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/components/content_view/OWNERS
[add] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/components/content_view/README
[rename] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/components/content_view/java/src/org/chromium/components/content_view/ContentView.java
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/content/public/android/BUILD.gn
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/content/shell/DEPS
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/content/shell/android/BUILD.gn
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/content/shell/android/java/src/org/chromium/content_shell/Shell.java
[modify] https://crrev.com/71c0c57f52d2b1b4238360610edff6116ee9a9f2/content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellActivityTestRule.java

Sign in to add a comment