Implement sidebar support for extension action popups

chrome/browser/ui/extensions/extension_action_view_controller.cc

 // Copyright 2014 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/extensions/extension_action_view_controller.h"
 
 #include "base/logging.h"
+#include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/api/commands/command_service.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_view.h"
 #include "chrome/browser/extensions/extension_view_host.h"
 #include "chrome/browser/extensions/extension_view_host_factory.h"
+#include "chrome/browser/extensions/sidebar_container.h"
+#include "chrome/browser/extensions/sidebar_manager.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/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/tabs/tab_strip_model_observer.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"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_constants.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_operations.h"
 
 using extensions::ActionInfo;
 using extensions::CommandService;
@@ skipping 17 matching lines @@
       popup_host_observer_(this),
       weak_factory_(this) {
   DCHECK(extension_action);
   DCHECK(extension_action->action_type() == ActionInfo::TYPE_PAGE ||
          extension_action->action_type() == ActionInfo::TYPE_BROWSER);
   DCHECK(extension);
 }
 
 ExtensionActionViewController::~ExtensionActionViewController() {
   DCHECK(!is_showing_popup());
+
+  extensions::SidebarManager* sidebar_manager =
+      extensions::SidebarManager::GetFromContext(browser_->profile());
+
+  sidebar_manager->RemoveObserver(this);

[Devlin, 2015/06/19 19:56:10]

ScopedObserver

[ltilve, 2015/06/28 22:44:20]

Done.

+  for (std::set<content::WebContents*>::iterator it =
+           active_in_webcontents_.begin();
+       it != active_in_webcontents_.end(); ++it)
+    sidebar_manager->HideSidebar(*it);
 }
 
 const std::string& ExtensionActionViewController::GetId() const {
   return extension_->id();
 }
 
 void ExtensionActionViewController::SetDelegate(
     ToolbarActionViewDelegate* delegate) {
   DCHECK((delegate == nullptr) ^ (view_delegate_ == nullptr));
   if (delegate) {
@@ skipping 226 matching lines @@
 }
 
 bool ExtensionActionViewController::TriggerPopupWithUrl(
     PopupShowAction show_action,
     const GURL& popup_url,
     bool grant_tab_permissions) {
   if (!ExtensionIsValid())
     return false;
 
   bool already_showing = is_showing_popup();
+  bool use_sidebar = extension_action_->open_in_sidebar();
 
   // 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 (!use_sidebar)

[Devlin, 2015/06/19 19:56:10]

This raises the question of if a popup and a sidebar can be showing at the same
time.  If the answer is yes, then we need to restructure the idea of a popup
owner, since you're setting the popup owner here even if it's a sidebar, but
only closing the current one if it's not.

[ltilve, 2015/06/28 22:44:20]

Done, popups and sidebars should not be showing at the same time. You were
totally right about the problems of ownership, the behaviour should be the same
for any case, this is, closing the former sidebar/popup.

[Devlin, 2015/07/07 21:39:29]

Why don't you think popups and sidebars should be able to both be present?  I
think it's probably okay, since sidebars are designed to be persistent
information added to a page.  I think it would be annoying to have a sidebar I
liked, but then be afraid to activate another popup because it closes that
sidebar... and it would be even weirder for the case of "pinned" sidebars
(though we'll still have to address something similar there when a new sidebar
opens).

[ltilve, 2015/07/09 22:15:50]

Done. I had misunderstood the ownership issue. We are allowing again opening an
extension popup without closing the visible sidebar. About the popup owner, as
sidebarManager owns the sidebars, we don't need to assign it to the
toolbarActionsBar, so we are not setting it any more if it's a sidebar. 

+    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)
     return false;
 
+  if (use_sidebar) {
+    extensions::SidebarManager* sidebar_manager =
+        extensions::SidebarManager::GetFromContext(browser_->profile());
+
+    content::WebContents* web_contents =
+        view_delegate_->GetCurrentWebContents();
+
+    extensions::SidebarContainer* sidebar =
+        sidebar_manager->GetSidebarContainerFor(web_contents);
+    if (sidebar && sidebar->extension_id() == GetId()) {
+      sidebar_manager->HideSidebar(web_contents);
+      return false;
+    }
+
+    active_in_webcontents_.insert(web_contents);
+    sidebar_manager->AddObserver(this);
+
+    sidebar_manager->CreateSidebar(web_contents, popup_url, browser_);
+    return true;

[Devlin, 2015/06/19 19:56:10]

I think this should still handle sliding out the action if it's overflowed, if
we're going to try and have attribution.

[ltilve, 2015/06/28 22:44:20]

Done. After the previous change, the attribution problem that could happen with
overflowed popups and sidebars is apparently fixed.

+  }
+
   scoped_ptr<extensions::ExtensionViewHost> host(
       extensions::ExtensionViewHostFactory::CreatePopupHost(popup_url,
                                                             browser_));
   if (!host)
     return false;
 
   popup_host_ = host.get();
   popup_host_observer_.Add(popup_host_);
   if (toolbar_actions_bar_)
     toolbar_actions_bar_->SetPopupOwner(this);
@@ skipping 33 matching lines @@
   popup_host_observer_.Remove(popup_host_);
   popup_host_ = nullptr;
   if (toolbar_actions_bar_) {
     toolbar_actions_bar_->SetPopupOwner(nullptr);
     if (toolbar_actions_bar_->popped_out_action() == this &&
         !view_delegate_->IsMenuRunning())
       toolbar_actions_bar_->UndoPopOut();
   }
   view_delegate_->OnPopupClosed();
 }
+
+void ExtensionActionViewController::OnPopupShown(bool grant_tab_permissions) {
+  view_delegate_->OnPopupShown(grant_tab_permissions);
+}
+
+void ExtensionActionViewController::OnSidebarHidden(
+    content::WebContents* tab,
+    const std::string& content_id) {
+  if (view_delegate_->GetCurrentWebContents() == tab && content_id == GetId()) {
+    view_delegate_->OnPopupClosed();
+    active_in_webcontents_.erase(
+        active_in_webcontents_.find(view_delegate_->GetCurrentWebContents()));
+    if (active_in_webcontents_.size() == 0) {
+      extensions::SidebarManager::GetFromContext(browser_->profile())
+          ->RemoveObserver(this);
+
+      if (toolbar_actions_bar_) {
+        toolbar_actions_bar_->SetPopupOwner(nullptr);
+        if (toolbar_actions_bar_->popped_out_action() == this &&
+            !view_delegate_->IsMenuRunning())
+          toolbar_actions_bar_->UndoPopOut();
+      }
+    }
+  }
+}
+
+void ExtensionActionViewController::OnSidebarSwitched(
+    content::WebContents* old_tab,
+    const std::string& old_content_id,
+    content::WebContents* new_tab,
+    const std::string& new_content_id) {
+  if (browser_->tab_strip_model()->GetIndexOfWebContents(
+          new_tab ? new_tab : old_tab) == TabStripModel::kNoTab)
+    return;
+
+  if (old_content_id == GetId() && old_content_id != new_content_id) {

[Devlin, 2015/06/19 19:56:10]

Under which circumstances would a sidebar be switched to one with the same id?

[ltilve, 2015/06/28 22:44:20]

Done. That code was refactored into a clearer UpdateButtonState function.

+    if (toolbar_actions_bar_) {
+      toolbar_actions_bar_->SetPopupOwner(nullptr);
+      if (toolbar_actions_bar_->popped_out_action() == this &&
+          !view_delegate_->IsMenuRunning())
+        toolbar_actions_bar_->UndoPopOut();
+    }
+    view_delegate_->OnPopupClosed();
+  }
+
+  if (view_delegate_->GetCurrentWebContents() == new_tab &&
+      new_content_id == GetId() && old_content_id != new_content_id) {
+    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::OnPopupShown,
+                           weak_factory_.GetWeakPtr(), true));
+    } else {
+      // Without the popup corner arrow indicator, marking the browserAction
+      // icon
+      // is necessary for extension attribution
+      view_delegate_->OnPopupShown(true);
+    }
+  }
+
+  if (old_tab == new_tab && old_content_id == GetId())
+    OnSidebarHidden(old_tab, old_content_id);
+}