Refactor the extension message bubbles.

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

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/extensions/extension_message_bubble_view.h"
 
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/dev_mode_bubble_controller.h"
@@ skipping 36 matching lines @@
 
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // ExtensionMessageBubbleView
 
 namespace extensions {
 
 ExtensionMessageBubbleView::ExtensionMessageBubbleView(
     views::View* anchor_view,
-    ExtensionMessageBubbleController::Delegate* delegate)
+    ExtensionMessageBubbleController* controller)
     : BubbleDelegateView(anchor_view, views::BubbleBorder::TOP_RIGHT),
       weak_factory_(this),
-      delegate_(delegate),
+      controller_(controller),
       headline_(NULL),
       learn_more_(NULL),
       dismiss_button_(NULL),
       link_clicked_(false),
       action_taken_(false) {
   DCHECK(anchor_view->GetWidget());
   set_close_on_deactivate(false);
   set_move_with_anchor(true);
   set_close_on_esc(true);
 
   // Compensate for built-in vertical padding in the anchor view's image.
   set_anchor_view_insets(gfx::Insets(5, 0, 5, 0));
 }
 
 // static
 void ExtensionMessageBubbleView::MaybeShow(

[not at google - send to devlin, 2014/01/10 21:28:43]

i still think it's odd to have this on extension_message_bubble_view rather than
where the MaybeShow is called from, but, i won't object at this point :)

[Finnur, 2014/01/13 15:36:32]

Yup. I believe we are in agreement. Someone wants to add a third type of bubble
(listing admin force-installed extensions that continuously crash) and that
necessitates some queuing mechanism for the bubbles (because both the DevMode
bubble and that bubble want to always show and we don't want multiple bubbles
popping up at once). This code should therefore, when that is implemented, be
moved there.

     Browser* browser,
     ToolbarView* toolbar_view,
     views::View* anchor_view) {
 #if defined(OS_WIN)
   // The list of suspicious extensions takes priority over the dev mode bubble,
   // since that needs to be shown as soon as we disable something. The dev mode
   // bubble is not as time sensitive so we'll catch the dev mode extensions on
   // the next startup/next window that opens. That way, we're not too spammy
   // with the bubbles.
-  SuspiciousExtensionBubbleController* suspicious_extensions =
-      extensions::SuspiciousExtensionBubbleController::Get(
-          browser->profile());
+  scoped_ptr<SuspiciousExtensionBubbleController> suspicious_extensions(
+      new SuspiciousExtensionBubbleController(browser->profile()));
   if (suspicious_extensions->ShouldShow()) {
+    // Show the ExtensionMessageBubbleView and pass ownership of |controller| to
+    // the view.

[not at google - send to devlin, 2014/01/10 21:28:43]

i think this comment, particularly after the scoped_ptr change, is self
explanatory 

[Finnur, 2014/01/13 15:36:32]

Done.

+    SuspiciousExtensionBubbleController* controller =
+        suspicious_extensions.release();

[not at google - send to devlin, 2014/01/10 21:28:43]

when the View takes a scoped_ptr<controller> this would be .get() and when
constructing the view suspicious_extensions.Pass()

[Finnur, 2014/01/13 15:36:32]

Done.

     ExtensionMessageBubbleView* bubble_delegate =
-        new ExtensionMessageBubbleView(anchor_view, suspicious_extensions);
+        new ExtensionMessageBubbleView(anchor_view, controller);
     views::BubbleDelegateView::CreateBubble(bubble_delegate);
-    suspicious_extensions->Show(bubble_delegate);
+    controller->Show(bubble_delegate);
     return;
   }
 
-  DevModeBubbleController* dev_mode_extensions =
-      extensions::DevModeBubbleController::Get(
-          browser->profile());
+  scoped_ptr<DevModeBubbleController> dev_mode_extensions(
+      new DevModeBubbleController(browser->profile()));
   if (dev_mode_extensions->ShouldShow()) {
     views::View* reference_view = NULL;
     BrowserActionsContainer* container = toolbar_view->browser_actions();
     if (container->animating())
       return;
 
     ExtensionService* service = extensions::ExtensionSystem::Get(
         browser->profile())->extension_service();
     extensions::ExtensionActionManager* extension_action_manager =
         extensions::ExtensionActionManager::Get(browser->profile());
@@ skipping 14 matching lines @@
     }
     if (reference_view) {
       // If we have a view, it means we found a browser action and we want to
       // point to the chevron, not the hotdog menu.
       if (!reference_view->visible())
         reference_view = container->chevron();  // It's hidden, use the chevron.
     }
     if (reference_view && reference_view->visible())
       anchor_view = reference_view;  // Catch-all is the hotdog menu.
 
-    // Show the bubble.
+    // Show the ExtensionMessageBubbleView and pass ownership of |controller| to
+    // the view.

[not at google - send to devlin, 2014/01/10 21:28:43]

likewise x2

[Finnur, 2014/01/13 15:36:32]

Done.

+    DevModeBubbleController* controller = dev_mode_extensions.release();
     ExtensionMessageBubbleView* bubble_delegate =
-        new ExtensionMessageBubbleView(anchor_view, dev_mode_extensions);
+        new ExtensionMessageBubbleView(anchor_view, controller);
     views::BubbleDelegateView::CreateBubble(bubble_delegate);
-    dev_mode_extensions->Show(bubble_delegate);
+    controller->Show(bubble_delegate);
   }
 #endif
 }
 
 void ExtensionMessageBubbleView::OnActionButtonClicked(
     const base::Closure& callback) {
   action_callback_ = callback;
 }
 
 void ExtensionMessageBubbleView::OnDismissButtonClicked(
@@ skipping 40 matching lines @@
 }
 
 void ExtensionMessageBubbleView::Init() {
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
 
   views::GridLayout* layout = views::GridLayout::CreatePanel(this);
   layout->SetInsets(kInsetTop, kInsetLeft,
                     kInsetBottomRight, kInsetBottomRight);
   SetLayoutManager(layout);
 
+  ExtensionMessageBubbleController::Delegate* delegate =
+      controller_->delegate();
+
   const int headline_column_set_id = 0;
   views::ColumnSet* top_columns = layout->AddColumnSet(headline_column_set_id);
   top_columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER,
                          0, views::GridLayout::USE_PREF, 0, 0);
   top_columns->AddPaddingColumn(1, 0);
   layout->StartRow(0, headline_column_set_id);
 
-  headline_ = new views::Label(delegate_->GetTitle(),
+  headline_ = new views::Label(delegate->GetTitle(),
                                rb.GetFontList(ui::ResourceBundle::MediumFont));
   layout->AddView(headline_);
 
   layout->AddPaddingRow(0, kHeadlineRowPadding);
 
   const int text_column_set_id = 1;
   views::ColumnSet* upper_columns = layout->AddColumnSet(text_column_set_id);
   upper_columns->AddColumn(
       views::GridLayout::LEADING, views::GridLayout::LEADING,
       0, views::GridLayout::USE_PREF, 0, 0);
   layout->StartRow(0, text_column_set_id);
 
   views::Label* message = new views::Label();
   message->SetMultiLine(true);
   message->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  message->SetText(delegate_->GetMessageBody());
+  message->SetText(delegate->GetMessageBody());
   message->SizeToFit(views::Widget::GetLocalizedContentsWidth(
       IDS_EXTENSION_WIPEOUT_BUBBLE_WIDTH_CHARS));
   layout->AddView(message);
 
-  if (delegate_->ShouldShowExtensionList()) {
+  if (delegate->ShouldShowExtensionList()) {
     const int extension_list_column_set_id = 2;
     views::ColumnSet* middle_columns =
         layout->AddColumnSet(extension_list_column_set_id);
     middle_columns->AddPaddingColumn(0, kExtensionListPadding);
     middle_columns->AddColumn(
         views::GridLayout::LEADING, views::GridLayout::CENTER,
         0, views::GridLayout::USE_PREF, 0, 0);
 
     layout->StartRowWithPadding(0, extension_list_column_set_id,
         0, kHeadlineMessagePadding);
     views::Label* extensions = new views::Label();
     extensions->SetMultiLine(true);
     extensions->SetHorizontalAlignment(gfx::ALIGN_LEFT);
 
     std::vector<base::string16> extension_list;
     base::char16 bullet_point = 0x2022;
 
-    std::vector<base::string16> suspicious = delegate_->GetExtensions();
+    std::vector<base::string16> suspicious = controller_->GetExtensionList();
     size_t i = 0;
     for (; i < suspicious.size() && i < kMaxExtensionsToShow; ++i) {
       // Add each extension with bullet point.
       extension_list.push_back(
           bullet_point + base::ASCIIToUTF16(" ") + suspicious[i]);
     }
 
     if (i > kMaxExtensionsToShow) {
       base::string16 difference = base::IntToString16(i - kMaxExtensionsToShow);
       extension_list.push_back(bullet_point + base::ASCIIToUTF16(" ") +
-          delegate_->GetOverflowText(difference));
+          delegate->GetOverflowText(difference));
     }
 
     extensions->SetText(JoinString(extension_list, base::ASCIIToUTF16("\n")));
     extensions->SizeToFit(views::Widget::GetLocalizedContentsWidth(
         IDS_EXTENSION_WIPEOUT_BUBBLE_WIDTH_CHARS));
     layout->AddView(extensions);
   }
 
-  base::string16 action_button = delegate_->GetActionButtonLabel();
+  base::string16 action_button = delegate->GetActionButtonLabel();
 
   const int action_row_column_set_id = 3;
   views::ColumnSet* bottom_columns =
       layout->AddColumnSet(action_row_column_set_id);
   bottom_columns->AddColumn(views::GridLayout::LEADING,
       views::GridLayout::CENTER, 0, views::GridLayout::USE_PREF, 0, 0);
   bottom_columns->AddPaddingColumn(1, 0);
   bottom_columns->AddColumn(views::GridLayout::TRAILING,
       views::GridLayout::CENTER, 0, views::GridLayout::USE_PREF, 0, 0);
   if (!action_button.empty()) {
     bottom_columns->AddColumn(views::GridLayout::TRAILING,
         views::GridLayout::CENTER, 0, views::GridLayout::USE_PREF, 0, 0);
   }
   layout->StartRowWithPadding(0, action_row_column_set_id,
                               0, kMessageBubblePadding);
 
-  learn_more_ = new views::Link(delegate_->GetLearnMoreLabel());
+  learn_more_ = new views::Link(delegate->GetLearnMoreLabel());
   learn_more_->set_listener(this);
   layout->AddView(learn_more_);
 
   if (!action_button.empty()) {
     action_button_ = new views::LabelButton(this, action_button.c_str());
     action_button_->SetStyle(views::Button::STYLE_NATIVE_TEXTBUTTON);
     layout->AddView(action_button_);
   }
 
   dismiss_button_ = new views::LabelButton(this,
-      delegate_->GetDismissButtonLabel());
+      delegate->GetDismissButtonLabel());
   dismiss_button_->SetStyle(views::Button::STYLE_NATIVE_TEXTBUTTON);
   layout->AddView(dismiss_button_);
 }
 
 void ExtensionMessageBubbleView::ButtonPressed(views::Button* sender,
                                                const ui::Event& event) {
   if (sender == action_button_) {
     action_taken_ = true;
     action_callback_.Run();
   } else {
@@ skipping 15 matching lines @@
   state->role = ui::AccessibilityTypes::ROLE_ALERT;
 }
 
 void ExtensionMessageBubbleView::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
   if (details.is_add && details.child == this)
     NotifyAccessibilityEvent(ui::AccessibilityTypes::EVENT_ALERT, true);
 }
 
 }  // namespace extensions