Revert "Remove set_owned_by_client() from AutoclickScrollPositionHandler."

This reverts commit be2aab1c.

Reason for revert: Step "compile" failing on 3 builder(s)

Builder: chromeos-betty-pi-arc-cfi-thin-lto-chrome
https://ci.chromium.org/p/chrome/builders/ci/chromeos-betty-pi-arc-cfi-thin-lto-chrome
First failing build:
https://ci.chromium.org/p/chrome/builders/ci/chromeos-betty-pi-arc-cfi-thin-lto-chrome/b8879666664893307472
--------------------
Builder: chromeos-arm-generic-cfi-thin-lto-chrome
https://ci.chromium.org/p/chrome/builders/ci/chromeos-arm-generic-cfi-thin-lto-chrome
First failing build:
https://ci.chromium.org/p/chrome/builders/ci/chromeos-arm-generic-cfi-thin-lto-chrome/b8879668479378837648
--------------------
Builder: chromeos-eve-chrome
https://ci.chromium.org/p/chrome/builders/ci/chromeos-eve-chrome
First failing build:
https://ci.chromium.org/p/chrome/builders/ci/chromeos-eve-chrome/b8879665644223805168


Original change's description:
> Remove set_owned_by_client() from AutoclickScrollPositionHandler.
> 
> It's safe for the Widget to own the underlying View via
> SetContentsView().  This was not immediately obvious; much of this CL is
> adjacent refactoring and cleanup that simplifies the ownership/lifetime.
> 
> Bug: 1044687
> Change-Id: I76cbc402edb7195c42cbae679a89839384d0615a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2210914
> Commit-Queue: Peter Kasting <pkasting@chromium.org>
> Auto-Submit: Peter Kasting <pkasting@chromium.org>
> Reviewed-by: Katie Dektar <katie@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#771122}

TBR=pkasting@chromium.org,katie@chromium.org

Change-Id: I7a18d84cfae23938b683f9a3aaf7b533781b975a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1044687
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212824Reviewed-by: Ramya Nagarajan <ramyan@chromium.org>
Commit-Queue: Ramya Nagarajan <ramyan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771158}

ash/autoclick/autoclick_controller.cc

@@ -52,7 +52,8 @@ base::TimeDelta CalculateStartGestureDelay(base::TimeDelta total_delay) {
 }
 
 views::Widget::InitParams CreateAutoclickOverlayWidgetParams(
-    aura::Window* root_window) {
+    aura::Window* target) {
+  aura::Window* root_window = target->GetRootWindow();
   views::Widget::InitParams params;
   params.type = views::Widget::InitParams::TYPE_WINDOW_FRAMELESS;
   params.accept_events = false;
@@ -127,7 +128,7 @@ void AutoclickController::SetEnabled(bool enabled,
     menu_bubble_controller_->ShowBubble(event_type_, menu_position_);
     if (event_type_ == AutoclickEventType::kScroll) {
       InitializeScrollLocation();
-      UpdateScrollPosition();
+      UpdateScrollPosition(scroll_location_);
     }
     enabled_ = enabled;
   } else {
@@ -189,7 +190,7 @@ void AutoclickController::SetAutoclickEventType(AutoclickEventType type) {
 
   if (type == AutoclickEventType::kScroll) {
     InitializeScrollLocation();
-    UpdateScrollPosition();
+    UpdateScrollPosition(scroll_location_);
   } else {
     over_scroll_button_ = false;
     HideScrollPosition();
@@ -289,12 +290,32 @@ void AutoclickController::UpdateAutoclickMenuBoundsIfNeeded() {
     menu_bubble_controller_->SetPosition(menu_position_);
 }
 
+void AutoclickController::CreateAutoclickRingWidget(
+    const gfx::Point& point_in_screen) {
+  aura::Window* target = window_util::GetRootWindowAt(point_in_screen);
+  SetTapDownTarget(target);
+  ring_widget_.reset(new views::Widget);
+  ring_widget_->Init(CreateAutoclickOverlayWidgetParams(target));
+  ring_widget_->SetOpacity(1.f);
+}
+
+void AutoclickController::CreateAutoclickScrollPositionWidget(
+    const gfx::Point& point_in_screen) {
+  aura::Window* target = window_util::GetRootWindowAt(point_in_screen);
+  SetTapDownTarget(target);
+  scroll_position_widget_.reset(new views::Widget);
+  scroll_position_widget_->Init(CreateAutoclickOverlayWidgetParams(target));
+}
+
 void AutoclickController::UpdateAutoclickWidgetPosition(
-    gfx::NativeView native_view,
-    aura::Window* root_window) {
-  if (native_view->GetRootWindow() != root_window) {
+    views::Widget* widget,
+    const gfx::Point& point_in_screen) {
+  aura::Window* target = window_util::GetRootWindowAt(point_in_screen);
+  SetTapDownTarget(target);
+  aura::Window* root_window = target->GetRootWindow();
+  if (widget->GetNativeView()->GetRootWindow() != root_window) {
     views::Widget::ReparentNativeView(
-        native_view,
+        widget->GetNativeView(),
         Shell::GetContainer(root_window, kShellWindowId_OverlayContainer));
   }
 }
@@ -335,7 +356,7 @@ void AutoclickController::DoAutoclickAction() {
     } else {
       scroll_location_ = gesture_anchor_location_;
       is_initial_scroll_location_ = false;
-      UpdateScrollPosition();
+      UpdateScrollPosition(scroll_location_);
       Shell::Get()
           ->accessibility_controller()
           ->RequestAutoclickScrollableBoundsForPoint(scroll_location_);
@@ -479,17 +500,11 @@ void AutoclickController::InitClickTimers() {
                           base::Unretained(this)));
 }
 
-void AutoclickController::UpdateRingWidget() {
-  aura::Window* const target =
-      window_util::GetRootWindowAt(last_mouse_location_);
-  SetTapDownTarget(target);
-  aura::Window* const root_window = target->GetRootWindow();
-  if (ring_widget_) {
-    UpdateAutoclickWidgetPosition(ring_widget_->GetNativeView(), root_window);
+void AutoclickController::UpdateRingWidget(const gfx::Point& point_in_screen) {
+  if (!ring_widget_) {
+    CreateAutoclickRingWidget(point_in_screen);
   } else {
-    ring_widget_ = std::make_unique<views::Widget>(
-        CreateAutoclickOverlayWidgetParams(root_window));
-    ring_widget_->SetOpacity(1.0f);
+    UpdateAutoclickWidgetPosition(ring_widget_.get(), point_in_screen);
   }
 }
 
@@ -507,28 +522,29 @@ void AutoclickController::InitializeScrollLocation() {
       ->RequestAutoclickScrollableBoundsForPoint(scroll_location_);
 }
 
-void AutoclickController::UpdateScrollPosition() {
+void AutoclickController::UpdateScrollPosition(
+    const gfx::Point& point_in_screen) {
   if (!enabled_)
     return;
-
-  aura::Window* const target = window_util::GetRootWindowAt(scroll_location_);
-  SetTapDownTarget(target);
-  aura::Window* const root_window = target->GetRootWindow();
-  if (autoclick_scroll_position_handler_) {
-    UpdateAutoclickWidgetPosition(
-        autoclick_scroll_position_handler_->GetNativeView(), root_window);
-  } else {
+  if (!scroll_position_widget_) {
+    CreateAutoclickScrollPositionWidget(point_in_screen);
     autoclick_scroll_position_handler_ =
         std::make_unique<AutoclickScrollPositionHandler>(
-            std::make_unique<views::Widget>(
-                CreateAutoclickOverlayWidgetParams(root_window)));
+            scroll_location_, scroll_position_widget_.get());
+  } else {
+    UpdateAutoclickWidgetPosition(scroll_position_widget_.get(),
+                                  point_in_screen);
+    autoclick_scroll_position_handler_->SetCenter(
+        scroll_location_, scroll_position_widget_.get());
   }
-  autoclick_scroll_position_handler_->SetScrollPointCenterInScreen(
-      scroll_location_);
 }
 
 void AutoclickController::HideScrollPosition() {
+  // Hide the scroll position UI if it exists.
+  if (autoclick_scroll_position_handler_)
     autoclick_scroll_position_handler_.reset();
+  if (scroll_position_widget_)
+    scroll_position_widget_.reset();
 
   // TODO(katie): Clear any Autoclick scroll focus rings here.
 }
@@ -595,7 +611,7 @@ void AutoclickController::OnMouseEvent(ui::MouseEvent* event) {
         drag_event_rewriter_->IsEnabled()))) {
     mouse_event_flags_ = event->flags();
     // Update the point even if the animation is not currently being shown.
-    UpdateRingWidget();
+    UpdateRingWidget(last_mouse_location_);
 
     // The distance between the mouse location and the anchor location
     // must exceed a certain threshold to initiate a new autoclick countdown.

ash/autoclick/autoclick_controller.h

@@ -13,7 +13,6 @@
 #include "ui/aura/window_observer.h"
 #include "ui/events/event_handler.h"
 #include "ui/gfx/geometry/point.h"
-#include "ui/gfx/native_widget_types.h"
 
 namespace base {
 class RetainingOneShotTimer;
@@ -120,17 +119,19 @@ class ASH_EXPORT AutoclickController
 
  private:
   void SetTapDownTarget(aura::Window* target);
-  void UpdateAutoclickWidgetPosition(gfx::NativeView native_view,
-                                     aura::Window* root_window);
+  void CreateAutoclickRingWidget(const gfx::Point& point_in_screen);
+  void CreateAutoclickScrollPositionWidget(const gfx::Point& point_in_screen);
+  void UpdateAutoclickWidgetPosition(views::Widget* widget,
+                                     const gfx::Point& point_in_screen);
   void DoAutoclickAction();
   void StartAutoclickGesture();
   void CancelAutoclickAction();
   void OnActionCompleted(AutoclickEventType event_type);
   void InitClickTimers();
-  void UpdateRingWidget();
+  void UpdateRingWidget(const gfx::Point& mouse_location);
   void UpdateRingSize();
   void InitializeScrollLocation();
-  void UpdateScrollPosition();
+  void UpdateScrollPosition(const gfx::Point& point_in_screen);
   void HideScrollPosition();
   void RecordUserAction(AutoclickEventType event_type) const;
   bool DragInProgress() const;
@@ -199,6 +200,8 @@ class ASH_EXPORT AutoclickController
 
   // The widget containing the autoclick ring.
   std::unique_ptr<views::Widget> ring_widget_;
+  // The widget containing the autoclick scroll position indiciator.
+  std::unique_ptr<views::Widget> scroll_position_widget_;
   base::TimeDelta delay_;
   // The timer that counts down from the beginning of a gesture until a click.
   std::unique_ptr<base::RetainingOneShotTimer> autoclick_timer_;

ash/autoclick/autoclick_scroll_position_handler.cc

@@ -6,82 +6,126 @@
 
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "third_party/skia/include/core/SkColor.h"
-#include "ui/gfx/animation/tween.h"
+#include "third_party/skia/include/core/SkPath.h"
+#include "third_party/skia/include/core/SkRect.h"
+#include "ui/aura/window.h"
+#include "ui/compositor/layer.h"
+#include "ui/gfx/animation/linear_animation.h"
 #include "ui/gfx/canvas.h"
-#include "ui/gfx/color_palette.h"
-#include "ui/gfx/geometry/point.h"
-#include "ui/gfx/geometry/rect.h"
-#include "ui/gfx/geometry/size.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/paint_vector_icon.h"
-#include "ui/views/controls/image_view.h"
-#include "ui/views/layout/fill_layout.h"
+#include "ui/gfx/transform.h"
 #include "ui/views/view.h"
-#include "ui/views/widget/widget.h"
 
 namespace ash {
 
+namespace {
+constexpr int kScrollBackgroundSizeInDips = 32;
+constexpr int kScrollIconSizeInDips = 24;
+constexpr SkColor kIconBackgroundColor = SkColorSetARGB(255, 128, 134, 139);
+constexpr base::TimeDelta kAnimationTime =
+    base::TimeDelta::FromMilliseconds(500);
+constexpr float kFadedOpacity = 0.5;
+}  // namespace
+
+// View of the AutoclickScrollPositionHandler. Draws the actual contents and
+// maintains the views::Widget that the animation is shown in.
 class AutoclickScrollPositionView : public views::View {
  public:
-  AutoclickScrollPositionView();
-  AutoclickScrollPositionView(const AutoclickScrollPositionView&) = delete;
-  AutoclickScrollPositionView& operator=(const AutoclickScrollPositionView&) =
-      delete;
+  AutoclickScrollPositionView(const gfx::Point& event_location,
+                              views::Widget* widget)
+      : views::View(), widget_(widget) {
+    image_ = gfx::CreateVectorIcon(kAutoclickScrollIcon, kScrollIconSizeInDips,
+                                   SK_ColorWHITE);
+    widget_->SetContentsView(this);
+    SetSize(
+        gfx::Size(kScrollBackgroundSizeInDips, kScrollBackgroundSizeInDips));
+
+    // Owned by the AutoclickScrollPositionHandler.
+    set_owned_by_client();
+
+    SetLocation(event_location);
+  }
+
   ~AutoclickScrollPositionView() override = default;
 
- protected:
-  // views::View:
-  void OnPaintBackground(gfx::Canvas* canvas) override;
-};
+  void SetLocation(const gfx::Point& new_event_location) {
+    gfx::Point point = new_event_location;
+    widget_->SetBounds(gfx::Rect(point.x() - kScrollBackgroundSizeInDips / 2,
+                                 point.y() - kScrollBackgroundSizeInDips / 2,
+                                 kScrollBackgroundSizeInDips,
+                                 kScrollBackgroundSizeInDips));
+    widget_->Show();
+    widget_->SetOpacity(1.0);
+    SchedulePaint();
+  }
 
-AutoclickScrollPositionView::AutoclickScrollPositionView() {
-  SetLayoutManager(std::make_unique<views::FillLayout>());
-  AddChildView(std::make_unique<views::ImageView>())
-      ->SetImage(
-          gfx::CreateVectorIcon(kAutoclickScrollIcon, 24, SK_ColorWHITE));
-}
+  void UpdateForAnimationStep(gfx::Animation* animation) {
+    widget_->SetOpacity(animation->CurrentValueBetween(1.0, kFadedOpacity));
+  }
+
+ private:
+  void OnPaint(gfx::Canvas* canvas) override {
+    gfx::Point center(kScrollBackgroundSizeInDips / 2,
+                      kScrollBackgroundSizeInDips / 2);
+    canvas->Save();
 
-void AutoclickScrollPositionView::OnPaintBackground(gfx::Canvas* canvas) {
     cc::PaintFlags flags;
     flags.setAntiAlias(true);
-  flags.setColor(gfx::kGoogleGrey600);
-  canvas->DrawCircle(GetLocalBounds().CenterPoint(), width() / 2, flags);
-}
 
-// static
-constexpr base::TimeDelta AutoclickScrollPositionHandler::kOpaqueTime;
-constexpr base::TimeDelta AutoclickScrollPositionHandler::kFadeTime;
+    // Draw the grey background.
+    flags.setStyle(cc::PaintFlags::kFill_Style);
+    flags.setColor(kIconBackgroundColor);
+    canvas->DrawCircle(center, kScrollBackgroundSizeInDips / 2, flags);
+
+    // Draw the icon on top.
+    canvas->DrawImageInt(image_, center.x() - kScrollIconSizeInDips / 2,
+                         center.y() - kScrollIconSizeInDips / 2);
+
+    canvas->Restore();
+  }
+
+  views::Widget* widget_;
+  gfx::ImageSkia image_;
+
+  DISALLOW_COPY_AND_ASSIGN(AutoclickScrollPositionView);
+};
 
 AutoclickScrollPositionHandler::AutoclickScrollPositionHandler(
-    std::unique_ptr<views::Widget> widget)
-    : widget_(std::move(widget)) {
-  widget_->SetContentsView(std::make_unique<AutoclickScrollPositionView>());
+    const gfx::Point& center_point_in_screen,
+    views::Widget* widget)
+    : gfx::LinearAnimation(nullptr) {
+  view_ = std::make_unique<AutoclickScrollPositionView>(center_point_in_screen,
+                                                        widget);
+  SetDuration(kAnimationTime);
+  animation_state_ = AnimationState::kWait;
+  Start();
 }
 
-AutoclickScrollPositionHandler::~AutoclickScrollPositionHandler() = default;
-
-gfx::NativeView AutoclickScrollPositionHandler::GetNativeView() {
-  return widget_->GetNativeView();
+AutoclickScrollPositionHandler::~AutoclickScrollPositionHandler() {
+  view_.reset();
 }
 
-void AutoclickScrollPositionHandler::SetScrollPointCenterInScreen(
-    const gfx::Point& scroll_point_center) {
-  constexpr int kBackgroundSizeDips = 32;
-  gfx::Rect bounds(gfx::Size(kBackgroundSizeDips, kBackgroundSizeDips));
-  bounds.set_origin(scroll_point_center -
-                    bounds.CenterPoint().OffsetFromOrigin());
-  widget_->SetBounds(bounds);
-  widget_->Show();
-  widget_->SetOpacity(1.0f);
+void AutoclickScrollPositionHandler::SetCenter(
+    const gfx::Point& center_point_in_screen,
+    views::Widget* widget) {
+  view_->SetLocation(center_point_in_screen);
+  animation_state_ = AnimationState::kWait;
+  Start();
+}
 
-  timer_.Reset();
+void AutoclickScrollPositionHandler::AnimateToState(double state) {
+  if (animation_state_ == AnimationState::kFade)
+    view_->UpdateForAnimationStep(this);
 }
 
-void AutoclickScrollPositionHandler::AnimationProgressed(
-    const gfx::Animation* animation) {
-  constexpr float kSteadyStateOpacity = 0.5f;
-  widget_->SetOpacity(gfx::Tween::FloatValueBetween(
-      animation_.GetCurrentValue(), 1.0f, kSteadyStateOpacity));
+void AutoclickScrollPositionHandler::AnimationStopped() {
+  if (animation_state_ == AnimationState::kWait) {
+    animation_state_ = AnimationState::kFade;
+    Start();
+  } else if (animation_state_ == AnimationState::kFade) {
+    animation_state_ = AnimationState::kDone;
+  }
 }
 
 }  // namespace ash

ash/autoclick/autoclick_scroll_position_handler.h

@@ -7,54 +7,43 @@
 
 #include <memory>
 
-#include "base/location.h"
-#include "base/time/time.h"
-#include "base/timer/timer.h"
-#include "ui/gfx/animation/animation_delegate.h"
+#include "base/macros.h"
 #include "ui/gfx/animation/linear_animation.h"
-#include "ui/gfx/native_widget_types.h"
-
-namespace gfx {
-class Point;
-}  // namespace gfx
-
-namespace views {
-class Widget;
-}  // namespace views
+#include "ui/gfx/geometry/point.h"
+#include "ui/views/widget/widget.h"
 
 namespace ash {
 
+class AutoclickScrollPositionView;
+
 // AutoclickScrollPositionHandler displays the position at which the next scroll
 // event will occur, giving users a sense of which part of the screen will
 // receive scroll events. It will display at full opacity for a short time, then
 // partially fade out to keep from blocking content.
-class AutoclickScrollPositionHandler : public gfx::AnimationDelegate {
+class AutoclickScrollPositionHandler : public gfx::LinearAnimation {
  public:
-  explicit AutoclickScrollPositionHandler(
-      std::unique_ptr<views::Widget> widget);
-  AutoclickScrollPositionHandler(const AutoclickScrollPositionHandler&) =
-      delete;
-  AutoclickScrollPositionHandler& operator=(
-      const AutoclickScrollPositionHandler&) = delete;
+  AutoclickScrollPositionHandler(const gfx::Point& center_point_in_screen,
+                                 views::Widget* widget);
   ~AutoclickScrollPositionHandler() override;
 
-  gfx::NativeView GetNativeView();
-
-  void SetScrollPointCenterInScreen(const gfx::Point& scroll_point_center);
+  void SetCenter(const gfx::Point& center_point_in_screen,
+                 views::Widget* widget);
 
  private:
-  static constexpr auto kOpaqueTime = base::TimeDelta::FromMilliseconds(500);
-  static constexpr auto kFadeTime = base::TimeDelta::FromMilliseconds(500);
-
-  // gfx::AnimationDelegate:
-  void AnimationProgressed(const gfx::Animation* animation) override;
-
-  std::unique_ptr<views::Widget> widget_;
-  base::DelayTimer timer_{FROM_HERE, kOpaqueTime,
-                          static_cast<gfx::Animation*>(&animation_),
-                          &gfx::Animation::Start};
-  gfx::LinearAnimation animation_{
-      kFadeTime, gfx::LinearAnimation::kDefaultFrameRate, this};
+  enum AnimationState {
+    kWait,
+    kFade,
+    kDone,
+  };
+
+  // Overridden from gfx::LinearAnimation.
+  void AnimateToState(double state) override;
+  void AnimationStopped() override;
+
+  std::unique_ptr<AutoclickScrollPositionView> view_;
+  AnimationState animation_state_ = kDone;
+
+  DISALLOW_COPY_AND_ASSIGN(AutoclickScrollPositionHandler);
 };
 
 }  // namespace ash