cbui: remove ToolbarActionsBarBubbleViews CreateExtraView

Instead, use SetExtraView. The create method is renamed to avoid conflict with
the deprecated parent method it used to override, but left as a method on the
clsas because it sets up some of the class's internal state.

The tests for this class were calling CreateExtraView manually, which is
not allowed by the contract of DialogDelegate::CreateExtraView, but
happened to work on this class. This change therefore introduces an
accessor DialogDelegate::GetExtraView() that pulls the extra view out of
the DialogClientView and uses that in these tests to access the existing
extra view.

Bug: 1011446
Change-Id: I8ad2f3d20d94b461d78b02f3b2a671b2a15a6a31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1876789
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709503}

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

@@ -38,6 +38,7 @@ ToolbarActionsBarBubbleViews::ToolbarActionsBarBubbleViews(
                                    delegate_->GetActionButtonText());
   DialogDelegate::set_button_label(ui::DIALOG_BUTTON_CANCEL,
                                    delegate_->GetDismissButtonText());
+  DialogDelegate::SetExtraView(CreateExtraInfoView());
 
   DCHECK(anchor_view);
   set_close_on_deactivate(delegate_->ShouldCloseOnDeactivate());
@@ -62,7 +63,8 @@ std::string ToolbarActionsBarBubbleViews::GetAnchorActionId() {
   return delegate_->GetAnchorActionId();
 }
 
-std::unique_ptr<views::View> ToolbarActionsBarBubbleViews::CreateExtraView() {
+std::unique_ptr<views::View>
+ToolbarActionsBarBubbleViews::CreateExtraInfoView() {
   std::unique_ptr<ToolbarActionsBarBubbleDelegate::ExtraViewInfo>
       extra_view_info = delegate_->GetExtraViewInfo();
 
@@ -86,8 +88,8 @@ std::unique_ptr<views::View> ToolbarActionsBarBubbleViews::CreateExtraView() {
       image_button->SetTooltipText(text);
       views::SetImageFromVectorIcon(image_button.get(),
                                     vector_icons::kHelpOutlineIcon);
-      image_button_ = image_button.release();
-      extra_view.reset(image_button_);
+      learn_more_button_ = image_button.get();
+      extra_view = std::move(image_button);
     } else {
       extra_view = std::make_unique<views::Label>(text);
     }

chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.h

@@ -34,15 +34,16 @@ class ToolbarActionsBarBubbleViews : public views::BubbleDialogDelegateView,
 
   const views::Label* body_text() const { return body_text_; }
   const views::Label* item_list() const { return item_list_; }
-  views::ImageButton* learn_more_button() const { return image_button_; }
+  views::ImageButton* learn_more_button() const { return learn_more_button_; }
 
  private:
   friend class ToolbarActionsBarBubbleViewsTest;
 
+  std::unique_ptr<views::View> CreateExtraInfoView();
+
   // views::BubbleDialogDelegateView:
   base::string16 GetWindowTitle() const override;
   bool ShouldShowCloseButton() const override;
-  std::unique_ptr<views::View> CreateExtraView() override;
   bool Cancel() override;
   bool Accept() override;
   bool Close() override;
@@ -56,7 +57,7 @@ class ToolbarActionsBarBubbleViews : public views::BubbleDialogDelegateView,
   bool delegate_notified_of_close_ = false;
   views::Label* body_text_ = nullptr;
   views::Label* item_list_ = nullptr;
-  views::ImageButton* image_button_ = nullptr;
+  views::ImageButton* learn_more_button_ = nullptr;
   const bool anchored_to_action_;
 
   DISALLOW_COPY_AND_ASSIGN(ToolbarActionsBarBubbleViews);

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

@@ -32,12 +32,6 @@ const int kIconSize = 16;
 }
 
 class ToolbarActionsBarBubbleViewsTest : public ChromeViewsTestBase {
- public:
-  std::unique_ptr<views::View> TestCreateExtraView() {
-    DCHECK(bubble_);
-    return bubble_->CreateExtraView();
-  }
-
  protected:
   ToolbarActionsBarBubbleViewsTest() {}
   ~ToolbarActionsBarBubbleViewsTest() override {}
@@ -136,7 +130,7 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestBubbleLayoutNoButtons) {
   delegate.set_action_button_text(base::string16());
   ShowBubble(&delegate);
 
-  std::unique_ptr<views::View> extra_view(TestCreateExtraView());
+  EXPECT_EQ(nullptr, bubble()->GetExtraView());
   EXPECT_FALSE(bubble()->GetDialogClientView()->ok_button());
   EXPECT_FALSE(bubble()->GetDialogClientView()->cancel_button());
   EXPECT_FALSE(bubble()->learn_more_button());
@@ -352,8 +346,7 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestNullExtraView) {
   TestToolbarActionsBarBubbleDelegate delegate(HeadingString(), BodyString(),
                                                ActionString());
   ShowBubble(&delegate);
-  std::unique_ptr<views::View> extra_view(TestCreateExtraView());
-  ASSERT_FALSE(extra_view);
+  EXPECT_EQ(nullptr, bubble()->GetExtraView());
   CloseBubble();
 }
 
@@ -366,11 +359,11 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestCreateExtraViewIconOnly) {
   extra_view_info->resource = &vector_icons::kBusinessIcon;
   delegate.set_extra_view_info(std::move(extra_view_info));
   ShowBubble(&delegate);
-  std::unique_ptr<views::View> extra_view(TestCreateExtraView());
+  const views::View* const extra_view = bubble()->GetExtraView();
   ASSERT_TRUE(extra_view);
   ASSERT_EQ("ImageView", std::string(extra_view->GetClassName()));
   EXPECT_TRUE(gfx::test::AreImagesEqual(
-      gfx::Image(static_cast<views::ImageView*>(extra_view.get())->GetImage()),
+      gfx::Image(static_cast<const views::ImageView*>(extra_view)->GetImage()),
       gfx::Image(gfx::CreateVectorIcon(vector_icons::kBusinessIcon, kIconSize,
                                        gfx::kChromeIconGrey))));
   CloseBubble();
@@ -389,7 +382,7 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestCreateExtraViewLinkedTextOnly) {
 
   ShowBubble(&delegate);
 
-  std::unique_ptr<views::View> extra_view(TestCreateExtraView());
+  const views::View* const extra_view = bubble()->GetExtraView();
   ASSERT_TRUE(extra_view);
   ASSERT_EQ("ImageButton", std::string(extra_view->GetClassName()));
   EXPECT_EQ(l10n_util::GetStringUTF16(IDS_EXTENSIONS_INSTALLED_BY_ADMIN),
@@ -410,11 +403,11 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestCreateExtraViewLabelTextOnly) {
 
   ShowBubble(&delegate);
 
-  std::unique_ptr<views::View> extra_view(TestCreateExtraView());
+  const views::View* const extra_view = bubble()->GetExtraView();
   ASSERT_TRUE(extra_view);
   EXPECT_EQ("Label", std::string(extra_view->GetClassName()));
   EXPECT_EQ(l10n_util::GetStringUTF16(IDS_EXTENSIONS_INSTALLED_BY_ADMIN),
-            static_cast<views::Label*>(extra_view.get())->GetText());
+            static_cast<const views::Label*>(extra_view)->GetText());
   CloseBubble();
 }
 
@@ -432,7 +425,7 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestCreateExtraViewImageAndText) {
 
   ShowBubble(&delegate);
 
-  std::unique_ptr<views::View> extra_view(TestCreateExtraView());
+  const views::View* const extra_view = bubble()->GetExtraView();
   ASSERT_TRUE(extra_view);
   EXPECT_STREQ("View", extra_view->GetClassName());
   EXPECT_EQ(2u, extra_view->children().size());

ui/views/window/dialog_client_view.h

@@ -46,6 +46,7 @@ class VIEWS_EXPORT DialogClientView : public ClientView,
   // Accessors in case the user wishes to adjust these buttons.
   LabelButton* ok_button() const { return ok_button_; }
   LabelButton* cancel_button() const { return cancel_button_; }
+  View* extra_view() const { return extra_view_; }
 
   void SetButtonRowInsets(const gfx::Insets& insets);
 

ui/views/window/dialog_delegate.cc

@@ -259,6 +259,12 @@ views::LabelButton* DialogDelegate::GetCancelButton() {
   return client ? client->cancel_button() : nullptr;
 }
 
+views::View* DialogDelegate::GetExtraView() {
+  DCHECK(GetWidget()) << "Don't call this before OnDialogInitialized";
+  auto* client = GetDialogClientView();
+  return client ? client->extra_view() : nullptr;
+}
+
 void DialogDelegate::AddObserver(DialogObserver* observer) {
   observer_list_.AddObserver(observer);
 }

ui/views/window/dialog_delegate.h

@@ -164,6 +164,7 @@ class VIEWS_EXPORT DialogDelegate : public WidgetDelegate {
   // about DialogClientView. Do not call these before OnDialogInitialized.
   views::LabelButton* GetOkButton();
   views::LabelButton* GetCancelButton();
+  views::View* GetExtraView();
 
   // Add or remove an observer notified by calls to DialogModelChanged().
   void AddObserver(DialogObserver* observer);