Move observing ui::Compositor from FlingController to FlingScheduler

This should make it easier to have an Android implementation where
ui::Compositor is not available.

Bug: 848881
Change-Id: Ib7838f53b0935bf178954f1aa64bbf777cae3b6d
Reviewed-on: https://chromium-review.googlesource.com/1087807Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Sahel Sharifymoghaddam <sahel@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565053}

content/browser/renderer_host/input/fling_controller.cc

@@ -7,7 +7,6 @@
 #include "base/trace_event/trace_event.h"
 #include "content/browser/renderer_host/input/gesture_event_queue.h"
 #include "content/public/common/content_features.h"
-#include "ui/compositor/compositor.h"
 #include "ui/events/base_event_utils.h"
 #include "ui/events/blink/fling_booster.h"
 #include "ui/events/gestures/blink/web_gesture_curve_impl.h"
@@ -53,10 +52,7 @@ FlingController::FlingController(
   DCHECK(scheduler_client);
 }
 
-FlingController::~FlingController() {
-  if (compositor_)
-    compositor_->RemoveAnimationObserver(this);
-}
+FlingController::~FlingController() = default;
 
 bool FlingController::ShouldForwardForGFCFiltering(
     const GestureEventWithLatencyInfo& gesture_event) const {
@@ -441,26 +437,6 @@ gfx::Vector2dF FlingController::CurrentFlingVelocity() const {
   return current_fling_parameters_.velocity;
 }
 
-void FlingController::OnAnimationStep(base::TimeTicks timestamp) {
-  ProgressFling(timestamp);
-}
-
-void FlingController::OnCompositingShuttingDown(ui::Compositor* compositor) {
-  compositor->RemoveAnimationObserver(this);
-  compositor_ = nullptr;
-}
-
-void FlingController::SetCompositor(ui::Compositor* compositor) {
-  if (compositor) {
-    DCHECK(!compositor_);
-    compositor->AddAnimationObserver(this);
-  } else {
-    DCHECK(compositor_);
-    compositor_->RemoveAnimationObserver(this);
-  }
-  compositor_ = compositor;
-}
-
 TouchpadTapSuppressionController*
 FlingController::GetTouchpadTapSuppressionController() {
   return &touchpad_tap_suppression_controller_;

content/browser/renderer_host/input/fling_controller.h

@@ -8,14 +8,12 @@
 #include "content/browser/renderer_host/input/touchpad_tap_suppression_controller.h"
 #include "content/browser/renderer_host/input/touchscreen_tap_suppression_controller.h"
 #include "content/public/common/input_event_ack_state.h"
-#include "ui/compositor/compositor_animation_observer.h"
 
 namespace blink {
 class WebGestureCurve;
 }
 
 namespace ui {
-class Compositor;
 class FlingBooster;
 }
 
@@ -50,7 +48,7 @@ class CONTENT_EXPORT FlingControllerSchedulerClient {
       base::WeakPtr<FlingController> fling_controller) = 0;
 };
 
-class CONTENT_EXPORT FlingController : public ui::CompositorAnimationObserver {
+class CONTENT_EXPORT FlingController {
  public:
   struct CONTENT_EXPORT Config {
     Config();
@@ -78,7 +76,7 @@ class CONTENT_EXPORT FlingController : public ui::CompositorAnimationObserver {
                   FlingControllerSchedulerClient* scheduler_client,
                   const Config& config);
 
-  ~FlingController() override;
+  ~FlingController();
 
   // Used to progress an active fling on every begin frame.
   void ProgressFling(base::TimeTicks current_time);
@@ -99,8 +97,6 @@ class CONTENT_EXPORT FlingController : public ui::CompositorAnimationObserver {
 
   bool fling_in_progress() const { return fling_in_progress_; }
 
-  ui::Compositor* compositor() const { return compositor_; }
-
   bool FlingCancellationIsDeferred() const;
 
   bool TouchscreenFlingInProgress() const;
@@ -110,12 +106,6 @@ class CONTENT_EXPORT FlingController : public ui::CompositorAnimationObserver {
   // Returns the |TouchpadTapSuppressionController| instance.
   TouchpadTapSuppressionController* GetTouchpadTapSuppressionController();
 
-  // ui::CompositorAnimationObserver
-  void OnAnimationStep(base::TimeTicks timestamp) override;
-  void OnCompositingShuttingDown(ui::Compositor* compositor) override;
-
-  void SetCompositor(ui::Compositor* compositor);
-
  protected:
   std::unique_ptr<ui::FlingBooster> fling_booster_;
 
@@ -190,8 +180,6 @@ class CONTENT_EXPORT FlingController : public ui::CompositorAnimationObserver {
 
   bool send_wheel_events_nonblocking_;
 
-  ui::Compositor* compositor_ = nullptr;
-
   base::WeakPtrFactory<FlingController> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(FlingController);

content/browser/renderer_host/input/fling_scheduler.cc

@@ -18,35 +18,35 @@ namespace content {
 FlingScheduler::FlingScheduler(RenderWidgetHostImpl* host) : host_(host) {
   DCHECK(host);
 }
-FlingScheduler::~FlingScheduler() = default;
+
+FlingScheduler::~FlingScheduler() {
+  if (observed_compositor_)
+    observed_compositor_->RemoveAnimationObserver(this);
+}
 
 void FlingScheduler::ScheduleFlingProgress(
     base::WeakPtr<FlingController> fling_controller) {
   DCHECK(fling_controller);
   fling_controller_ = fling_controller;
+  // Don't do anything if a ui::Compositor is already being observed.
+  if (observed_compositor_)
+    return;
   ui::Compositor* compositor = GetCompositor();
-
-  if (compositor) {
-    // |fling_controller->SetCompositor(compositor)| adds the fling_controller
-    // as an animation observer to this compositor when fling_controller has
-    // called |ScheduleFlingProgress| for the first time.
-    if (!is_observer_added_) {
-      fling_controller->SetCompositor(compositor);
-      is_observer_added_ = true;
-    }
-  } else {  // No compositor is available.
+  // If a ui::Compositor can't be obtained, ask the host for BeginFrames.
+  if (!compositor) {
     host_->SetNeedsBeginFrameForFlingProgress();
+    return;
   }
+  compositor->AddAnimationObserver(this);
+  observed_compositor_ = compositor;
 }
+
 void FlingScheduler::DidStopFlingingOnBrowser(
     base::WeakPtr<FlingController> fling_controller) {
   DCHECK(fling_controller);
-  if (GetCompositor()) {
-    // |fling_controller->SetCompositor(nullptr)| removes the fling_controller
-    // as an animation observer from its current compositor when flinging has
-    // stopped.
-    fling_controller->SetCompositor(nullptr);
-    is_observer_added_ = false;
+  if (observed_compositor_) {
+    observed_compositor_->RemoveAnimationObserver(this);
+    observed_compositor_ = nullptr;
   }
   fling_controller_ = nullptr;
   host_->DidStopFlinging();
@@ -59,7 +59,7 @@ void FlingScheduler::ProgressFlingOnBeginFrameIfneeded(
     return;
 
   // FlingProgress will be called within FlingController::OnAnimationStep.
-  if (is_observer_added_)
+  if (observed_compositor_)
     return;
 
   fling_controller_->ProgressFling(current_time);
@@ -77,4 +77,14 @@ ui::Compositor* FlingScheduler::GetCompositor() {
   return nullptr;
 }
 
+void FlingScheduler::OnAnimationStep(base::TimeTicks timestamp) {
+  fling_controller_->ProgressFling(timestamp);
+}
+
+void FlingScheduler::OnCompositingShuttingDown(ui::Compositor* compositor) {
+  DCHECK_EQ(observed_compositor_, compositor);
+  observed_compositor_->RemoveAnimationObserver(this);
+  observed_compositor_ = nullptr;
+}
+
 }  // namespace content

content/browser/renderer_host/input/fling_scheduler.h

@@ -6,6 +6,7 @@
 #define CONTENT_BROWSER_RENDERER_HOST_INPUT_FLING_SCHEDULER_H_
 
 #include "content/browser/renderer_host/input/fling_controller.h"
+#include "ui/compositor/compositor_animation_observer.h"
 
 namespace ui {
 class Compositor;
@@ -15,7 +16,8 @@ namespace content {
 
 class RenderWidgetHostImpl;
 
-class CONTENT_EXPORT FlingScheduler : public FlingControllerSchedulerClient {
+class CONTENT_EXPORT FlingScheduler : public FlingControllerSchedulerClient,
+                                      private ui::CompositorAnimationObserver {
  public:
   FlingScheduler(RenderWidgetHostImpl* host);
   ~FlingScheduler() override;
@@ -32,12 +34,16 @@ class CONTENT_EXPORT FlingScheduler : public FlingControllerSchedulerClient {
   virtual ui::Compositor* GetCompositor();
   RenderWidgetHostImpl* host_;
   base::WeakPtr<FlingController> fling_controller_;
+  ui::Compositor* observed_compositor_ = nullptr;
 
  private:
-  bool is_observer_added_ = false;
+  // ui::CompositorAnimationObserver
+  void OnAnimationStep(base::TimeTicks timestamp) override;
+  void OnCompositingShuttingDown(ui::Compositor* compositor) override;
 
   DISALLOW_COPY_AND_ASSIGN(FlingScheduler);
 };
+
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_INPUT_FLING_SCHEDULER_H_

content/browser/renderer_host/input/fling_scheduler_unittest.cc

@@ -33,6 +33,7 @@ class FakeFlingScheduler : public FlingScheduler {
   bool fling_in_progress() const { return fling_in_progress_; }
 
   ui::Compositor* compositor() { return GetCompositor(); }
+  ui::Compositor* observed_compositor() { return observed_compositor_; }
 
   base::WeakPtr<FlingController> fling_controller() const {
     return fling_controller_;
@@ -136,7 +137,8 @@ TEST_F(FlingSchedulerTest, ScheduleNextFlingProgress) {
   EXPECT_TRUE(fling_scheduler_->fling_in_progress());
   EXPECT_EQ(fling_controller_.get(),
             fling_scheduler_->fling_controller().get());
-  EXPECT_EQ(fling_scheduler_->compositor(), fling_controller_->compositor());
+  EXPECT_EQ(fling_scheduler_->compositor(),
+            fling_scheduler_->observed_compositor());
 
   progress_time += base::TimeDelta::FromMilliseconds(17);
   fling_controller_->ProgressFling(progress_time);
@@ -148,12 +150,13 @@ TEST_F(FlingSchedulerTest, FlingCancelled) {
   EXPECT_TRUE(fling_scheduler_->fling_in_progress());
   EXPECT_EQ(fling_controller_.get(),
             fling_scheduler_->fling_controller().get());
-  EXPECT_EQ(fling_scheduler_->compositor(), fling_controller_->compositor());
+  EXPECT_EQ(fling_scheduler_->compositor(),
+            fling_scheduler_->observed_compositor());
 
   SimulateFlingCancel();
   EXPECT_FALSE(fling_scheduler_->fling_in_progress());
   EXPECT_EQ(nullptr, fling_scheduler_->fling_controller());
-  EXPECT_EQ(nullptr, fling_controller_->compositor());
+  EXPECT_EQ(nullptr, fling_scheduler_->observed_compositor());
 }
 
 }  // namespace content