Separate SwipeRefreshHandler and ContentViewCore

content/browser/renderer_host/render_widget_host_view_android.cc

Index: content/browser/renderer_host/render_widget_host_view_android.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc
index c0daedb3742ad67bd53a166e75877f14c7c4db38..915c03fcdb1cfbd5a86cd5b60a6158e4459c7eb5 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.cc
+++ b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -55,6 +55,7 @@
 #include "content/browser/renderer_host/input/synthetic_gesture_target_android.h"
 #include "content/browser/renderer_host/input/web_input_event_builders_android.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
+#include "content/browser/renderer_host/render_view_host_delegate_view.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/common/gpu_host_messages.h"
@@ -336,13 +337,6 @@ std::unique_ptr<ui::TouchSelectionController> CreateSelectionController(
   return base::MakeUnique<ui::TouchSelectionController>(client, config);
 }
 
-std::unique_ptr<OverscrollControllerAndroid> CreateOverscrollController(
-    ContentViewCoreImpl* content_view_core,
-    float dpi_scale) {
-  return base::MakeUnique<OverscrollControllerAndroid>(content_view_core,
-                                                       dpi_scale);
-}
-
 gfx::RectF GetSelectionRect(const ui::TouchSelectionController& controller) {
   gfx::RectF rect = controller.GetRectBetweenBounds();
   if (rect.IsEmpty())
@@ -469,6 +463,8 @@ RenderWidgetHostViewAndroid::RenderWidgetHostViewAndroid(
   host_->SetView(this);
   SetContentViewCore(content_view_core);
 
+  CreateOverscrollControllerIfPossible();
+
   if (GetTextInputManager())
     GetTextInputManager()->AddObserver(this);
 }
@@ -1220,6 +1216,11 @@ void RenderWidgetHostViewAndroid::SetSynchronousCompositorClient(
   }
 }
 
+void RenderWidgetHostViewAndroid::OnOverscrollRefreshHandlerSet() {
+  DCHECK(!overscroll_controller_);
+  CreateOverscrollControllerIfPossible();
+}
+
 bool RenderWidgetHostViewAndroid::SupportsAnimation() const {
   // The synchronous (WebView) compositor does not have a proper browser
   // compositor with which to drive animations.
@@ -1768,7 +1769,6 @@ void RenderWidgetHostViewAndroid::SetContentViewCore(
 
   bool resize = false;
   if (content_view_core != content_view_core_) {
-    overscroll_controller_.reset();
     selection_controller_.reset();
     ReleaseLocksOnSurface();
     // TODO(yusufo) : Get rid of the below conditions and have a better handling
@@ -1815,12 +1815,6 @@ void RenderWidgetHostViewAndroid::SetContentViewCore(
 
   if (!selection_controller_)
     selection_controller_ = CreateSelectionController(this, content_view_core_);
-
-  if (!overscroll_controller_ &&
-      view_.GetWindowAndroid()->GetCompositor()) {
-    overscroll_controller_ = CreateOverscrollController(
-        content_view_core_, ui::GetScaleFactorForNativeView(GetNativeView()));
-  }
 }
 
 void RenderWidgetHostViewAndroid::RunAckCallbacks() {
@@ -1847,6 +1841,7 @@ void RenderWidgetHostViewAndroid::OnGestureEvent(
 
 void RenderWidgetHostViewAndroid::OnContentViewCoreDestroyed() {
   SetContentViewCore(NULL);
+  overscroll_controller_.reset();
 }
 
 void RenderWidgetHostViewAndroid::OnCompositingDidCommit() {
@@ -1884,9 +1879,7 @@ void RenderWidgetHostViewAndroid::OnDetachedFromWindow() {
 
 void RenderWidgetHostViewAndroid::OnAttachCompositor() {
   DCHECK(content_view_core_);
-  if (!overscroll_controller_)
-    overscroll_controller_ = CreateOverscrollController(
-        content_view_core_, ui::GetScaleFactorForNativeView(GetNativeView()));
+  CreateOverscrollControllerIfPossible();
   ui::WindowAndroidCompositor* compositor =
       view_.GetWindowAndroid()->GetCompositor();
   delegated_frame_host_->RegisterFrameSinkHierarchy(
@@ -2000,4 +1993,40 @@ void RenderWidgetHostViewAndroid::ComputeEventLatencyOSTouchHistograms(
   }
 }
 
+void RenderWidgetHostViewAndroid::CreateOverscrollControllerIfPossible() {

[boliu, 2016/12/07 21:05:51]

also should call this in SetCVC when CVC is not null, matters for the case when
the compositor has already been created when SetCVC is called

[rlanday, 2016/12/07 22:13:39]

Ok

+  // an OverscrollController is already set
+  if (overscroll_controller_)
+    return;
+
+  RenderWidgetHostDelegate* delegate = host_->delegate();
+  if (!delegate)
+    return;
+
+  RenderViewHostDelegateView* delegate_view = delegate->GetDelegateView();
+  // render_widget_host_unittest.cc uses an object called
+  // MockRenderWidgetHostDelegate that does not have a DelegateView
+  if (!delegate_view)
+    return;
+
+  ui::OverscrollRefreshHandler* overscroll_refresh_handler =
+      delegate_view->GetOverscrollRefreshHandler();
+  if (!overscroll_refresh_handler)
+    return;
+
+  if (!content_view_core_)
+    return;
+
+  ui::WindowAndroid* window_android = content_view_core_->GetWindowAndroid();
+  if (!window_android)

[boliu, 2016/12/07 21:05:52]

this one can't happen

[rlanday, 2016/12/07 22:13:39]

Oh, I saw all the "if (GetWindowAndroid())" checks in ContentViewCoreImpl and
thought this might be necessary

[boliu, 2016/12/08 01:05:01]

Ohh, you are right. This can be null due to tab re-parenting, which is a user
action. But if we ever hit it here, we'd actually leave things in a bad state,
since we don't listen for WindowAndroid changing here.

Keep this check for now, with a comment to fix this edge case when we get
WindowAndroid from ViewAndroid..

+    return;
+
+  ui::WindowAndroidCompositor* compositor = window_android->GetCompositor();
+  if (!compositor)
+    return;
+
+  overscroll_controller_ = base::MakeUnique<OverscrollControllerAndroid>(
+      overscroll_refresh_handler,
+      compositor, ui::GetScaleFactorForNativeView(GetNativeView()));
+}
+
 }  // namespace content