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..55b2cfc1dadda3c2d42e7c2e6a1bdae92ac2c4ba 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/SwipeRefreshHandler.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/SwipeRefreshHandler.java
@@ -5,15 +5,16 @@
 package org.chromium.chrome.browser;
 
 import android.content.Context;
+import android.view.ViewGroup;
 import android.view.ViewGroup.LayoutParams;
 
 import org.chromium.base.TraceEvent;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.tab.Tab;
-import org.chromium.content.browser.ContentViewCore;
-import org.chromium.content.browser.OverscrollRefreshHandler;
+import org.chromium.content_public.browser.WebContents;
 import org.chromium.third_party.android.swiperefresh.SwipeRefreshLayout;
+import org.chromium.ui.OverscrollRefreshHandler;
 
 /**
  * An overscroll handler implemented in terms a modified version of the Android
@@ -35,10 +36,9 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
     // The Tab where the swipe occurs.
     private Tab mTab;
 
-    // The ContentViewCore with which the handler is associated. The handler
-    // will set/unset itself as the default OverscrollRefreshHandler as the
-    // association changes.
-    private ContentViewCore mContentViewCore;
+    // The container view the SwipeRefreshHandler instance is currently
+    // associated with.
+    private ViewGroup mContainerView;
 
     // Async runnable for ending the refresh animation after the page first
     // loads a frame. This is used to provide a reasonable minimum animation time.
@@ -57,9 +57,9 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
      * @param context The associated context.
      * @param tab The Tab where the swipe occurs.
      */
-    public SwipeRefreshHandler(Context context, Tab tab) {
+    public SwipeRefreshHandler(final Context context, Tab tab, WebContents webContents) {

[Ted C, 2016/12/13 21:09:48]

from an earlier comment, but you can just do tab.getWebContents to not pass in
webContents.

it's odd to do getContentViewCore() but not getWebContents() IMO

(and if you needed it, the javadoc would need to be updated)

[rlanday, 2016/12/14 01:11:14]

Ok

         mTab = tab;
-        mContentViewCore = mTab.getContentViewCore();
+        mContainerView = mTab.getContentViewCore().getContainerView();
 
         mSwipeRefreshLayout = new SwipeRefreshLayout(context);
         mSwipeRefreshLayout.setLayoutParams(
@@ -78,8 +78,7 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
                         getStopRefreshingRunnable(), MAX_REFRESH_ANIMATION_DURATION_MS);
                 if (mAccessibilityRefreshString == null) {
                     int resId = R.string.accessibility_swipe_refresh;
-                    mAccessibilityRefreshString =
-                            mContentViewCore.getContext().getResources().getString(resId);
+                    mAccessibilityRefreshString = context.getResources().getString(resId);
                 }
                 mSwipeRefreshLayout.announceForAccessibility(mAccessibilityRefreshString);
                 mTab.reload();
@@ -100,8 +99,7 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
                 mSwipeRefreshLayout.post(mDetachLayoutRunnable);
             }
         });
-
-        mContentViewCore.setOverscrollRefreshHandler(this);
+        webContents.setOverscrollRefreshHandler(this);
     }
 
     /**
@@ -111,7 +109,6 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
         setEnabled(false);
         cancelStopRefreshingRunnable();
         mSwipeRefreshLayout.setOnRefreshListener(null);
-        mContentViewCore.setOverscrollRefreshHandler(null);

[Ted C, 2016/12/13 21:09:48]

we don't need to do webContents.setOverscrolllRefreshHandler(null) here?

[rlanday, 2016/12/14 01:11:14]

The code as written does not support doing that...there are several places where
we can call CreateOverscrollControllerIfPossible() (one of them being when
setOverscrollRefreshHandler() is called), but we don't have code to destroy the
controller if we unset the OverscrollRefreshHandler. I talked to boliu, he
argues that since SwipeRefreshHandler is only ever destroyed when the CVC is
about to be destroyed (inside Tab.destroyContentViewCore()), this doesn't really
matter.

     }
 
     /**
@@ -188,14 +185,14 @@ public class SwipeRefreshHandler implements OverscrollRefreshHandler {
     private void attachSwipeRefreshLayoutIfNecessary() {
         cancelDetachLayoutRunnable();
         if (mSwipeRefreshLayout.getParent() == null) {
-            mContentViewCore.getContainerView().addView(mSwipeRefreshLayout);
+            mContainerView.addView(mSwipeRefreshLayout);
         }
     }
 
     private void detachSwipeRefreshLayoutIfNecessary() {
         cancelDetachLayoutRunnable();
         if (mSwipeRefreshLayout.getParent() != null) {
-            mContentViewCore.getContainerView().removeView(mSwipeRefreshLayout);
+            mContainerView.removeView(mSwipeRefreshLayout);
         }
     }
 }