Move animating layout from extensions container to ToolbarIconContainerView

Fix all related tests.

Bug: 1008444
Change-Id: Idff548f6f9547960cb81c931e72015578bd8cf29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1826991
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703455}

chrome/browser/ui/views/autofill/payments/local_card_migration_browsertest.cc

@@ -82,6 +82,7 @@
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/label_button.h"
+#include "ui/views/layout/animating_layout_manager.h"
 #include "ui/views/test/widget_test.h"
 #include "ui/views/window/dialog_client_view.h"
 
@@ -139,6 +140,36 @@ class PersonalDataLoadedObserverMock : public PersonalDataManagerObserver {
   MOCK_METHOD0(OnPersonalDataChanged, void());
 };
 
+// Helper to wait until the hover card widget is visible.
+class AnimatingLayoutWaiter : public views::AnimatingLayoutManager::Observer {
+ public:
+  explicit AnimatingLayoutWaiter(
+      views::AnimatingLayoutManager* animating_layout)
+      : animating_layout_(animating_layout) {
+    observer_.Add(animating_layout_);
+  }
+
+  void Wait() {
+    if (!animating_layout_->is_animating())
+      return;
+    run_loop_.Run();
+  }
+
+  // views::AnimatingLayoutManager overrides:
+  void OnLayoutIsAnimatingChanged(views::AnimatingLayoutManager* source,
+                                  bool is_animating) override {
+    if (!is_animating)
+      run_loop_.Quit();
+  }
+
+ private:
+  views::AnimatingLayoutManager* const animating_layout_;
+  ScopedObserver<views::AnimatingLayoutManager,
+                 views::AnimatingLayoutManager::Observer>
+      observer_{this};
+  base::RunLoop run_loop_;
+};
+
 class LocalCardMigrationBrowserTest
     : public SyncTest,
       public LocalCardMigrationManager::ObserverForTest {
@@ -323,6 +354,15 @@ class LocalCardMigrationBrowserTest
          DialogEvent::RECEIVED_GET_UPLOAD_DETAILS_RESPONSE});
     FillAndSubmitFormWithCard(card_number);
     WaitForObservedEvent();
+    if (base::FeatureList::IsEnabled(
+            features::kAutofillEnableToolbarStatusChip)) {
+      AnimatingLayoutWaiter waiter(static_cast<views::AnimatingLayoutManager*>(
+          BrowserView::GetBrowserViewForBrowser(browser())
+              ->toolbar()
+              ->toolbar_account_icon_container()
+              ->GetLayoutManager()));
+      waiter.Wait();
+    }
   }
 
   void ClickOnSaveButtonAndWaitForMigrationResults() {

chrome/browser/ui/views/autofill/payments/save_card_bubble_views_browsertest.cc

@@ -77,6 +77,7 @@
 #include "ui/views/controls/combobox/combobox.h"
 #include "ui/views/controls/styled_label.h"
 #include "ui/views/controls/textfield/textfield.h"
+#include "ui/views/layout/animating_layout_manager.h"
 #include "ui/views/test/widget_test.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_client_view.h"
@@ -123,6 +124,36 @@ const double kFakeGeolocationLatitude = 1.23;
 const double kFakeGeolocationLongitude = 4.56;
 }  // namespace
 
+// Helper to wait until the hover card widget is visible.
+class AnimatingLayoutWaiter : public views::AnimatingLayoutManager::Observer {
+ public:
+  explicit AnimatingLayoutWaiter(
+      views::AnimatingLayoutManager* animating_layout)
+      : animating_layout_(animating_layout) {
+    observer_.Add(animating_layout_);
+  }
+
+  void Wait() {
+    if (!animating_layout_->is_animating())
+      return;
+    run_loop_.Run();
+  }
+
+  // views::AnimatingLayoutManager overrides:
+  void OnLayoutIsAnimatingChanged(views::AnimatingLayoutManager* source,
+                                  bool is_animating) override {
+    if (!is_animating)
+      run_loop_.Quit();
+  }
+
+ private:
+  views::AnimatingLayoutManager* const animating_layout_;
+  ScopedObserver<views::AnimatingLayoutManager,
+                 views::AnimatingLayoutManager::Observer>
+      observer_{this};
+  base::RunLoop run_loop_;
+};
+
 namespace autofill {
 
 class SaveCardBubbleViewsFullFormBrowserTest
@@ -349,6 +380,7 @@ class SaveCardBubbleViewsFullFormBrowserTest
     ResetEventWaiterForSequence({DialogEvent::OFFERED_LOCAL_SAVE});
     SubmitForm();
     WaitForObservedEvent();
+    WaitForAnimationToEnd();
     EXPECT_TRUE(FindViewInBubbleById(DialogViewId::MAIN_CONTENT_VIEW_LOCAL)
                     ->GetVisible());
   }
@@ -361,6 +393,7 @@ class SaveCardBubbleViewsFullFormBrowserTest
          DialogEvent::RECEIVED_GET_UPLOAD_DETAILS_RESPONSE});
     SubmitForm();
     WaitForObservedEvent();
+    WaitForAnimationToEnd();
     EXPECT_TRUE(FindViewInBubbleById(DialogViewId::MAIN_CONTENT_VIEW_UPLOAD)
                     ->GetVisible());
     EXPECT_TRUE(
@@ -777,6 +810,18 @@ class SaveCardBubbleViewsFullFormBrowserTest
 
   void WaitForObservedEvent() { event_waiter_->Wait(); }
 
+  void WaitForAnimationToEnd() {
+    if (base::FeatureList::IsEnabled(
+            features::kAutofillEnableToolbarStatusChip)) {
+      AnimatingLayoutWaiter waiter(static_cast<views::AnimatingLayoutManager*>(
+          BrowserView::GetBrowserViewForBrowser(browser())
+              ->toolbar()
+              ->toolbar_account_icon_container()
+              ->GetLayoutManager()));
+      waiter.Wait();
+    }
+  }
+
   network::TestURLLoaderFactory* test_url_loader_factory() {
     return &test_url_loader_factory_;
   }
@@ -2681,7 +2726,6 @@ IN_PROC_BROWSER_TEST_F(SaveCardBubbleViewsFullFormBrowserTestForStatusChip,
                        ClickingOnCreditCardIconInStatusChipReshowsBubble) {
   FillForm();
   SubmitFormAndWaitForCardLocalSaveBubble();
-
   ClickOnCloseButton();
   AddEventObserverToController();
   ResetEventWaiterForSequence({DialogEvent::BUBBLE_SHOWN});

chrome/browser/ui/views/extensions/extensions_menu_view_browsertest.cc

@@ -36,9 +36,8 @@ class AnimatingLayoutWaiter : public views::AnimatingLayoutManager::Observer {
   explicit AnimatingLayoutWaiter(
       views::AnimatingLayoutManager* animating_layout)
       : animating_layout_(animating_layout) {
-    animating_layout_->AddObserver(this);
+    observer_.Add(animating_layout_);
   }
-  ~AnimatingLayoutWaiter() override { animating_layout_->RemoveObserver(this); }
 
   void Wait() {
     if (!animating_layout_->is_animating())
@@ -55,6 +54,9 @@ class AnimatingLayoutWaiter : public views::AnimatingLayoutManager::Observer {
 
  private:
   views::AnimatingLayoutManager* const animating_layout_;
+  ScopedObserver<views::AnimatingLayoutManager,
+                 views::AnimatingLayoutManager::Observer>
+      observer_{this};
   base::RunLoop run_loop_;
 };
 
@@ -120,11 +122,11 @@ class ExtensionsMenuViewBrowserTest : public DialogBrowserTest {
         ->primary_action_button_for_testing()
         ->button_controller()
         ->OnMouseReleased(click_event);
-    AnimatingLayoutWaiter waiter(
+    AnimatingLayoutWaiter waiter(static_cast<views::AnimatingLayoutManager*>(
         BrowserView::GetBrowserViewForBrowser(browser())
             ->toolbar()
             ->extensions_container()
-            ->animating_layout_for_testing());
+            ->GetLayoutManager()));
     waiter.Wait();
   }
 

chrome/browser/ui/views/extensions/extensions_toolbar_container.cc

@@ -13,8 +13,6 @@
 #include "chrome/browser/ui/views/extensions/extensions_toolbar_button.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.h"
 #include "ui/views/layout/animating_layout_manager.h"
-#include "ui/views/layout/flex_layout.h"
-#include "ui/views/view_class_properties.h"
 #include "ui/views/widget/widget_observer.h"
 
 struct ExtensionsToolbarContainer::DropInfo {
@@ -38,15 +36,6 @@ ExtensionsToolbarContainer::ExtensionsToolbarContainer(Browser* browser)
       model_(ToolbarActionsModel::Get(browser_->profile())),
       model_observer_(this),
       extensions_button_(new ExtensionsToolbarButton(browser_, this)) {
-  animating_layout_ =
-      SetLayoutManager(std::make_unique<views::AnimatingLayoutManager>());
-  animating_layout_->SetShouldAnimateBounds(true);
-  auto* flex_layout = animating_layout_->SetTargetLayoutManager(
-      std::make_unique<views::FlexLayout>());
-  flex_layout->SetCollapseMargins(true)
-      .SetIgnoreDefaultMainAxisMargins(true)
-      .SetDefault(views::kMarginsKey,
-                  gfx::Insets(0, GetLayoutConstant(TOOLBAR_ELEMENT_PADDING)));
   model_observer_.Add(model_);
   // Do not flip the Extensions icon in RTL.
   extensions_button_->EnableCanvasFlippingForRTLUI(false);
@@ -140,7 +129,8 @@ void ExtensionsToolbarContainer::PopOutAction(
   popped_out_action_ = action;
   icons_[popped_out_action_->GetId()]->SetVisible(true);
   ReorderViews();
-  animating_layout_->RunOrQueueAction(closure);
+  static_cast<views::AnimatingLayoutManager*>(GetLayoutManager())
+      ->RunOrQueueAction(closure);
 }
 
 void ExtensionsToolbarContainer::ShowToolbarActionBubble(
@@ -153,10 +143,11 @@ void ExtensionsToolbarContainer::ShowToolbarActionBubble(
 
   anchor_view->SetVisible(true);
 
-  animating_layout_->RunOrQueueAction(
-      base::BindOnce(&ExtensionsToolbarContainer::ShowActiveBubble,
-                     weak_ptr_factory_.GetWeakPtr(), anchor_view,
-                     base::Passed(std::move(controller))));
+  static_cast<views::AnimatingLayoutManager*>(GetLayoutManager())
+      ->RunOrQueueAction(
+          base::BindOnce(&ExtensionsToolbarContainer::ShowActiveBubble,
+                         weak_ptr_factory_.GetWeakPtr(), anchor_view,
+                         base::Passed(std::move(controller))));
 }
 
 void ExtensionsToolbarContainer::ShowToolbarActionBubbleAsync(

chrome/browser/ui/views/extensions/extensions_toolbar_container.h

@@ -20,10 +20,6 @@ class ExtensionsToolbarButton;
 class ToolbarActionViewController;
 class ToolbarActionsBarBubbleViews;
 
-namespace views {
-class AnimatingLayoutManager;
-}  // namespace views
-
 // Container for extensions shown in the toolbar. These include pinned
 // extensions and extensions that are 'popped out' transitively to show dialogs
 // or be called out to the user.
@@ -49,10 +45,6 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView,
     return active_bubble_;
   }
 
-  views::AnimatingLayoutManager* animating_layout_for_testing() {
-    return animating_layout_;
-  }
-
   // ToolbarIconContainerView:
   void UpdateAllIcons() override;
 
@@ -170,8 +162,6 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView,
   // there is none.
   std::unique_ptr<DropInfo> drop_info_;
 
-  views::AnimatingLayoutManager* animating_layout_ = nullptr;
-
   base::WeakPtrFactory<ExtensionsToolbarContainer> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionsToolbarContainer);

chrome/browser/ui/views/toolbar/toolbar_icon_container_view.cc

@@ -11,17 +11,21 @@
 #include "chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/background.h"
+#include "ui/views/layout/animating_layout_manager.h"
 #include "ui/views/layout/flex_layout.h"
 #include "ui/views/view_class_properties.h"
 
 ToolbarIconContainerView::ToolbarIconContainerView(bool uses_highlight)
     : uses_highlight_(uses_highlight) {
-  auto layout_manager = std::make_unique<views::FlexLayout>();
-  layout_manager->SetCollapseMargins(true)
+  views::AnimatingLayoutManager* animating_layout =
+      SetLayoutManager(std::make_unique<views::AnimatingLayoutManager>());
+  animating_layout->SetShouldAnimateBounds(true);
+  auto* flex_layout = animating_layout->SetTargetLayoutManager(
+      std::make_unique<views::FlexLayout>());
+  flex_layout->SetCollapseMargins(true)
       .SetIgnoreDefaultMainAxisMargins(true)
       .SetDefault(views::kMarginsKey,
                   gfx::Insets(0, GetLayoutConstant(TOOLBAR_ELEMENT_PADDING)));
-  SetLayoutManager(std::move(layout_manager));
 }
 
 ToolbarIconContainerView::~ToolbarIconContainerView() = default;
@@ -39,10 +43,8 @@ void ToolbarIconContainerView::AddMainButton(views::Button* main_button) {
 void ToolbarIconContainerView::OnHighlightChanged(
     views::Button* observed_button,
     bool highlighted) {
-  if (highlighted) {
+  if (highlighted)
     DCHECK(observed_button);
-    DCHECK(observed_button->GetVisible());
-  }
 
   // TODO(crbug.com/932818): Pass observed button type to container.
   highlighted_button_ = highlighted ? observed_button : nullptr;