Move ShowPopup logic from BrowserActionsContainer to BrowserActionView

chrome/browser/ui/views/toolbar/browser_action_view.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_action_view.h"
 
+#include <string>
+
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/commands/command_service.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_context_menu_model.h"
-#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/extensions/accelerator_priority.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "extensions/common/extension.h"
@@ skipping 107 matching lines @@
   // to GTK requires that we regrab our browser action images.
   registrar_.Add(
       this,
       chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
       content::Source<ThemeService>(
           ThemeServiceFactory::GetForProfile(browser->profile())));
 }
 
 void BrowserActionButton::Destroy() {
   MaybeUnregisterExtensionCommand(false);
-
+  HidePopup();
   if (menu_runner_) {
     menu_runner_->Cancel();
     base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
   } else {
     delete this;
   }
 }
 
 void BrowserActionButton::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
@@ skipping 17 matching lines @@
   return true;
 }
 
 void BrowserActionButton::GetAccessibleState(ui::AXViewState* state) {
   views::MenuButton::GetAccessibleState(state);
   state->role = ui::AX_ROLE_BUTTON;
 }
 
 void BrowserActionButton::ButtonPressed(views::Button* sender,
                                         const ui::Event& event) {
-  delegate_->OnBrowserActionExecuted(this);
+  ExecuteBrowserAction();
 }
 
 void BrowserActionButton::ShowContextMenuForView(
     View* source,
     const gfx::Point& point,
     ui::MenuSourceType source_type) {
   if (!extension()->ShowConfigureContextMenus())
     return;
 
   SetButtonPushed();
 
   // Reconstructs the menu every time because the menu's contents are dynamic.
   scoped_refptr<ExtensionContextMenuModel> context_menu_contents(
-      new ExtensionContextMenuModel(extension(), browser_, delegate_));
+      new ExtensionContextMenuModel(extension(), browser_, this));
   gfx::Point screen_loc;
   views::View::ConvertPointToScreen(this, &screen_loc);
 
   views::Widget* parent = NULL;
   int run_types = views::MenuRunner::HAS_MNEMONICS |
                   views::MenuRunner::CONTEXT_MENU;
   if (delegate_->ShownInsideMenu()) {
     run_types |= views::MenuRunner::IS_NESTED;
     // RunMenuAt expects a nested menu to be parented by the same widget as the
     // already visible menu, in this case the Chrome menu.
@@ skipping 60 matching lines @@
 bool BrowserActionButton::IsPopup() {
   int tab_id = delegate_->GetCurrentTabId();
   return (tab_id < 0) ? false : browser_action_->HasPopup(tab_id);
 }
 
 GURL BrowserActionButton::GetPopupUrl() {
   int tab_id = delegate_->GetCurrentTabId();
   return (tab_id < 0) ? GURL() : browser_action_->GetPopupUrl(tab_id);
 }
 
+bool BrowserActionButton::ShowPopup(
+    ExtensionPopup::ShowAction show_action,
+    bool grant_tab_permissions) {
+  GURL popup_url;
+  if (delegate_->GetModel()->ExecuteBrowserAction(
+          extension_, browser_, &popup_url, grant_tab_permissions) ==
+      extensions::ExtensionToolbarModel::ACTION_NONE) {
+    return false;
+  }
+
+  // If we're showing the popup for this browser action, just hide it and

[Peter Kasting, 2014/07/29 21:49:48]

Nit: showing -> already showing

[Devlin, 2014/07/30 16:14:26]

Done.

+  // return.
+  bool already_showing = popup_ != NULL;
+
+  // Always hide the current popup, even if it's not the same.
+  // Only one popup should be visible at a time.
+  delegate_->HideActivePopup();
+  if (already_showing)
+    return false;
+
+  // Browser actions in the overflow menu can still show popups, so we may need
+  // a reference view other than this view. If so, use the overflow view.

[Yoyo Zhou, 2014/07/30 01:46:33]

"other than this button"?

[Devlin, 2014/07/30 16:14:26]

Done.

+  views::View* reference_view =
+      parent()->visible() ? this : delegate_->GetOverflowReferenceView();
+
+  popup_ = ExtensionPopup::ShowPopup(popup_url,
+                                     browser_,
+                                     reference_view,
+                                     views::BubbleBorder::TOP_RIGHT,
+                                     show_action);
+  popup_->GetWidget()->AddObserver(this);
+  delegate_->SetPopupOwner(this);
+
+  // Only set button as pushed if it was triggered by a user click.
+  if (grant_tab_permissions)
+    SetButtonPushed();
+  return true;
+}
+
+void BrowserActionButton::HidePopup() {
+  if (popup_)
+    CleanupPopup(true);
+}
+
+void BrowserActionButton::ExecuteBrowserAction() {
+  ShowPopup(ExtensionPopup::SHOW, true);
+}
+
 void BrowserActionButton::Observe(int type,
                                   const content::NotificationSource& source,
                                   const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_UPDATED:
       UpdateState();
       // The browser action may have become visible/hidden so we need to make
       // sure the state gets updated.
       delegate_->OnBrowserActionVisibilityChanged();
       break;
@@ skipping 24 matching lines @@
 void BrowserActionButton::OnIconUpdated() {
   UpdateState();
   if (icon_observer_)
     icon_observer_->OnIconUpdated(GetIconWithBadge());
 }
 
 bool BrowserActionButton::Activate() {
   if (!IsPopup())
     return true;
 
-  delegate_->OnBrowserActionExecuted(this);
+  ExecuteBrowserAction();
 
   // TODO(erikkay): Run a nested modal loop while the mouse is down to
   // enable menu-like drag-select behavior.
 
   // The return value of this method is returned via OnMousePressed.
   // We need to return false here since we're handing off focus to another
   // widget/view, and true will grab it right back and try to send events
   // to us.
   return false;
 }
@@ skipping 50 matching lines @@
 }
 
 bool BrowserActionButton::AcceleratorPressed(
     const ui::Accelerator& accelerator) {
   // Normal priority shortcuts must be handled via standard browser commands to
   // be processed at the proper time.
   if (GetAcceleratorPriority(accelerator, extension_) ==
       ui::AcceleratorManager::kNormalPriority)
     return false;
 
-  delegate_->OnBrowserActionExecuted(this);
+  ExecuteBrowserAction();
   return true;
 }
 
 void BrowserActionButton::SetButtonPushed() {
   SetState(views::CustomButton::STATE_PRESSED);
   menu_visible_ = true;
 }
 
 void BrowserActionButton::SetButtonNotPushed() {
   SetState(views::CustomButton::STATE_NORMAL);
@@ skipping 13 matching lines @@
   return browser_action_->GetIconWithBadge(icon, tab_id, spacing);
 }
 
 gfx::ImageSkia BrowserActionButton::GetIconForTest() {
   return GetImage(views::Button::STATE_NORMAL);
 }
 
 BrowserActionButton::~BrowserActionButton() {
 }
 
+void BrowserActionButton::InspectPopup(ExtensionAction* action) {
+  DCHECK_EQ(browser_action_, action);
+  ShowPopup(ExtensionPopup::SHOW_AND_INSPECT, true);  // grant active tab

[Yoyo Zhou, 2014/07/30 01:46:33]

May as well be consistent about commenting on boolean parameters.

[Devlin, 2014/07/30 16:14:26]

Done.

+}
+
+void BrowserActionButton::OnWidgetDestroying(views::Widget* widget) {
+  DCHECK(popup_);
+  DCHECK_EQ(popup_->GetWidget(), widget);
+  CleanupPopup(false);
+}
+
 void BrowserActionButton::MaybeRegisterExtensionCommand() {
   extensions::CommandService* command_service =
       extensions::CommandService::Get(browser_->profile());
   extensions::Command browser_action_command;
   if (command_service->GetBrowserActionCommand(
           extension_->id(),
           extensions::CommandService::ACTIVE_ONLY,
           &browser_action_command,
           NULL)) {
     keybinding_.reset(new ui::Accelerator(
@@ skipping 16 matching lines @@
   extensions::Command browser_action_command;
   if (!only_if_active || !command_service->GetBrowserActionCommand(
           extension_->id(),
           extensions::CommandService::ACTIVE_ONLY,
           &browser_action_command,
           NULL)) {
     GetFocusManager()->UnregisterAccelerator(*keybinding_.get(), this);
     keybinding_.reset(NULL);
   }
 }
+
+void BrowserActionButton::CleanupPopup(bool close_widget) {
+  DCHECK(popup_);
+  // We need to do these actions synchronously (instead of closing and then
+  // performing the rest of the cleanup in OnWidgetDestroyed()) because
+  // OnWidgetDestroyed() can be called asynchronously from Close(), and we need
+  // to keep the delegate's popup owner up-to-date.
+  popup_->GetWidget()->RemoveObserver(this);
+  if (close_widget)
+    popup_->GetWidget()->Close();
+  popup_ = NULL;
+  SetButtonNotPushed();
+  delegate_->SetPopupOwner(NULL);
+}