New issue
Advanced search Search tips

Issue 786197 link

Starred by 1 user
Status: Fixed
Owner:
jinsuk...@chromium.org
Closed: Nov 2017
Cc:
tedc...@chromium.org
boliu@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task



Sign in to add a comment

Turn Java ContentViewCore to an interface

Project Member Reported by jinsuk...@chromium.org, Nov 17 2017 
Java ContentViewCore class contains too much details on content view related stuff widely open to embedders. In order to stop it from exposing further inner workings and geting more bloated beyond content boundary, I'm thinking of splitting it into an interface and implementation (namely ContentViewCoreInternal or ContentViewCoreImpl), with the former defining tentative APIs consisting of the public methods in use at this moment, and disallow any more changes.

Once its interface is frozen, I can stop worrying about seeing people add more stuff and even out the ongoing breaking-up efforts. This approach is similar to what has been done to native  Issue 626764 . The next step is to remove APIs one at a time, and eventually eliminate the entire interface.

Comment 1 by jinsuk...@chromium.org, Nov 17 2017

Owner: jinsuk...@chromium.org
Status: Assigned (was: Available)
Project Member

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

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

commit f386e8f4cc527392cf5708cdf30a8fe0cec4399d
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Fri Nov 24 03:38:46 2017

Turn ContentViewCore to a public interface

Splits ContentViewCore class to a public interface (of a same
name) and its implementation (ContentViewCoreImpl). This split
intends to stop ContentViewCore from growing bigger with new
content APIs. In the end, the interface will be broken up
into more specific, well-defined ones.

Bug:  598880 ,  786197 
Change-Id: I30180b23e7a8e05b6c770dc790686df79b5ac4f2
Reviewed-on: https://chromium-review.googlesource.com/776405
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Stephen Lanham <slan@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519053}
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/chrome/android/javatests/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelEventFilterTest.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFakeServer.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/chrome/android/javatests/src/org/chromium/chrome/browser/input/SelectPopupOtherContentViewTest.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsActivity.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastWebContentsService.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/browser/android/content_view_core.cc
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/public/android/BUILD.gn
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[add] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/public/android/java/src/org/chromium/content/browser/ContentViewCoreImpl.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/public/android/java/src/org/chromium/content/browser/PopupZoomer.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/public/android/java/src/org/chromium/content/browser/input/TextSuggestionHost.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/public/test/android/BUILD.gn
[add] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewCore.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/shell/android/java/src/org/chromium/content_shell/Shell.java
[modify] https://crrev.com/f386e8f4cc527392cf5708cdf30a8fe0cec4399d/content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellActivityTestRule.java

Comment 3 by jinsuk...@chromium.org, Nov 24 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment