Separate SwipeRefreshHandler and ContentViewCore

chrome/android/java/src/org/chromium/chrome/browser/SwipeRefreshHandler.java

Index: chrome/android/java/src/org/chromium/chrome/browser/SwipeRefreshHandler.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/SwipeRefreshHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/SwipeRefreshHandler.java
index 6543a19062599063a3ac9df68aae16da4a9ff796..0991a72f377c8516f6e18d603576cb67e50b2d4b 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/SwipeRefreshHandler.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/SwipeRefreshHandler.java
@@ -8,6 +8,7 @@ import android.content.Context;
 import android.view.ViewGroup.LayoutParams;
 
 import org.chromium.base.TraceEvent;
+import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.tab.Tab;
@@ -51,6 +52,9 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
     // Accessibility utterance used to indicate refresh activation.
     private String mAccessibilityRefreshString;
 
+    // Pointer to the C++ SwipeRefreshHandler
+    private long mNativeSwipeRefreshHandler;
+
     /**
      * Simple constructor to use when creating an OverscrollRefresh instance from code.
      *
@@ -61,6 +65,8 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
         mTab = tab;
         mContentViewCore = mTab.getContentViewCore();
 
+        mNativeSwipeRefreshHandler = nativeInit();
+
         mSwipeRefreshLayout = new SwipeRefreshLayout(context);
         mSwipeRefreshLayout.setLayoutParams(
                 new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT));
@@ -100,8 +106,6 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
                 mSwipeRefreshLayout.post(mDetachLayoutRunnable);
             }
         });
-
-        mContentViewCore.setOverscrollRefreshHandler(this);
     }
 
     /**
@@ -111,7 +115,7 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
         setEnabled(false);
         cancelStopRefreshingRunnable();
         mSwipeRefreshLayout.setOnRefreshListener(null);
-        mContentViewCore.setOverscrollRefreshHandler(null);
+        nativeDestroy(mNativeSwipeRefreshHandler);
     }
 
     /**
@@ -127,12 +131,14 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
     }
 
     @Override
+    @CalledByNative
     public boolean start() {
         attachSwipeRefreshLayoutIfNecessary();
         return mSwipeRefreshLayout.start();
     }
 
     @Override
+    @CalledByNative
     public void pull(float delta) {
         TraceEvent.begin("SwipeRefreshHandler.pull");
         mSwipeRefreshLayout.pull(delta);
@@ -140,6 +146,7 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
     }
 
     @Override
+    @CalledByNative
     public void release(boolean allowRefresh) {
         TraceEvent.begin("SwipeRefreshHandler.release");
         mSwipeRefreshLayout.release(allowRefresh);
@@ -147,6 +154,7 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
     }
 
     @Override
+    @CalledByNative
     public void reset() {
         cancelStopRefreshingRunnable();
         mSwipeRefreshLayout.reset();
@@ -183,6 +191,10 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
         return mStopRefreshingRunnable;
     }
 
+    public long getNativePtr() {
+        return mNativeSwipeRefreshHandler;
+    }
+
     // The animation view is attached/detached on-demand to minimize overlap
     // with composited SurfaceView content.
     private void attachSwipeRefreshLayoutIfNecessary() {

[rlanday, 2016/11/28 19:23:54]

Unfortunately here and in the following method we're still depending on
mContentViewCore for attaching/detaching the swipe refresh layout view...I'll
look into what we can do about this

[rlanday, 2016/11/28 22:03:47]

I think the challenge here is that ContentViewCore's container view can change,
so I'm not sure how to remove this dependency right now. I think this is
ultimately going to have to be done as part of some larger refactoring effort
that threads these container view attach/detach events through a class other
than ContentViewCore.

[Jinsuk Kim, 2016/11/28 23:06:40]

Chrome's container view does not change; only WebView does. So it will be safe
to just pass the container view not entire CVC.

[Ted C, 2016/11/29 00:06:08]

Tab exposes getView(), so you might be able to add to it directly
instead of the CVC.  I don't know if we really support adding arbitrary
views to that class or if that might cause subtle issues.

+ianwen@ for that discussion.

[Ian Wen, 2016/11/29 22:34:18]

Tab#getView() returns a TabContentViewParent, which holds either a contentView
or a nativePageView as its first child, and an infobar container as its second
child. You could definitely try to add the swipe layout as its third view.

An alternative way is to let the TabContentViewParent know of the swipe layout,
and every time the tab's content is changed, it can add the swipe layout to the
new content view (either web content view or native page view).

A side effect of decoupling CVC with swipe handling is that we could finally use
swipe-to-refresh in native pages, which is very handy IMO. :)

@@ -198,4 +210,7 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
             mContentViewCore.getContainerView().removeView(mSwipeRefreshLayout);
         }
     }
+
+    private native long nativeInit();
+    private native void nativeDestroy(long nativeSwipeRefreshHandler);
 }