Refactor ContentViewClient (3/6)

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

Index: content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
index 952f300d2e701811e9d720a2d1a3bea586a45b42..e6fc1f7a2de0a6835573285f4d2f8c1025ed2b22 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
@@ -701,6 +701,11 @@ public class ContentViewCore implements AccessibilityStateChangeListener, Displa
         }
     }
 
+    /* TODO(jinsukkim): Remove this method once view delegate is accessible from embedder. */
+    public ViewAndroidDelegate getViewDelegate() {

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

should not need this, this is supplied by the embedder, it should never have to
ask for it

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

It was added mostly for getDesiredMeasureSpec*() so can be reverted I think. I'm
also aware of your point (see the TODO) 

And one test
Ichrome/android/javatests/src/org/chromium/chrome/browser/OSKOverscrollTest.java)
needs to use it by way of 
viewCoreRef.get().getViewDelegate().getSystemWindowInsetBottom() will it be okay
to keep and mark it VisibleForTesting if this is necessary?

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

For the test, you can just have Tab keep a ref, and return it?

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

Ah right that would do. Done.

+        return mViewAndroidDelegate;
+    }
+
     /**
      * Sets a new container view for this {@link ContentViewCore}.
      *
@@ -888,11 +893,6 @@ public class ContentViewCore implements AccessibilityStateChangeListener, Displa
         return mContentViewClient;
     }
 
-    @CalledByNative
-    private void onBackgroundColorChanged(int color) {
-        getContentViewClient().onBackgroundColorChanged(color);
-    }
-
     /**
      * @return Viewport width in physical pixels as set from onSizeChanged.
      */
@@ -914,7 +914,7 @@ public class ContentViewCore implements AccessibilityStateChangeListener, Displa
      */
     @CalledByNative
     public int getViewportHeightWithOSKHiddenPix() {
-        return mViewportHeightPix + getContentViewClient().getSystemWindowInsetBottom();
+        return mViewportHeightPix + mViewAndroidDelegate.getSystemWindowInsetBottom();

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

these moved calls should be RWHVA->ViewAndroid->ViewAndroidDelegate, it should
not involve CVC at all

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

I also had tried so that VAD will handle |getViewportHeightWidhtOSKHiddenPix|.
It means mViewportHeightPix needs to move VAD too. So does
|getViewportHeightPix()|. Then 4 other classes should be updated and access VAD
to get the value... That made me decide to make this intermediate change and
take care of the rest better in follow up CLs.

     }
 
     /**
@@ -1979,11 +1979,11 @@ public class ContentViewCore implements AccessibilityStateChangeListener, Displa
 
         if (topBarChanged) {
             float topBarTranslate = topBarShownPix - browserControlsHeightDp * deviceScale;
-            getContentViewClient().onTopControlsChanged(topBarTranslate, topBarShownPix);
+            mViewAndroidDelegate.onTopControlsChanged(topBarTranslate, topBarShownPix);

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

how hard is it to go through ViewAndroid for these two calls, rather than CVC?

I think eventually, logic in this method should move to RWHVA and/or
ViewAndroid, perhaps should do that now?

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

Not impossible but hard to do in one step. I'm afraid it will make this CL
pretty big and somewhat beyond the scope I had in mind for this CL.
|updateFrameInfo| does many things here

- view/scroll update
- ImeAdpater
- RenderCoordinate
- Popup zoomer

to name a few. Eventually most of these will move to ViewAndroid like you said
but I'd like to do it in several steps.

If you really want to see the migration begin, what I can do is to duplicate the
call |content_view_core_->UpdateFrameInfo| in RWHVA::OnFrameMetadataUpdated()
and do |view_android.UpdateFrameInfo| too, and call up VAD only when top/bottom
bar has changed.

         }
         if (bottomBarChanged) {
             float bottomBarTranslate = bottomControlsHeightDp * deviceScale - bottomBarShownPix;
-            getContentViewClient().onBottomControlsChanged(bottomBarTranslate, bottomBarShownPix);
+            mViewAndroidDelegate.onBottomControlsChanged(bottomBarTranslate, bottomBarShownPix);
         }
 
         if (mBrowserAccessibilityManager != null) {