[Sharesheet] Refactor SharesheetServiceDelegate.

This CL refactors sharesheet_service_delegate to be identified
by its native_window. This is so that we can ensure only one
sharesheet for any one native_window at any time.

Bug: 1142706
Change-Id: I7892bb5b95f6ab4fa17c772e173084b11e3ab69f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2563010Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Melissa Zhang <melzhang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833098}

chrome/browser/sharesheet/sharesheet_controller.h

@@ -15,10 +15,6 @@ class SharesheetController {
  public:
   virtual ~SharesheetController() = default;
 
-  // Each Controller is assigned a unique id used to distinuish between
-  // different invocations of the sharesheet.
-  virtual uint32_t GetId() = 0;
-
   virtual Profile* GetProfile() = 0;
 
   // When called will set the bubble size to |width| and |height|.

chrome/browser/sharesheet/sharesheet_service.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/sharesheet/sharesheet_types.h"
 #include "chrome/common/chrome_features.h"
 #include "components/services/app_service/public/cpp/intent_util.h"
+#include "content/public/browser/web_contents.h"
 #include "ui/display/types/display_constants.h"
 #include "ui/views/view.h"
 
@@ -45,20 +46,18 @@ void SharesheetService::ShowBubble(content::WebContents* web_contents,
                                    sharesheet::CloseCallback close_callback) {
   DCHECK(intent->action == apps_util::kIntentActionSend ||
          intent->action == apps_util::kIntentActionSendMultiple);
-  auto sharesheet_service_delegate =
-      std::make_unique<SharesheetServiceDelegate>(delegate_counter_++,
-                                                  web_contents, this);
-  ShowBubbleWithDelegate(std::move(sharesheet_service_delegate),
-                         std::move(intent), contains_hosted_document,
-                         std::move(close_callback));
+  auto* sharesheet_service_delegate =
+      GetOrCreateDelegate(web_contents->GetTopLevelNativeWindow());
+  ShowBubbleWithDelegate(sharesheet_service_delegate, std::move(intent),
+                         contains_hosted_document, std::move(close_callback));
 }
 
 // Cleanup delegate when bubble closes.
-void SharesheetService::OnBubbleClosed(uint32_t id,
+void SharesheetService::OnBubbleClosed(gfx::NativeWindow native_window,
                                        const base::string16& active_action) {
   auto iter = active_delegates_.begin();
   while (iter != active_delegates_.end()) {
-    if ((*iter)->GetId() == id) {
+    if ((*iter)->GetNativeWindow() == native_window) {
       if (!active_action.empty()) {
         ShareAction* share_action =
             sharesheet_action_cache_->GetActionFromName(active_action);
@@ -72,12 +71,12 @@ void SharesheetService::OnBubbleClosed(uint32_t id,
   }
 }
 
-void SharesheetService::OnTargetSelected(uint32_t delegate_id,
+void SharesheetService::OnTargetSelected(gfx::NativeWindow native_window,
                                          const base::string16& target_name,
                                          const TargetType type,
                                          apps::mojom::IntentPtr intent,
                                          views::View* share_action_view) {
-  SharesheetServiceDelegate* delegate = GetDelegate(delegate_id);
+  SharesheetServiceDelegate* delegate = GetDelegate(native_window);
   if (delegate == nullptr)
     return;
 
@@ -106,11 +105,23 @@ void SharesheetService::OnTargetSelected(uint32_t delegate_id,
   }
 }
 
+SharesheetServiceDelegate* SharesheetService::GetOrCreateDelegate(
+    gfx::NativeWindow native_window) {
+  auto* delegate = GetDelegate(native_window);
+  if (delegate == nullptr) {
+    auto new_delegate =
+        std::make_unique<SharesheetServiceDelegate>(native_window, this);
+    delegate = new_delegate.get();
+    active_delegates_.push_back(std::move(new_delegate));
+  }
+  return delegate;
+}
+
 SharesheetServiceDelegate* SharesheetService::GetDelegate(
-    uint32_t delegate_id) {
+    gfx::NativeWindow native_window) {
   auto iter = active_delegates_.begin();
   while (iter != active_delegates_.end()) {
-    if ((*iter)->GetId() == delegate_id) {
+    if ((*iter)->GetNativeWindow() == native_window) {
       return iter->get();
     }
     ++iter;
@@ -179,17 +190,16 @@ void SharesheetService::OnIconLoaded(
 }
 
 void SharesheetService::OnAppIconsLoaded(
-    std::unique_ptr<SharesheetServiceDelegate> delegate,
+    SharesheetServiceDelegate* delegate,
     apps::mojom::IntentPtr intent,
     sharesheet::CloseCallback close_callback,
     std::vector<TargetInfo> targets) {
   delegate->ShowBubble(std::move(targets), std::move(intent),
                        std::move(close_callback));
-  active_delegates_.push_back(std::move(delegate));
 }
 
 void SharesheetService::ShowBubbleWithDelegate(
-    std::unique_ptr<SharesheetServiceDelegate> delegate,
+    SharesheetServiceDelegate* delegate,
     apps::mojom::IntentPtr intent,
     bool contains_hosted_document,
     sharesheet::CloseCallback close_callback) {
@@ -212,7 +222,7 @@ void SharesheetService::ShowBubbleWithDelegate(
       intent_launch_info.size());
   LoadAppIcons(std::move(intent_launch_info), std::move(targets), 0,
                base::BindOnce(&SharesheetService::OnAppIconsLoaded,
-                              weak_factory_.GetWeakPtr(), std::move(delegate),
+                              weak_factory_.GetWeakPtr(), delegate,
                               std::move(intent), std::move(close_callback)));
 }
 

chrome/browser/sharesheet/sharesheet_service.h

@@ -15,6 +15,7 @@
 #include "chrome/browser/sharesheet/sharesheet_types.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/services/app_service/public/mojom/types.mojom.h"
+#include "ui/gfx/native_widget_types.h"
 
 class Profile;
 
@@ -60,13 +61,16 @@ class SharesheetService : public KeyedService {
                   apps::mojom::IntentPtr intent,
                   bool contains_hosted_document,
                   sharesheet::CloseCallback close_callback);
-  void OnBubbleClosed(uint32_t id, const base::string16& active_action);
-  void OnTargetSelected(uint32_t delegate_id,
+  void OnBubbleClosed(gfx::NativeWindow native_window,
+                      const base::string16& active_action);
+  void OnTargetSelected(gfx::NativeWindow native_window,
                         const base::string16& target_name,
                         const TargetType type,
                         apps::mojom::IntentPtr intent,
                         views::View* share_action_view);
-  SharesheetServiceDelegate* GetDelegate(uint32_t delegate_id);
+  SharesheetServiceDelegate* GetOrCreateDelegate(
+      gfx::NativeWindow native_window);
+  SharesheetServiceDelegate* GetDelegate(gfx::NativeWindow native_window);
 
   // If the files to share contains a Google Drive hosted document, only the
   // drive share action will be shown.
@@ -90,18 +94,16 @@ class SharesheetService : public KeyedService {
                     SharesheetServiceIconLoaderCallback callback,
                     apps::mojom::IconValuePtr icon_value);
 
-  void OnAppIconsLoaded(std::unique_ptr<SharesheetServiceDelegate> delegate,
+  void OnAppIconsLoaded(SharesheetServiceDelegate* delegate,
                         apps::mojom::IntentPtr intent,
                         sharesheet::CloseCallback close_callback,
                         std::vector<TargetInfo> targets);
 
-  void ShowBubbleWithDelegate(
-      std::unique_ptr<SharesheetServiceDelegate> delegate,
-      apps::mojom::IntentPtr intent,
-      bool contains_hosted_document,
-      sharesheet::CloseCallback close_callback);
+  void ShowBubbleWithDelegate(SharesheetServiceDelegate* delegate,
+                              apps::mojom::IntentPtr intent,
+                              bool contains_hosted_document,
+                              sharesheet::CloseCallback close_callback);
 
-  uint32_t delegate_counter_ = 0;
   Profile* profile_;
   std::unique_ptr<SharesheetActionCache> sharesheet_action_cache_;
   apps::AppServiceProxy* app_service_proxy_;

chrome/browser/sharesheet/sharesheet_service_delegate.cc

@@ -11,18 +11,16 @@
 #include "chrome/browser/sharesheet/sharesheet_service.h"
 #include "chrome/browser/sharesheet/sharesheet_service_factory.h"
 #include "chrome/browser/ui/views/sharesheet/sharesheet_bubble_view.h"
-#include "content/public/browser/web_contents.h"
 #include "ui/views/view.h"
 
 namespace sharesheet {
 
 SharesheetServiceDelegate::SharesheetServiceDelegate(
-    uint32_t id,
-    content::WebContents* web_contents,
+    gfx::NativeWindow native_window,
     SharesheetService* sharesheet_service)
-    : id_(id),
+    : native_window_(native_window),
       sharesheet_bubble_view_(
-          std::make_unique<SharesheetBubbleView>(web_contents, this)),
+          std::make_unique<SharesheetBubbleView>(native_window, this)),
       sharesheet_service_(sharesheet_service) {}
 
 SharesheetServiceDelegate::~SharesheetServiceDelegate() = default;
@@ -31,14 +29,18 @@ void SharesheetServiceDelegate::ShowBubble(
     std::vector<TargetInfo> targets,
     apps::mojom::IntentPtr intent,
     sharesheet::CloseCallback close_callback) {
+  if (is_bubble_open_)
+    return;
   sharesheet_bubble_view_->ShowBubble(std::move(targets), std::move(intent),
                                       std::move(close_callback));
+  is_bubble_open_ = true;
 }
 
 void SharesheetServiceDelegate::OnBubbleClosed(
     const base::string16& active_action) {
   sharesheet_bubble_view_.release();
-  sharesheet_service_->OnBubbleClosed(id_, active_action);
+  sharesheet_service_->OnBubbleClosed(native_window_, active_action);
+  is_bubble_open_ = false;
 }
 
 void SharesheetServiceDelegate::OnActionLaunched() {
@@ -50,12 +52,12 @@ void SharesheetServiceDelegate::OnTargetSelected(
     const TargetType type,
     apps::mojom::IntentPtr intent,
     views::View* share_action_view) {
-  sharesheet_service_->OnTargetSelected(id_, target_name, type,
+  sharesheet_service_->OnTargetSelected(native_window_, target_name, type,
                                         std::move(intent), share_action_view);
 }
 
-uint32_t SharesheetServiceDelegate::GetId() {
-  return id_;
+gfx::NativeWindow SharesheetServiceDelegate::GetNativeWindow() {
+  return native_window_;
 }
 
 Profile* SharesheetServiceDelegate::GetProfile() {

chrome/browser/sharesheet/sharesheet_service_delegate.h

@@ -12,6 +12,7 @@
 #include "chrome/browser/sharesheet/sharesheet_controller.h"
 #include "chrome/browser/sharesheet/sharesheet_types.h"
 #include "components/services/app_service/public/mojom/types.mojom.h"
+#include "ui/gfx/native_widget_types.h"
 
 class Profile;
 class SharesheetBubbleView;
@@ -20,10 +21,6 @@ namespace views {
 class View;
 }
 
-namespace content {
-class WebContents;
-}
-
 namespace gfx {
 struct VectorIcon;
 }
@@ -36,8 +33,7 @@ class SharesheetService;
 // business logic in the sharesheet.
 class SharesheetServiceDelegate : public SharesheetController {
  public:
-  SharesheetServiceDelegate(uint32_t id,
-                            content::WebContents* web_contents,
+  SharesheetServiceDelegate(gfx::NativeWindow native_window,
                             SharesheetService* sharesheet_service);
   ~SharesheetServiceDelegate() override;
   SharesheetServiceDelegate(const SharesheetServiceDelegate&) = delete;
@@ -54,15 +50,20 @@ class SharesheetServiceDelegate : public SharesheetController {
                         views::View* share_action_view);
   void OnActionLaunched();
   const gfx::VectorIcon* GetVectorIcon(const base::string16& display_name);
+  gfx::NativeWindow GetNativeWindow();
 
   // SharesheetController overrides
-  uint32_t GetId() override;
   Profile* GetProfile() override;
   void SetSharesheetSize(const int& width, const int& height) override;
   void CloseSharesheet() override;
 
  private:
-  const uint32_t id_;
+  bool is_bubble_open_ = false;
+
+  // Only used for ID purposes. NativeWindow will always outlive the
+  // SharesheetServiceDelegate.
+  gfx::NativeWindow native_window_;
+
   base::string16 active_action_;
   std::unique_ptr<SharesheetBubbleView> sharesheet_bubble_view_;
   SharesheetService* sharesheet_service_;

chrome/browser/ui/views/sharesheet/sharesheet_bubble_view.cc

@@ -18,7 +18,6 @@
 #include "chrome/browser/ui/views/sharesheet/sharesheet_target_button.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
-#include "content/public/browser/web_contents.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/browser/app_window/app_window_registry.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ -108,12 +107,12 @@ bool IsKeyboardCodeArrow(ui::KeyboardCode key_code) {
 }  // namespace
 
 SharesheetBubbleView::SharesheetBubbleView(
-    content::WebContents* web_contents,
+    gfx::NativeWindow native_window,
     sharesheet::SharesheetServiceDelegate* delegate)
     : delegate_(delegate) {
-  gfx::NativeWindow parent = web_contents->GetTopLevelNativeWindow();
-  set_parent_window(parent);
-  parent_view_ = views::Widget::GetWidgetForNativeWindow(parent)->GetRootView();
+  set_parent_window(native_window);
+  parent_view_ =
+      views::Widget::GetWidgetForNativeWindow(native_window)->GetRootView();
   UpdateAnchorPosition();
 
   CreateBubble();

chrome/browser/ui/views/sharesheet/sharesheet_bubble_view.h

@@ -9,6 +9,7 @@
 
 #include "chrome/browser/sharesheet/sharesheet_types.h"
 #include "components/services/app_service/public/mojom/types.mojom.h"
+#include "ui/gfx/native_widget_types.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
 
 namespace views {
@@ -20,10 +21,6 @@ namespace sharesheet {
 class SharesheetServiceDelegate;
 }
 
-namespace content {
-class WebContents;
-}
-
 class SharesheetExpandButton;
 
 class SharesheetBubbleView : public views::BubbleDialogDelegateView {
@@ -32,7 +29,7 @@ class SharesheetBubbleView : public views::BubbleDialogDelegateView {
 
   SharesheetBubbleView(views::View* anchor_view,
                        sharesheet::SharesheetServiceDelegate* delegate);
-  SharesheetBubbleView(content::WebContents* web_contents,
+  SharesheetBubbleView(gfx::NativeWindow native_window,
                        sharesheet::SharesheetServiceDelegate* delegate);
   SharesheetBubbleView(const SharesheetBubbleView&) = delete;
   SharesheetBubbleView& operator=(const SharesheetBubbleView&) = delete;