Move ShowPopup logic from BrowserActionsContainer to BrowserActionView

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

 // Copyright 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/toolbar/browser_actions_container.h"
 
 #include "base/compiler_specific.h"
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
-#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/extension_view_host.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/extensions/browser_action_drag_data.h"
 #include "chrome/browser/ui/views/extensions/extension_keybinding_registry_views.h"
 #include "chrome/browser/ui/views/extensions/extension_popup.h"
-#include "chrome/browser/ui/views/toolbar/browser_action_view.h"
+#include "chrome/browser/ui/views/toolbar/browser_actions_container_observer.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/extensions/command.h"
-#include "chrome/common/pref_names.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/browser/runtime_data.h"
 #include "extensions/common/feature_switch.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "grit/ui_resources.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/dragdrop/drag_utils.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/nine_image_painter_factory.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/animation/slide_animation.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/views/controls/button/label_button_border.h"
+#include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/resize_area.h"
 #include "ui/views/metrics.h"
 #include "ui/views/painter.h"
 #include "ui/views/widget/widget.h"
 
 using extensions::Extension;
 
 namespace {
 
 // Horizontal spacing between most items in the container, as well as after the
@@ skipping 51 matching lines @@
 // BrowserActionsContainer
 
 BrowserActionsContainer::BrowserActionsContainer(
     Browser* browser,
     View* owner_view,
     BrowserActionsContainer* main_container)
     : profile_(browser->profile()),
       browser_(browser),
       owner_view_(owner_view),
       main_container_(main_container),
-      popup_(NULL),
-      popup_button_(NULL),
+      popup_owner_(NULL),
       model_(NULL),
       container_width_(0),
       resize_area_(NULL),
       chevron_(NULL),
       overflow_menu_(NULL),
       suppress_chevron_(false),
       resize_amount_(0),
       animation_target_size_(0),
       drop_indicator_position_(-1),
       task_factory_(this),
@@ skipping 36 matching lines @@
 BrowserActionsContainer::~BrowserActionsContainer() {
   FOR_EACH_OBSERVER(BrowserActionsContainerObserver,
                     observers_,
                     OnBrowserActionsContainerDestroyed());
 
   if (overflow_menu_)
     overflow_menu_->set_observer(NULL);
   if (model_)
     model_->RemoveObserver(this);
   StopShowFolderDropMenuTimer();
-  if (popup_)
-    popup_->GetWidget()->RemoveObserver(this);
-  HidePopup();
+  HideActivePopup();
   DeleteBrowserActionViews();
 }
 
 void BrowserActionsContainer::Init() {
   LoadImages();
 
   // We wait to set the container width until now so that the chevron images
   // will be loaded.  The width calculation needs to know the chevron size.
   if (model_ &&
       !profile_->GetPrefs()->HasPrefPath(
@@ skipping 35 matching lines @@
     if (!ShouldDisplayBrowserAction(i->get()))
       continue;
 
     BrowserActionView* view = new BrowserActionView(i->get(), browser_, this);
     browser_action_views_.push_back(view);
     AddChildView(view);
   }
 }
 
 void BrowserActionsContainer::DeleteBrowserActionViews() {
-  HidePopup();
+  HideActivePopup();
   if (overflow_menu_)
     overflow_menu_->NotifyBrowserActionViewsDeleting();
   STLDeleteElements(&browser_action_views_);
 }
 
 size_t BrowserActionsContainer::VisibleBrowserActions() const {
   size_t visible_actions = 0;
   for (size_t i = 0; i < browser_action_views_.size(); ++i) {
     if (browser_action_views_[i]->visible())
       ++visible_actions;
@@ skipping 24 matching lines @@
 
 void BrowserActionsContainer::OnBrowserActionViewDragDone() {
   // We notify here as well as in OnPerformDrop because the dragged view is
   // removed in OnPerformDrop, so it will never get its OnDragDone() call.
   // TODO(devlin): we should see about fixing that.
   FOR_EACH_OBSERVER(BrowserActionsContainerObserver,
                     observers_,
                     OnBrowserActionDragDone());
 }
 
+views::View* BrowserActionsContainer::GetOverflowReferenceView() {
+  // We should only need an overflow reference when using the traditional
+  // chevron overflow.
+  DCHECK(chevron_);
+  return chevron_;
+}
+
+void BrowserActionsContainer::SetPopupOwner(BrowserActionButton* popup_owner) {
+  // We should never be setting a popup owner when one already exists.
+  DCHECK(!popup_owner_);
+  popup_owner_ = popup_owner;
+}
+
+void BrowserActionsContainer::HideActivePopup() {
+  if (popup_owner_)
+    popup_owner_->HidePopup();
+}
+
+extensions::ExtensionToolbarModel* BrowserActionsContainer::GetModel() {
+  return model_;
+}
+
 void BrowserActionsContainer::AddObserver(
     BrowserActionsContainerObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void BrowserActionsContainer::RemoveObserver(
     BrowserActionsContainerObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
@@ skipping 316 matching lines @@
                     observers_,
                     OnBrowserActionsContainerAnimationEnded());
 }
 
 void BrowserActionsContainer::NotifyMenuDeleted(
     BrowserActionOverflowMenuController* controller) {
   DCHECK_EQ(overflow_menu_, controller);
   overflow_menu_ = NULL;
 }
 
-void BrowserActionsContainer::OnWidgetDestroying(views::Widget* widget) {
-  DCHECK_EQ(popup_->GetWidget(), widget);
-  popup_->GetWidget()->RemoveObserver(this);
-  popup_ = NULL;
-  // |popup_button_| is NULL if the extension has been removed.
-  if (popup_button_) {
-    popup_button_->SetButtonNotPushed();
-    popup_button_ = NULL;
-  }
-}
-
-void BrowserActionsContainer::InspectPopup(ExtensionAction* action) {
-  BrowserActionView* view = GetBrowserActionView(action);
-  ShowPopup(view->button(), ExtensionPopup::SHOW_AND_INSPECT, true);
-}
-
 int BrowserActionsContainer::GetCurrentTabId() const {
   content::WebContents* active_tab =
       browser_->tab_strip_model()->GetActiveWebContents();
   if (!active_tab)
     return -1;
 
   return SessionTabHelper::FromWebContents(active_tab)->session_id().id();
 }
 
-void BrowserActionsContainer::OnBrowserActionExecuted(
-    BrowserActionButton* button) {
-  ShowPopup(button, ExtensionPopup::SHOW, true);
-}
-
 void BrowserActionsContainer::OnBrowserActionVisibilityChanged() {
   SetVisible(!browser_action_views_.empty());
   if (owner_view_) {
     owner_view_->Layout();
     owner_view_->SchedulePaint();
   }
 }
 
 extensions::ActiveTabPermissionGranter*
     BrowserActionsContainer::GetActiveTabPermissionGranter() {
   content::WebContents* web_contents =
       browser_->tab_strip_model()->GetActiveWebContents();
   if (!web_contents)
     return NULL;
   return extensions::TabHelper::FromWebContents(web_contents)->
       active_tab_permission_granter();
 }
 
 void BrowserActionsContainer::MoveBrowserAction(const std::string& extension_id,
                                                 size_t new_index) {
-  ExtensionService* service =
-      extensions::ExtensionSystem::Get(profile_)->extension_service();
-  if (service) {
-    const Extension* extension = service->GetExtensionById(extension_id, false);
-    model_->MoveBrowserAction(extension, new_index);
-    SchedulePaint();
-  }
+  const Extension* extension = extensions::ExtensionRegistry::Get(profile_)
+      ->enabled_extensions().GetByID(extension_id);

[Peter Kasting, 2014/07/26 02:33:20]

Nit: See prior nit about -> wrapping

[Devlin, 2014/07/29 19:07:15]

Done.

+  model_->MoveBrowserAction(extension, new_index);
+  SchedulePaint();
 }
 
-bool BrowserActionsContainer::ShowPopup(const extensions::Extension* extension,
-                                        bool should_grant) {
-  // Do not override other popups and only show in active window. The window
-  // must also have a toolbar, otherwise it should not be showing popups.
-  // TODO(justinlin): Remove toolbar check when http://crbug.com/308645 is
-  // fixed.
-  if (popup_ ||
-      !browser_->window()->IsActive() ||
-      !browser_->window()->IsToolbarVisible()) {
-    return false;
-  }
-
-  for (BrowserActionViews::iterator it = browser_action_views_.begin();
-       it != browser_action_views_.end(); ++it) {
-    BrowserActionButton* button = (*it)->button();
-    if (button && button->extension() == extension)
-      return ShowPopup(button, ExtensionPopup::SHOW, should_grant);
-  }
-  return false;
-}
-
-void BrowserActionsContainer::HidePopup() {
-  // Remove this as an observer and clear |popup_| and |popup_button_| here,
-  // since we might change them before OnWidgetDestroying() gets called.
-  if (popup_) {
-    popup_->GetWidget()->RemoveObserver(this);
-    popup_->GetWidget()->Close();
-    popup_ = NULL;
-  }
-  if (popup_button_) {
-    popup_button_->SetButtonNotPushed();
-    popup_button_ = NULL;
-  }
-}
-
-void BrowserActionsContainer::TestExecuteBrowserAction(int index) {
-  BrowserActionButton* button = browser_action_views_[index]->button();
-  OnBrowserActionExecuted(button);
+ExtensionPopup* BrowserActionsContainer::TestGetPopup() {
+  return popup_owner_ ? popup_owner_->popup() : NULL;
 }
 
 void BrowserActionsContainer::TestSetIconVisibilityCount(size_t icons) {
   model_->SetVisibleIconCount(icons);
   chevron_->SetVisible(icons < browser_action_views_.size());
   container_width_ = IconCountToWidth(icons, chevron_->visible());
   Layout();
   SchedulePaint();
 }
 
@@ skipping 101 matching lines @@
     SaveDesiredSizeAndAnimate(gfx::Tween::LINEAR, visible_actions + 1);
   } else {
     // Just redraw the (possibly modified) visible icon set.
     OnBrowserActionVisibilityChanged();
   }
 }
 
 void BrowserActionsContainer::BrowserActionRemoved(const Extension* extension) {
   CloseOverflowMenu();
 
-  if (popup_ && popup_->host()->extension() == extension)
-    HidePopup();
-
   size_t visible_actions = VisibleBrowserActionsAfterAnimation();
   for (BrowserActionViews::iterator i(browser_action_views_.begin());
        i != browser_action_views_.end(); ++i) {
     if ((*i)->button()->extension() == extension) {
       delete *i;
       browser_action_views_.erase(i);
 
       // If the extension is being upgraded we don't want the bar to shrink
       // because the icon is just going to get re-added to the same location.
       if (extensions::ExtensionSystem::Get(profile_)->runtime_data()->
@@ skipping 28 matching lines @@
     index = model_->OriginalIndexToIncognito(index);
 
   DCHECK(index >= 0 && index < static_cast<int>(browser_action_views_.size()));
 
   DeleteBrowserActionViews();
   CreateBrowserActionViews();
   Layout();
   SchedulePaint();
 }
 
-bool BrowserActionsContainer::BrowserActionShowPopup(
-    const extensions::Extension* extension) {
-  return ShowPopup(extension, false);
-}
-
 void BrowserActionsContainer::VisibleCountChanged() {
   SetContainerWidth();
 }
 
 void BrowserActionsContainer::HighlightModeChanged(bool is_highlighting) {
   // The visual highlighting is done in OnPaint(). It's a bit of a pain that
   // we delete and recreate everything here, but that's how it's done in
   // BrowserActionMoved(), too. If we want to optimize it, we could move the
   // existing icons, instead of deleting it all.
   DeleteBrowserActionViews();
@@ skipping 130 matching lines @@
   }
 }
 
 bool BrowserActionsContainer::ShouldDisplayBrowserAction(
     const Extension* extension) {
   // Only display incognito-enabled extensions while in incognito mode.
   return !profile_->IsOffTheRecord() ||
       extensions::util::IsIncognitoEnabled(extension->id(), profile_);
 }
 
-bool BrowserActionsContainer::ShowPopup(
-    BrowserActionButton* button,
-    ExtensionPopup::ShowAction show_action,
-    bool should_grant) {
-  const Extension* extension = button->extension();
-  GURL popup_url;
-  if (model_->ExecuteBrowserAction(
-          extension, browser_, &popup_url, should_grant) !=
-      extensions::ExtensionToolbarModel::ACTION_SHOW_POPUP) {
-    return false;
+bool BrowserActionsContainer::ShowPopupForExtension(
+    const extensions::Extension* extension,
+    bool grant_tab_permissions,
+    bool can_override) {
+  if (!can_override) {
+    // If the popup cannot override other views, then no other popups can be

[Peter Kasting, 2014/07/26 02:33:20]

Nit: Hoist this comment above the above conditional, then combine the nested
conditionals.

[Devlin, 2014/07/29 19:07:14]

Done.

+    // showing, and it must be shown in the active widow with a visible toolbar.
+    // TODO(justinlin): Remove toolbar check when http://crbug.com/308645 is
+    // fixed.
+    if (popup_owner_ ||
+        !browser_->window()->IsActive() ||
+        !browser_->window()->IsToolbarVisible()) {
+      return false;
+    }
   }
 
-  // If we're showing the same popup, just hide it and return.
-  bool same_showing = popup_ && button == popup_button_;
-
-  // Always hide the current popup, even if it's not the same.
-  // Only one popup should be visible at a time.
-  HidePopup();
-
-  if (same_showing)
-    return false;
-
-  // We can get the execute event for browser actions that are not visible,
-  // since buttons can be activated from the overflow menu (chevron). In that
-  // case we show the popup as originating from the chevron.
-  View* reference_view = button->parent()->visible() ? button : chevron_;
-  popup_ = ExtensionPopup::ShowPopup(popup_url, browser_, reference_view,
-                                     views::BubbleBorder::TOP_RIGHT,
-                                     show_action);
-  popup_->GetWidget()->AddObserver(this);
-  popup_button_ = button;
-
-  // Only set button as pushed if it was triggered by a user click.
-  if (should_grant)
-    popup_button_->SetButtonPushed();
-  return true;
+  for (BrowserActionViews::iterator iter = browser_action_views_.begin();
+       iter != browser_action_views_.end(); ++iter) {
+    if ((*iter)->button()->extension() == extension)
+      return (*iter)->button()->ShowPopup(ExtensionPopup::SHOW,
+                                          grant_tab_permissions);

[Peter Kasting, 2014/07/26 02:33:20]

Nit: Same length, but feels a little nicer to me:

    BrowserActionButton* button = (*iter)->button();
    if (button->extension() == extension)
      return button->ShowPopup(ExtensionPopup::SHOW, grant_tab_permissions);

[Devlin, 2014/07/29 19:07:15]

Done.

+  }
+  return false;
 }