[Extensions Click-to-Script] Update refresh-required bubble

New mocks use a bubble with title only (as opposed to title and body),
different strings, and no cancel button. Update the bubble accordingly,
and allow ToolbarActionsBarBubble* to accept a null body.

Bug: 834494

Change-Id: Iea23483d894e001e23b14e4e7450120c21d6c87a
Reviewed-on: https://chromium-review.googlesource.com/1033320Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555470}

chrome/app/generated_resources.grd

@@ -3323,16 +3323,10 @@ are declared in build/common.gypi.
       <if expr="enable_extensions">
         <!-- Extension blocked action bubble -->
         <message name="IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_HEADING" desc="Heading of the bubble to tell users that in order to run an extension, they'll need to refresh the page.">
-          Refresh required
-        </message>
-        <message name="IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_CONTENT" desc="The content of the bubble to tell users that in order to run an extension, they'll need to refresh the page.">
-          In order to run this extension, you need to refresh the page. You can run this extension automatically on this site by right-clicking on the extension icon.
+          Reload page to use this extension
         </message>
         <message name="IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_OK_BUTTON" desc="The text of the button to proceed with the page refresh for running an extension.">
-          OK, refresh
-        </message>
-        <message name="IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_CANCEL_BUTTON" desc="The text of the button to close the bubble and not refresh the page or run the extension.">
-          Never mind
+          Reload
         </message>
       </if>
 

chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.h

@@ -27,6 +27,10 @@ class ToolbarActionsBarBubbleDelegate;
   // Required.
   NSButton* actionButton_;
 
+  // The text to display in the body of the bubble, excluding the item list.
+  // Optional.
+  NSTextField* bodyText_;
+
   // The list of items to display. Optional.
   NSTextField* itemList_;
 
@@ -57,6 +61,7 @@ class ToolbarActionsBarBubbleDelegate;
 + (void)setAnimationEnabledForTesting:(BOOL)enabled;
 
 @property(readonly, nonatomic) NSButton* actionButton;
+@property(readonly, nonatomic) NSTextField* bodyText;
 @property(readonly, nonatomic) NSTextField* itemList;
 @property(readonly, nonatomic) NSButton* dismissButton;
 @property(readonly, nonatomic) NSButton* link;

chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm

@@ -61,6 +61,7 @@ CGFloat kMinWidth = 320.0;
 @implementation ToolbarActionsBarBubbleMac
 
 @synthesize actionButton = actionButton_;
+@synthesize bodyText = bodyText_;
 @synthesize itemList = itemList_;
 @synthesize dismissButton = dismissButton_;
 @synthesize link = link_;
@@ -258,6 +259,9 @@ CGFloat kMinWidth = 320.0;
   }
 
   DCHECK(actionButton_ || dismissButton_);
+  // TODO(devlin): This doesn't currently take into account
+  // delegate_->GetDefaultDialogButton().
+
   CGFloat buttonStripHeight =
       std::max(actionButtonSize.height, dismissButtonSize.height);
 
@@ -276,16 +280,19 @@ CGFloat kMinWidth = 320.0;
   CGFloat windowWidth =
       std::max(std::max(kMinWidth, buttonStripWidth), headingWidth);
 
-  NSTextField* content =
-      [self addTextFieldWithString:delegate_->GetBodyText(anchoredToAction_)
-                          fontSize:12.0
-                         alignment:NSLeftTextAlignment];
-  [content setFrame:NSMakeRect(0, 0, windowWidth, 0)];
-  // The content should have the same (max) width as the heading, which means
-  // the text will most likely wrap.
-  NSSize contentSize = NSMakeSize(windowWidth,
-                                  [GTMUILocalizerAndLayoutTweaker
-                                       sizeToFitFixedWidthTextField:content]);
+  base::string16 bodyTextString = delegate_->GetBodyText(anchoredToAction_);
+  NSSize bodyTextSize;
+  if (!bodyTextString.empty()) {
+    bodyText_ = [self addTextFieldWithString:bodyTextString
+                                    fontSize:12.0
+                                   alignment:NSLeftTextAlignment];
+    [bodyText_ setFrame:NSMakeRect(0, 0, windowWidth, 0)];
+    // The content should have the same (max) width as the heading, which means
+    // the text will most likely wrap.
+    bodyTextSize =
+        NSMakeSize(windowWidth, [GTMUILocalizerAndLayoutTweaker
+                                    sizeToFitFixedWidthTextField:bodyText_]);
+  }
 
   const CGFloat kItemListIndentation = 10.0;
   base::string16 itemListStr = delegate_->GetItemListText();
@@ -360,11 +367,12 @@ CGFloat kMinWidth = 320.0;
     currentHeight += itemListSize.height + kVerticalPadding;
   }
 
-  [content setFrame:NSMakeRect(kHorizontalPadding,
-                               currentHeight,
-                               contentSize.width,
-                               contentSize.height)];
-  currentHeight += contentSize.height + kVerticalPadding;
+  if (bodyText_) {
+    [bodyText_ setFrame:NSMakeRect(kHorizontalPadding, currentHeight,
+                                   bodyTextSize.width, bodyTextSize.height)];
+    currentHeight += bodyTextSize.height + kVerticalPadding;
+  }
+
   [heading setFrame:NSMakeRect(kHorizontalPadding,
                                currentHeight,
                                headingSize.width,

chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac_unittest.mm

@@ -191,6 +191,7 @@ TEST_F(ToolbarActionsBarBubbleMacTest, ToolbarActionsBarBubbleLayout) {
     EXPECT_FALSE([bubble label]);
     EXPECT_FALSE([bubble link]);
     EXPECT_FALSE([bubble dismissButton]);
+    EXPECT_TRUE([bubble bodyText]);
     EXPECT_FALSE([bubble itemList]);
 
     [bubble close];
@@ -216,6 +217,7 @@ TEST_F(ToolbarActionsBarBubbleMacTest, ToolbarActionsBarBubbleLayout) {
     EXPECT_FALSE([bubble label]);
     EXPECT_TRUE([bubble link]);
     EXPECT_TRUE([bubble dismissButton]);
+    EXPECT_TRUE([bubble bodyText]);
     EXPECT_FALSE([bubble itemList]);
 
     [bubble close];
@@ -233,6 +235,7 @@ TEST_F(ToolbarActionsBarBubbleMacTest, ToolbarActionsBarBubbleLayout) {
     EXPECT_FALSE([bubble label]);
     EXPECT_FALSE([bubble link]);
     EXPECT_TRUE([bubble dismissButton]);
+    EXPECT_TRUE([bubble bodyText]);
     EXPECT_FALSE([bubble itemList]);
 
     [bubble close];
@@ -250,12 +253,30 @@ TEST_F(ToolbarActionsBarBubbleMacTest, ToolbarActionsBarBubbleLayout) {
     EXPECT_FALSE([bubble label]);
     EXPECT_FALSE([bubble link]);
     EXPECT_FALSE([bubble dismissButton]);
+    EXPECT_TRUE([bubble bodyText]);
     EXPECT_TRUE([bubble itemList]);
 
     [bubble close];
     chrome::testing::NSRunLoopRunAllPending();
   }
 
+  // Test with no body text.
+  {
+    TestToolbarActionsBarBubbleDelegate delegate(
+        HeadingString(), base::string16(), ActionString());
+    ToolbarActionsBarBubbleMac* bubble = CreateAndShowBubble(&delegate);
+    EXPECT_TRUE([bubble actionButton]);
+    EXPECT_FALSE([bubble iconView]);
+    EXPECT_FALSE([bubble label]);
+    EXPECT_FALSE([bubble link]);
+    EXPECT_FALSE([bubble dismissButton]);
+    EXPECT_FALSE([bubble bodyText]);
+    EXPECT_FALSE([bubble itemList]);
+
+    [bubble close];
+    chrome::testing::NSRunLoopRunAllPending();
+  }
+
   // Test with a null extra view.
   {
     TestToolbarActionsBarBubbleDelegate delegate(HeadingString(), BodyString(),
@@ -266,6 +287,7 @@ TEST_F(ToolbarActionsBarBubbleMacTest, ToolbarActionsBarBubbleLayout) {
     EXPECT_FALSE([bubble label]);
     EXPECT_FALSE([bubble link]);
     EXPECT_FALSE([bubble dismissButton]);
+    EXPECT_TRUE([bubble bodyText]);
     EXPECT_FALSE([bubble itemList]);
 
     [bubble close];
@@ -292,6 +314,7 @@ TEST_F(ToolbarActionsBarBubbleMacTest, ToolbarActionsBarBubbleLayout) {
     EXPECT_TRUE([bubble label]);
     EXPECT_FALSE([bubble link]);
     EXPECT_FALSE([bubble dismissButton]);
+    EXPECT_TRUE([bubble bodyText]);
     EXPECT_FALSE([bubble itemList]);
 
     [bubble close];
@@ -318,6 +341,7 @@ TEST_F(ToolbarActionsBarBubbleMacTest, ToolbarActionsBarBubbleLayout) {
     EXPECT_FALSE([bubble label]);
     EXPECT_TRUE([bubble link]);
     EXPECT_TRUE([bubble dismissButton]);
+    EXPECT_TRUE([bubble bodyText]);
     EXPECT_TRUE([bubble itemList]);
 
     [bubble close];

chrome/browser/ui/extensions/blocked_action_bubble_delegate.cc

@@ -8,6 +8,7 @@
 #include "base/strings/string16.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/ui_base_types.h"
 
 BlockedActionBubbleDelegate::BlockedActionBubbleDelegate(
     const base::Callback<void(CloseAction)>& callback,
@@ -32,7 +33,7 @@ base::string16 BlockedActionBubbleDelegate::GetHeadingText() {
 
 base::string16 BlockedActionBubbleDelegate::GetBodyText(
     bool anchored_to_action) {
-  return l10n_util::GetStringUTF16(IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_CONTENT);
+  return base::string16();
 }
 
 base::string16 BlockedActionBubbleDelegate::GetItemListText() {
@@ -45,8 +46,11 @@ base::string16 BlockedActionBubbleDelegate::GetActionButtonText() {
 }
 
 base::string16 BlockedActionBubbleDelegate::GetDismissButtonText() {
-  return l10n_util::GetStringUTF16(
-      IDS_EXTENSION_BLOCKED_ACTION_BUBBLE_CANCEL_BUTTON);
+  return base::string16();
+}
+
+ui::DialogButton BlockedActionBubbleDelegate::GetDefaultDialogButton() {
+  return ui::DIALOG_BUTTON_OK;
 }
 
 std::string BlockedActionBubbleDelegate::GetAnchorActionId() {

chrome/browser/ui/extensions/blocked_action_bubble_delegate.h

@@ -26,6 +26,7 @@ class BlockedActionBubbleDelegate : public ToolbarActionsBarBubbleDelegate {
   base::string16 GetItemListText() override;
   base::string16 GetActionButtonText() override;
   base::string16 GetDismissButtonText() override;
+  ui::DialogButton GetDefaultDialogButton() override;
   std::unique_ptr<ToolbarActionsBarBubbleDelegate::ExtraViewInfo>
   GetExtraViewInfo() override;
   std::string GetAnchorActionId() override;

chrome/browser/ui/extensions/extension_message_bubble_bridge.cc

@@ -70,6 +70,12 @@ base::string16 ExtensionMessageBubbleBridge::GetDismissButtonText() {
   return controller_->delegate()->GetDismissButtonLabel();
 }
 
+ui::DialogButton ExtensionMessageBubbleBridge::GetDefaultDialogButton() {
+  // TODO(estade): we should set a default where appropriate. See
+  // http://crbug.com/751279
+  return ui::DIALOG_BUTTON_NONE;
+}
+
 std::string ExtensionMessageBubbleBridge::GetAnchorActionId() {
   return controller_->GetExtensionIdList().size() == 1u
              ? controller_->GetExtensionIdList()[0]

chrome/browser/ui/extensions/extension_message_bubble_bridge.h

@@ -33,6 +33,7 @@ class ExtensionMessageBubbleBridge : public ToolbarActionsBarBubbleDelegate {
   base::string16 GetItemListText() override;
   base::string16 GetActionButtonText() override;
   base::string16 GetDismissButtonText() override;
+  ui::DialogButton GetDefaultDialogButton() override;
   std::unique_ptr<ExtraViewInfo> GetExtraViewInfo() override;
   std::string GetAnchorActionId() override;
   void OnBubbleShown(const base::Closure& close_bubble_callback) override;

chrome/browser/ui/toolbar/test_toolbar_actions_bar_bubble_delegate.cc

@@ -26,6 +26,9 @@ class TestToolbarActionsBarBubbleDelegate::DelegateImpl
   base::string16 GetItemListText() override { return parent_->item_list_; }
   base::string16 GetActionButtonText() override { return parent_->action_; }
   base::string16 GetDismissButtonText() override { return parent_->dismiss_; }
+  ui::DialogButton GetDefaultDialogButton() override {
+    return parent_->default_button_;
+  }
   std::unique_ptr<ToolbarActionsBarBubbleDelegate::ExtraViewInfo>
   GetExtraViewInfo() override {
     if (parent_->info_)
@@ -56,6 +59,7 @@ TestToolbarActionsBarBubbleDelegate::TestToolbarActionsBarBubbleDelegate(
       heading_(heading),
       body_(body),
       action_(action),
+      default_button_(ui::DIALOG_BUTTON_NONE),
       close_on_deactivate_(true) {}
 
 TestToolbarActionsBarBubbleDelegate::~TestToolbarActionsBarBubbleDelegate() {

chrome/browser/ui/toolbar/test_toolbar_actions_bar_bubble_delegate.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h"
+#include "ui/base/ui_base_types.h"
 
 // A test delegate for a bubble to hang off the toolbar actions bar.
 class TestToolbarActionsBarBubbleDelegate {
@@ -40,6 +41,9 @@ class TestToolbarActionsBarBubbleDelegate {
     info_->text = learn_more;
     info_->is_learn_more = true;
   }
+  void set_default_dialog_button(ui::DialogButton default_button) {
+    default_button_ = default_button;
+  }
   void set_item_list_text(const base::string16& item_list) {
     item_list_ = item_list;
   }
@@ -72,6 +76,9 @@ class TestToolbarActionsBarBubbleDelegate {
   base::string16 learn_more_;
   base::string16 item_list_;
 
+  // The default button for the bubble.
+  ui::DialogButton default_button_;
+
   // Whether to close the bubble on deactivation.
   bool close_on_deactivate_;
 

chrome/browser/ui/toolbar/toolbar_actions_bar_bubble_delegate.h

@@ -9,6 +9,7 @@
 
 #include "base/callback_forward.h"
 #include "base/strings/string16.h"
+#include "ui/base/ui_base_types.h"
 
 namespace gfx {
 struct VectorIcon;
@@ -73,6 +74,9 @@ class ToolbarActionsBarBubbleDelegate {
   // button will be added.
   virtual base::string16 GetDismissButtonText() = 0;
 
+  // Returns the button that should be set to the default.
+  virtual ui::DialogButton GetDefaultDialogButton() = 0;
+
   // Returns the id of the action to point to, or the empty string if the
   // bubble should point to the center of the actions container.
   virtual std::string GetAnchorActionId() = 0;

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

@@ -132,23 +132,27 @@ bool ToolbarActionsBarBubbleViews::Close() {
 }
 
 void ToolbarActionsBarBubbleViews::Init() {
+  base::string16 body_text_string = delegate_->GetBodyText(anchored_to_action_);
+  base::string16 item_list = delegate_->GetItemListText();
+  if (body_text_string.empty() && item_list.empty())
+    return;
+
   ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
   SetLayoutManager(std::make_unique<views::BoxLayout>(
       views::BoxLayout::kVertical, gfx::Insets(),
       provider->GetDistanceMetric(views::DISTANCE_RELATED_CONTROL_VERTICAL)));
 
-  // Add the content string.
-  views::Label* content_label =
-      new views::Label(delegate_->GetBodyText(anchored_to_action_));
-  content_label->SetMultiLine(true);
   int width = provider->GetDistanceMetric(
                   ChromeDistanceMetric::DISTANCE_BUBBLE_PREFERRED_WIDTH) -
               margins().width();
-  content_label->SizeToFit(width);
-  content_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  AddChildView(content_label);
 
-  base::string16 item_list = delegate_->GetItemListText();
+  if (!body_text_string.empty()) {
+    body_text_ = new views::Label(body_text_string);
+    body_text_->SetMultiLine(true);
+    body_text_->SizeToFit(width);
+    body_text_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    AddChildView(body_text_);
+  }
 
   if (!item_list.empty()) {
     item_list_ = new views::Label(item_list);
@@ -173,9 +177,7 @@ int ToolbarActionsBarBubbleViews::GetDialogButtons() const {
 }
 
 int ToolbarActionsBarBubbleViews::GetDefaultDialogButton() const {
-  // TODO(estade): we should set a default where appropriate. See
-  // http://crbug.com/751279
-  return ui::DIALOG_BUTTON_NONE;
+  return delegate_->GetDefaultDialogButton();
 }
 
 base::string16 ToolbarActionsBarBubbleViews::GetDialogButtonLabel(

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

@@ -33,6 +33,7 @@ class ToolbarActionsBarBubbleViews : public views::BubbleDialogDelegateView,
 
   void Show();
 
+  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_; }
 
@@ -56,6 +57,7 @@ class ToolbarActionsBarBubbleViews : public views::BubbleDialogDelegateView,
 
   std::unique_ptr<ToolbarActionsBarBubbleDelegate> delegate_;
   bool delegate_notified_of_close_ = false;
+  views::Label* body_text_ = nullptr;
   views::Label* item_list_ = nullptr;
   views::ImageButton* image_button_ = nullptr;
   const bool anchored_to_action_;

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

@@ -209,6 +209,38 @@ TEST_F(ToolbarActionsBarBubbleViewsTest, TestBubbleLayoutListView) {
   CloseBubble();
 }
 
+TEST_F(ToolbarActionsBarBubbleViewsTest, TestBubbleLayoutNoBodyText) {
+  TestToolbarActionsBarBubbleDelegate delegate(
+      HeadingString(), base::string16(), ActionString());
+  ShowBubble(&delegate);
+
+  EXPECT_TRUE(bubble()->GetDialogClientView()->ok_button());
+  EXPECT_EQ(ActionString(),
+            bubble()->GetDialogClientView()->ok_button()->GetText());
+  EXPECT_FALSE(bubble()->GetDialogClientView()->cancel_button());
+  EXPECT_FALSE(bubble()->learn_more_button());
+  EXPECT_FALSE(bubble()->body_text());
+  EXPECT_FALSE(bubble()->item_list());
+
+  CloseBubble();
+}
+
+TEST_F(ToolbarActionsBarBubbleViewsTest, TestBubbleDefaultDialogButtons) {
+  TestToolbarActionsBarBubbleDelegate delegate(HeadingString(), BodyString(),
+                                               ActionString());
+  delegate.set_dismiss_button_text(DismissString());
+  delegate.set_default_dialog_button(ui::DIALOG_BUTTON_OK);
+  ShowBubble(&delegate);
+
+  ASSERT_TRUE(bubble()->GetDialogClientView()->ok_button());
+  EXPECT_TRUE(bubble()->GetDialogClientView()->ok_button()->is_default());
+
+  ASSERT_TRUE(bubble()->GetDialogClientView()->cancel_button());
+  EXPECT_FALSE(bubble()->GetDialogClientView()->cancel_button()->is_default());
+
+  CloseBubble();
+}
+
 TEST_F(ToolbarActionsBarBubbleViewsTest, TestShowAndCloseBubble) {
   std::unique_ptr<views::Widget> anchor_widget = CreateAnchorWidget();
   TestToolbarActionsBarBubbleDelegate delegate(HeadingString(), BodyString(),