Refactor ContentViewClient (3/6)

content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java

Index: content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
index eece558099fe9f62138d81f9ae6693246cc4dd01..5c2e0fc5391b78fa3ffb99c1a02b8571815ebb90 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
@@ -8,7 +8,6 @@ import android.content.ActivityNotFoundException;
 import android.content.Context;
 import android.content.Intent;
 import android.view.KeyEvent;
-import android.view.View.MeasureSpec;
 
 import org.chromium.base.Log;
 import org.chromium.base.metrics.RecordUserAction;
@@ -24,40 +23,17 @@ import org.chromium.base.metrics.RecordUserAction;
  *  TODO(mkosiba): Rid this guy of default implementations. This class is used by both WebView and
  *  the browser and we don't want a the browser-specific default implementation to accidentally leak
  *  over to WebView.
+ *
+ *  WARNING: ConteViewClient is going away. Do not add new stuff in this class.
  */
 public class ContentViewClient {
     // Tag used for logging.
     private static final String TAG = "cr_ContentViewClient";
 
-    // Default value to signal that the ContentView's size should not be overridden.
-    private static final int UNSPECIFIED_MEASURE_SPEC =
-            MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED);
-
     private static final String GEO_SCHEME = "geo";
     private static final String TEL_SCHEME = "tel";
     private static final String MAILTO_SCHEME = "mailto";
 
-    /**
-     * Called whenever the background color of the page changes as notified by WebKit.
-     * @param color The new ARGB color of the page background.
-     */
-    public void onBackgroundColorChanged(int color) {
-    }
-
-    /**
-     * Notifies the client of the position of the top controls.
-     * @param topControlsOffsetY The Y offset of the top controls in physical pixels.
-     * @param topContentOffsetY The Y offset of the content in physical pixels.
-     */
-    public void onTopControlsChanged(float browserControlsOffsetY, float topContentOffsetY) {}
-
-    /**
-     * Notifies the client of the position of the bottom controls.
-     * @param bottomControlsOffsetY The Y offset of the bottom controls in physical pixels.
-     * @param bottomContentOffsetY The Y offset of the content in physical pixels.
-     */
-    public void onBottomControlsChanged(float bottomControlsOffsetY, float bottomContentOffsetY) { }
-
     public boolean shouldOverrideKeyEvent(KeyEvent event) {
         int keyCode = event.getKeyCode();
 
@@ -148,66 +124,4 @@ public class ContentViewClient {
         }
         return true;
     }
-
-    /**
-     * ContentViewClient users can return a custom value to override the width of
-     * the ContentView. By default, this method returns MeasureSpec.UNSPECIFIED, which
-     * indicates that the value should not be overridden.
-     *
-     * @return The desired width of the ContentView.
-     */
-    public int getDesiredWidthMeasureSpec() {

[boliu, 2016/11/30 23:22:35]

hmm, probably worthwhile to dig up why this was added as a callback from
content, this just seems like a super dumb way to set a view's size

these should just be explicit calls on ContentView to set its size instead

[Jinsuk Kim, 2016/12/01 07:59:38]

It was introduced to customize the view/physical backing size of contextual
search panel (of equal or smaller width/height), and now used for overlay panel
in general. This is for Chrome only, so doesn't have to be in content layer.
 
This is used in ContentView.onMeasure (for view size),
CompositorViewHolder.adjustPhysicalBackingSize. I'm not sure how to make a
explicit calls for these...  what I can think of alternatively is define
OverlayContentView inheriting from ContentView and override its onMeasure to
define its size.

For physical backing size, define a Chrome-only helper that contains the size
info and inject it into CompositorViewHolder. Let me know if it sound plausible,
or please elaborate your idea to help me get it better.

[boliu, 2016/12/01 23:57:52]

Looking at adjustPhysicalBackingSize, that's a classic example of why this API
is odd. Why is there a setter for PhysicalBackingSize that sometimes just
ignores the value that's passed in. That's just totally surprising behavior for
anyone who don't read the implementation.
+mdjones

[Jinsuk Kim, 2016/12/02 03:52:07]

Removed the interfaces getDesired*MeasureSpec() and add an api to ContentView to
set the desired size directly. Backing size update can make use of that too.
It's much simpler overall. And OverlayPanel doesn't need to provide
ContentViewClient any more. 

-        return UNSPECIFIED_MEASURE_SPEC;
-    }
-
-    /**
-     * ContentViewClient users can return a custom value to override the height of
-     * the ContentView. By default, this method returns MeasureSpec.UNSPECIFIED, which
-     * indicates that the value should not be overridden.
-     *
-     * @return The desired height of the ContentView.
-     */
-    public int getDesiredHeightMeasureSpec() {
-        return UNSPECIFIED_MEASURE_SPEC;
-    }
-
-    /**
-     * Returns the left system window inset in pixels. The system window inset represents the area
-     * of a full-screen window that is partially or fully obscured by the status bar, navigation
-     * bar, IME or other system windows.
-     * @return The left system window inset.
-     */
-    public int getSystemWindowInsetLeft() {
-        return 0;
-    }
-
-    /**
-     * Returns the top system window inset in pixels. The system window inset represents the area of
-     * a full-screen window that is partially or fully obscured by the status bar, navigation bar,
-     * IME or other system windows.
-     * @return The top system window inset.
-     */
-    public int getSystemWindowInsetTop() {
-        return 0;
-    }
-
-    /**
-     * Returns the right system window inset in pixels. The system window inset represents the area
-     * of a full-screen window that is partially or fully obscured by the status bar, navigation
-     * bar, IME or other system windows.
-     * @return The right system window inset.
-     */
-    public int getSystemWindowInsetRight() {
-        return 0;
-    }
-
-    /**
-     * Returns the bottom system window inset in pixels. The system window inset represents the area
-     * of a full-screen window that is partially or fully obscured by the status bar, navigation
-     * bar, IME or other system windows.
-     * @return The bottom system window inset.
-     */
-    public int getSystemWindowInsetBottom() {
-        return 0;
-    }
 }