Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(582)

Unified Diff: chrome/browser/ui/extensions/extension_action_view_controller.cc

Issue 1168383002: Implement sidebar support for extension action popups Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/extensions/extension_action_view_controller.cc
diff --git a/chrome/browser/ui/extensions/extension_action_view_controller.cc b/chrome/browser/ui/extensions/extension_action_view_controller.cc
index 182a90eeab6ae47308477d7300ccf35e6b55d19c..298370f5d7318dcdb7fef12fcc2ec605f324acf7 100644
--- a/chrome/browser/ui/extensions/extension_action_view_controller.cc
+++ b/chrome/browser/ui/extensions/extension_action_view_controller.cc
@@ -17,10 +17,11 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/extensions/accelerator_priority.h"
#include "chrome/browser/ui/extensions/extension_action_platform_delegate.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/toolbar/toolbar_action_view_delegate.h"
#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
-#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
#include "chrome/common/extensions/api/extension_action/action_info.h"
+#include "chrome/common/pref_names.h"
#include "extensions/browser/extension_host.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/common/extension.h"
@@ -148,14 +149,15 @@ bool ExtensionActionViewController::HasPopup(
}
void ExtensionActionViewController::HidePopup() {
- if (is_showing_popup()) {
- popup_host_->Close();
- // We need to do these actions synchronously (instead of closing and then
- // performing the rest of the cleanup in OnExtensionHostDestroyed()) because
- // the extension host can close asynchronously, and we need to keep the view
- // delegate up-to-date.
- OnPopupClosed();
- }
+ if (!is_showing_popup())
+ return;
+
+ popup_host_->Close();
+ // We need to do these actions synchronously (instead of closing and then
+ // performing the rest of the cleanup in OnExtensionHostDestroyed()) because
+ // the extension host can close asynchronously, and we need to keep the view
+ // delegate up-to-date.
+ OnPopupClosed();
}
gfx::NativeView ExtensionActionViewController::GetPopupNativeView() {
@@ -215,8 +217,13 @@ bool ExtensionActionViewController::ExecuteAction(PopupShowAction show_action,
ExtensionAction::ACTION_SHOW_POPUP) {
GURL popup_url = extension_action_->GetPopupUrl(
SessionTabHelper::IdForTab(view_delegate_->GetCurrentWebContents()));
- return GetPreferredPopupViewController()
- ->TriggerPopupWithUrl(show_action, popup_url, grant_tab_permissions);
+
+ if (extension_action_->open_in_sidebar())
+ return GetPreferredPopupViewController()->TriggerSidebarWithUrl(
Devlin 2015/07/30 17:56:46 The fact that so much of this code seems to be cop
ltilve 2015/08/07 16:15:02 We need to handle a case which opens a popup when
+ popup_url);
+ else
+ return GetPreferredPopupViewController()->TriggerPopupWithUrl(
+ show_action, popup_url, grant_tab_permissions);
}
return false;
}
@@ -300,20 +307,16 @@ bool ExtensionActionViewController::TriggerPopupWithUrl(
PopupShowAction show_action,
const GURL& popup_url,
bool grant_tab_permissions) {
- if (!ExtensionIsValid())
- return false;
-
- bool already_showing = is_showing_popup();
// Always hide the current popup, even if it's not owned by this extension.
// Only one popup should be visible at a time.
- HideActivePopup();
-
// If we were showing a popup already, then we treat the action to open the
// same one as a desire to close it (like clicking a menu button that was
// already open).
- if (already_showing)
+ if (is_showing_popup()) {
+ HideActivePopup();
return false;
+ }
scoped_ptr<extensions::ExtensionViewHost> host(
extensions::ExtensionViewHostFactory::CreatePopupHost(popup_url,
@@ -326,21 +329,32 @@ bool ExtensionActionViewController::TriggerPopupWithUrl(
if (toolbar_actions_bar_)
toolbar_actions_bar_->SetPopupOwner(this);
- if (toolbar_actions_bar_ &&
- !toolbar_actions_bar_->IsActionVisible(this) &&
- extensions::FeatureSwitch::extension_action_redesign()->IsEnabled()) {
- platform_delegate_->CloseOverflowMenu();
- toolbar_actions_bar_->PopOutAction(
- this,
- base::Bind(&ExtensionActionViewController::ShowPopup,
- weak_factory_.GetWeakPtr(),
- base::Passed(host.Pass()),
- grant_tab_permissions,
- show_action));
- } else {
- ShowPopup(host.Pass(), grant_tab_permissions, show_action);
+ PressButtonWithSlideOutIfEnabled(base::Bind(
+ &ExtensionActionViewController::ShowPopup, weak_factory_.GetWeakPtr(),
+ base::Passed(host.Pass()), grant_tab_permissions, show_action));
+
+ return true;
+}
+
+bool ExtensionActionViewController::TriggerSidebarWithUrl(
+ const GURL& popup_url) {
+ if (sidebar_container_) {
+ HideActiveSidebar();
+ return false;
}
+ HideActiveSidebar();
+
+ content::WebContents* web_contents = view_delegate_->GetCurrentWebContents();
+ sidebar_container_.reset(
+ new extensions::SidebarContainer(browser_, web_contents, popup_url));
+
+ if (toolbar_actions_bar_)
+ toolbar_actions_bar_->SetSidebarOwner(this);
+
+ PressButtonWithSlideOutIfEnabled(
Devlin 2015/07/30 17:56:46 Wait, what? This calls into PressButtonWithSlideO
ltilve 2015/08/07 16:15:02 Fixed. You were totally right, good catch.
+ base::Bind(&ExtensionActionViewController::PressButton,
+ weak_factory_.GetWeakPtr(), true));
return true;
}
@@ -354,17 +368,67 @@ void ExtensionActionViewController::ShowPopup(
return;
platform_delegate_->ShowPopup(
popup_host.Pass(), grant_tab_permissions, show_action);
- view_delegate_->OnPopupShown(grant_tab_permissions);
+ PressButton(grant_tab_permissions);
}
void ExtensionActionViewController::OnPopupClosed() {
- popup_host_observer_.Remove(popup_host_);
- popup_host_ = nullptr;
+ if (popup_host_) {
+ popup_host_observer_.Remove(popup_host_);
+ popup_host_ = nullptr;
+ }
+ toolbar_actions_bar_->SetPopupOwner(nullptr);
+ RaiseButton();
+}
+
+void ExtensionActionViewController::HideActiveSidebar() {
+ if (toolbar_actions_bar_) {
+ toolbar_actions_bar_->HideActiveSidebar();
+ } else {
+ DCHECK_EQ(ActionInfo::TYPE_PAGE, extension_action_->action_type());
+ // In the traditional toolbar, page actions only know how to close their own
Devlin 2015/07/30 17:56:46 Obviously from copy paste (another reason not to d
ltilve 2015/08/07 16:15:02 Done. Because the sidebar extension is only instan
+ // sidebar.
+ HideSidebar();
+ }
+}
+
+void ExtensionActionViewController::HideSidebar() {
+ if (!sidebar_container_)
+ return;
+
+ sidebar_container_.reset();
+ if (toolbar_actions_bar_)
+ toolbar_actions_bar_->SetSidebarOwner(nullptr);
+
+ RaiseButton();
+}
+
+void ExtensionActionViewController::RaiseButton() {
+ // Reset button state
if (toolbar_actions_bar_) {
- toolbar_actions_bar_->SetPopupOwner(nullptr);
if (toolbar_actions_bar_->popped_out_action() == this &&
- !view_delegate_->IsMenuRunning())
+ !view_delegate_->IsMenuRunning()) {
toolbar_actions_bar_->UndoPopOut();
+ }
}
view_delegate_->OnPopupClosed();
}
+
+void ExtensionActionViewController::PressButtonWithSlideOutIfEnabled(
+ const base::Closure& closure) {
+ if (!toolbar_actions_bar_ || toolbar_actions_bar_->IsActionVisible(this) ||
+ !extensions::FeatureSwitch::extension_action_redesign()->IsEnabled()) {
+ closure.Run();
+ return;
+ }
+
+ // We disabled PopOutAction for sidebar
+ if (extension_action_->open_in_sidebar())
+ return;
+
+ platform_delegate_->CloseOverflowMenu();
+ toolbar_actions_bar_->PopOutAction(this, closure);
+}
+
+void ExtensionActionViewController::PressButton(bool grant_tab_permissions) {
+ view_delegate_->OnPopupShown(grant_tab_permissions);
+}

Powered by Google App Engine
This is Rietveld 408576698