Fix autoclick scroll bubble's jumpy animation to position.

The autoclick scroll bubble animation was jumpy because the arrow
and anchor rect changes both kicked off animations. This gives
subclasses of BubbleDialogDelegateView the option to set the arrow
without creating an animation, in order to fix the visual bug.

Bug: 978163
Change-Id: Id4feb50e35a3945c24e5759d953261a02dd4970c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1691549
Commit-Queue: Katie Dektar <katie@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676211}

ash/system/accessibility/autoclick_scroll_bubble_controller.cc

@@ -52,8 +52,7 @@ void AutoclickScrollBubbleController::UpdateAnchorRect(
   menu_bubble_alignment_ = alignment;
   if (set_scroll_rect_)
     return;
-  bubble_view_->SetArrow(alignment);
-  bubble_view_->UpdateAnchorRect(rect);
+  bubble_view_->UpdateAnchorRect(rect, alignment);
 }
 
 void AutoclickScrollBubbleController::SetScrollPosition(
@@ -78,8 +77,8 @@ void AutoclickScrollBubbleController::SetScrollPosition(
     // Buffer around the point so that the scroll bubble does not overlap it.
     // ScrollBubbleController will automatically layout to avoid edges.
     anchor.Inset(-kScrollPointBufferDips, -kScrollPointBufferDips);
-    bubble_view_->SetArrow(views::BubbleBorder::Arrow::LEFT_CENTER);
-    bubble_view_->UpdateAnchorRect(anchor);
+    bubble_view_->UpdateAnchorRect(anchor,
+                                   views::BubbleBorder::Arrow::LEFT_CENTER);
     return;
   }
 
@@ -152,17 +151,18 @@ void AutoclickScrollBubbleController::SetScrollPosition(
   std::stable_sort(positions.begin(), positions.end(), comparePositions);
 
   // Position on the optimal side depending on the shortest distance.
-  bubble_view_->SetArrow(positions.front().arrow);
   if (positions.front().is_horizontal) {
     // Center it vertically on the scroll point.
-    bubble_view_->UpdateAnchorRect(gfx::Rect(scroll_bounds_in_dips.x(),
-                                             scroll_point_in_dips.y(),
-                                             scroll_bounds_in_dips.width(), 0));
+    bubble_view_->UpdateAnchorRect(
+        gfx::Rect(scroll_bounds_in_dips.x(), scroll_point_in_dips.y(),
+                  scroll_bounds_in_dips.width(), 0),
+        positions.at(0).arrow);
   } else {
     // Center horizontally on scroll point.
-    bubble_view_->UpdateAnchorRect(gfx::Rect(scroll_point_in_dips.x(),
-                                             scroll_bounds_in_dips.y(), 0,
-                                             scroll_bounds_in_dips.height()));
+    bubble_view_->UpdateAnchorRect(
+        gfx::Rect(scroll_point_in_dips.x(), scroll_bounds_in_dips.y(), 0,
+                  scroll_bounds_in_dips.height()),
+        positions.at(0).arrow);
   }
 }
 

ash/system/accessibility/autoclick_scroll_view.cc

@@ -327,11 +327,16 @@ AutoclickScrollBubbleView::AutoclickScrollBubbleView(
 
 AutoclickScrollBubbleView::~AutoclickScrollBubbleView() {}
 
-void AutoclickScrollBubbleView::UpdateAnchorRect(const gfx::Rect& rect) {
+void AutoclickScrollBubbleView::UpdateAnchorRect(
+    const gfx::Rect& rect,
+    views::BubbleBorder::Arrow arrow) {
   ui::ScopedLayerAnimationSettings settings(
       GetWidget()->GetLayer()->GetAnimator());
   settings.SetPreemptionStrategy(
       ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
+  // SetAnchorRect will resize, so set the arrow without reizing to avoid a
+  // double animation.
+  SetArrowWithoutResizing(arrow);
   SetAnchorRect(rect);
 }
 

ash/system/accessibility/autoclick_scroll_view.h

@@ -21,7 +21,8 @@ class AutoclickScrollBubbleView : public TrayBubbleView {
   explicit AutoclickScrollBubbleView(TrayBubbleView::InitParams init_params);
   ~AutoclickScrollBubbleView() override;
 
-  void UpdateAnchorRect(const gfx::Rect& rect);
+  void UpdateAnchorRect(const gfx::Rect& rect,
+                        views::BubbleBorder::Arrow arrow);
 
   // TrayBubbleView:
   bool IsAnchoredToStatusArea() const override;

ui/views/bubble/bubble_dialog_delegate_view.cc

@@ -265,6 +265,15 @@ void BubbleDialogDelegateView::SetHighlightedButton(
 }
 
 void BubbleDialogDelegateView::SetArrow(BubbleBorder::Arrow arrow) {
+  SetArrowWithoutResizing(arrow);
+  // If SetArrow() is called before CreateWidget(), there's no need to update
+  // the BubbleFrameView.
+  if (GetBubbleFrameView())
+    SizeToContents();
+}
+
+void BubbleDialogDelegateView::SetArrowWithoutResizing(
+    BubbleBorder::Arrow arrow) {
   if (base::i18n::IsRTL())
     arrow = BubbleBorder::horizontal_mirror(arrow);
   if (arrow_ == arrow)
@@ -273,10 +282,8 @@ void BubbleDialogDelegateView::SetArrow(BubbleBorder::Arrow arrow) {
 
   // If SetArrow() is called before CreateWidget(), there's no need to update
   // the BubbleFrameView.
-  if (GetBubbleFrameView()) {
+  if (GetBubbleFrameView())
     GetBubbleFrameView()->SetArrow(arrow);
-    SizeToContents();
-  }
 }
 
 gfx::Rect BubbleDialogDelegateView::GetAnchorRect() const {

ui/views/bubble/bubble_dialog_delegate_view.h

@@ -79,9 +79,19 @@ class VIEWS_EXPORT BubbleDialogDelegateView : public DialogDelegateView,
   // The anchor rect is used in the absence of an assigned anchor view.
   const gfx::Rect& anchor_rect() const { return anchor_rect_; }
 
-  // Set the desired arrow for the bubble. The arrow will be mirrored for RTL.
+  // Set the desired arrow for the bubble and updates the bubble's bounds
+  // accordingly. The arrow will be mirrored for RTL.
   void SetArrow(BubbleBorder::Arrow arrow);
 
+  // Sets the arrow without recaluclating or updating bounds. This could be used
+  // proceeding another function call which also sets bounds, so that bounds are
+  // not set multiple times in a row. When animating bounds changes, setting
+  // bounds twice in a row can make the widget position jump.
+  // TODO(crbug.com/982880) It would be good to be able to re-target the
+  // animation rather than expet callers to use SetArrowWithoutResizing if they
+  // are also changing the anchor rect, or similar.
+  void SetArrowWithoutResizing(BubbleBorder::Arrow arrow);
+
   BubbleBorder::Shadow GetShadow() const;
   void set_shadow(BubbleBorder::Shadow shadow) { shadow_ = shadow; }