Implement "More contents" on PlaylistActionBubble (#19290)

* Implement "More contents" on PlaylistActionBubble

When we found media items that hasn't been saved to Playlist,
we reveal this menu item and give users a chance to save them.

browser/playlist/playlist_tab_helper.cc

@@ -259,6 +259,25 @@ void PlaylistTabHelper::OnFoundMediaFromContents(
   }
 }
 
+std::vector<mojom::PlaylistItemPtr> PlaylistTabHelper::GetUnsavedItems() const {
+  if (found_items_.empty()) {
+    return {};
+  }
+
+  base::flat_set<std::string> saved_items;
+  base::ranges::transform(saved_items_,
+                          std::inserter(saved_items, saved_items.begin()),
+                          [](const auto& item) { return item->id; });
+
+  std::vector<mojom::PlaylistItemPtr> unsaved_items;
+  for (const auto& found_item : found_items_) {
+    if (!base::Contains(saved_items, found_item->id)) {
+      unsaved_items.push_back(found_item->Clone());
+    }
+  }
+  return unsaved_items;
+}
+
 void PlaylistTabHelper::OnAddedItems(
     std::vector<mojom::PlaylistItemPtr> items) {
   // mojo based observer tends to be notified later. i.e. OnItemCreated() will

browser/playlist/playlist_tab_helper.h

@@ -38,6 +38,8 @@ class PlaylistTabHelper
     return found_items_;
   }
 
+  std::vector<mojom::PlaylistItemPtr> GetUnsavedItems() const;
+
   void AddObserver(PlaylistTabHelperObserver* observer);
   void RemoveObserver(PlaylistTabHelperObserver* observer);
 

browser/ui/views/playlist/playlist_action_bubble_view.cc

@@ -13,6 +13,7 @@
 #include "brave/app/vector_icons/vector_icons.h"
 #include "brave/browser/playlist/playlist_tab_helper.h"
 #include "brave/browser/ui/color/brave_color_id.h"
+#include "brave/browser/ui/views/playlist/playlist_action_icon_view.h"
 #include "brave/browser/ui/views/side_panel/playlist/playlist_side_panel_coordinator.h"
 #include "brave/grit/brave_theme_resources.h"
 #include "chrome/grit/generated_resources.h"
@@ -32,6 +33,19 @@ namespace {
 
 PlaylistActionBubbleView* g_bubble = nullptr;
 
+template <class ActionBubbleView,
+          typename std::enable_if_t<
+              std::is_base_of_v<PlaylistActionBubbleView, ActionBubbleView>>* =
+              nullptr>
+void ShowBubble(std::unique_ptr<ActionBubbleView> bubble) {
+  DCHECK(!g_bubble);
+
+  g_bubble = bubble.release();
+
+  auto* widget = views::BubbleDialogDelegateView::CreateBubble(g_bubble);
+  widget->Show();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // DefaultThumbnailBackground
 //
@@ -65,7 +79,7 @@ class ConfirmBubble : public PlaylistActionBubbleView {
   METADATA_HEADER(ConfirmBubble);
 
   ConfirmBubble(Browser* browser,
-                views::View* anchor,
+                PlaylistActionIconView* anchor,
                 playlist::PlaylistTabHelper* playlist_tab_helper);
   ~ConfirmBubble() override = default;
 
@@ -87,6 +101,57 @@ class ConfirmBubble : public PlaylistActionBubbleView {
   void MoreMediaInContents();
 };
 
+////////////////////////////////////////////////////////////////////////////////
+// AddBubble
+//  * Shows when users try adding items found from the current contents.
+//  * Shows a list of found items and users can select which one to add.
+class AddBubble : public PlaylistActionBubbleView {
+ public:
+  METADATA_HEADER(AddBubble);
+  AddBubble(Browser* browser,
+            PlaylistActionIconView* anchor,
+            playlist::PlaylistTabHelper* playlist_tab_helper);
+  AddBubble(Browser* browser,
+            PlaylistActionIconView* anchor,
+            playlist::PlaylistTabHelper* playlist_tab_helper,
+            const std::vector<playlist::mojom::PlaylistItemPtr>& items);
+
+ private:
+  class ItemRow : public views::Button {
+   public:
+    static constexpr int kWidth = 288;
+
+    ItemRow(playlist::mojom::PlaylistItemPtr item,
+            base::RepeatingCallback<void(const ItemRow&)> callback);
+
+    bool selected() const { return selected_; }
+    const playlist::mojom::PlaylistItemPtr& item() const { return item_; }
+
+    // views::Button:
+    int GetHeightForWidth(int width) const override;
+    void OnThemeChanged() override;
+
+   private:
+    ItemRow& OnPressed(const ui::Event& event);
+
+    void SetSelected(bool selected);
+    void UpdateBackground();
+
+    playlist::mojom::PlaylistItemPtr item_;
+    bool selected_ = true;
+
+    raw_ptr<views::ImageView> selected_icon_ = nullptr;
+  };
+
+  void AddSelected();
+  void OnItemPressed(const ItemRow& row);
+
+  base::flat_set<const ItemRow*> selected_views_;
+};
+
+////////////////////////////////////////////////////////////////////////////////
+// ConfirmBubble Impl
+
 ConfirmBubble::Row::Row(const std::u16string& text,
                         const ui::ImageModel& icon,
                         views::Button::PressedCallback callback)
@@ -106,7 +171,7 @@ void ConfirmBubble::Row::Layout() {
 }
 
 ConfirmBubble::ConfirmBubble(Browser* browser,
-                             views::View* anchor,
+                             PlaylistActionIconView* anchor,
                              playlist::PlaylistTabHelper* playlist_tab_helper)
     : PlaylistActionBubbleView(browser, anchor, playlist_tab_helper) {
   // What this look like
@@ -144,13 +209,16 @@ ConfirmBubble::ConfirmBubble(Browser* browser,
                                      kIconSize),
       base::BindRepeating(&ConfirmBubble::RemoveFromPlaylist,
                           base::Unretained(this))));
-  AddChildView(std::make_unique<views::Separator>());
-  AddChildView(std::make_unique<Row>(
-      l10n_util::GetStringUTF16(IDS_PLAYLIST_MORE_MEDIA_IN_THIS_PAGE),
-      ui::ImageModel::FromVectorIcon(kLeoProductPlaylistIcon,
-                                     ui::kColorMenuIcon, kIconSize),
-      base::BindRepeating(&ConfirmBubble::MoreMediaInContents,
-                          base::Unretained(this))));
+
+  if (playlist_tab_helper->GetUnsavedItems().size()) {
+    AddChildView(std::make_unique<views::Separator>());
+    AddChildView(std::make_unique<Row>(
+        l10n_util::GetStringUTF16(IDS_PLAYLIST_MORE_MEDIA_IN_THIS_PAGE),
+        ui::ImageModel::FromVectorIcon(kLeoProductPlaylistIcon,
+                                       ui::kColorMenuIcon, kIconSize),
+        base::BindRepeating(&ConfirmBubble::MoreMediaInContents,
+                            base::Unretained(this))));
+  }
 }
 
 void ConfirmBubble::OpenInPlaylist() {
@@ -190,56 +258,41 @@ void ConfirmBubble::RemoveFromPlaylist() {
 }
 
 void ConfirmBubble::MoreMediaInContents() {
-  NOTIMPLEMENTED();
+  auto show_add_bubble = base::BindOnce(
+      [](base::WeakPtr<playlist::PlaylistTabHelper> tab_helper,
+         Browser* browser, base::WeakPtr<PlaylistActionIconView> anchor) {
+        if (!tab_helper || !anchor) {
+          return;
+        }
+
+        if (!tab_helper->found_items().size()) {
+          return;
+        }
+
+        ::ShowBubble(
+            std::make_unique<AddBubble>(browser, anchor.get(), tab_helper.get(),
+                                        tab_helper->GetUnsavedItems()));
+      },
+      playlist_tab_helper_->GetWeakPtr(),
+      // |Browser| outlives TabHelper so it's okay to bind raw ptr here
+      browser_.get(), icon_view_->GetWeakPtr());
+
+  SetCloseCallback(
+      // WindowClosingImpl should be called first to clean up data before
+      // showing up new bubble. This callback is called by itself, it's okay to
+      // pass Unretained().
+      base::BindOnce(&PlaylistActionBubbleView::WindowClosingImpl,
+                     base::Unretained(this))
+          .Then(std::move(show_add_bubble)));
+
+  GetWidget()->Close();
 }
 
 BEGIN_METADATA(ConfirmBubble, PlaylistActionBubbleView)
 END_METADATA
 
 ////////////////////////////////////////////////////////////////////////////////
-// AddBubble
-//  * Shows when users try adding items found from the current contents.
-//  * Shows a list of found items and users can select which one to add.
-class AddBubble : public PlaylistActionBubbleView {
- public:
-  METADATA_HEADER(AddBubble);
-  AddBubble(Browser* browser,
-            views::View* anchor,
-            playlist::PlaylistTabHelper* playlist_tab_helper);
-
- private:
-  class ItemRow : public views::Button {
-   public:
-    static constexpr int kWidth = 288;
-
-    ItemRow(playlist::mojom::PlaylistItemPtr item,
-            base::RepeatingCallback<void(const ItemRow&)> callback);
-
-    bool selected() const { return selected_; }
-    const playlist::mojom::PlaylistItemPtr& item() const { return item_; }
-
-    // views::Button:
-    int GetHeightForWidth(int width) const override;
-    void OnThemeChanged() override;
-
-   private:
-    ItemRow& OnPressed(const ui::Event& event);
-
-    void SetSelected(bool selected);
-    void UpdateBackground();
-
-    playlist::mojom::PlaylistItemPtr item_;
-    bool selected_ = true;
-
-    raw_ptr<views::ImageView> selected_icon_ = nullptr;
-  };
-
-  void AddSelected();
-  void OnItemPressed(const ItemRow& row);
-
-  base::flat_set<const ItemRow*> selected_views_;
-};
-
+// AddBubble Impl
 AddBubble::ItemRow::ItemRow(
     playlist::mojom::PlaylistItemPtr item,
     base::RepeatingCallback<void(const ItemRow&)> callback)
@@ -316,8 +369,17 @@ void AddBubble::ItemRow::UpdateBackground() {
 }
 
 AddBubble::AddBubble(Browser* browser,
-                     views::View* anchor,
+                     PlaylistActionIconView* anchor,
                      playlist::PlaylistTabHelper* playlist_tab_helper)
+    : AddBubble(browser,
+                anchor,
+                playlist_tab_helper,
+                playlist_tab_helper->found_items()) {}
+
+AddBubble::AddBubble(Browser* browser,
+                     PlaylistActionIconView* anchor,
+                     playlist::PlaylistTabHelper* playlist_tab_helper,
+                     const std::vector<playlist::mojom::PlaylistItemPtr>& items)
     : PlaylistActionBubbleView(browser, anchor, playlist_tab_helper) {
   // What this look like
   // https://user-images.githubusercontent.com/5474642/243532255-f82fc740-eea0-4c52-b43a-378ab703d229.png
@@ -344,7 +406,7 @@ AddBubble::AddBubble(Browser* browser,
       scroll_view->SetContents(std::make_unique<views::BoxLayoutView>());
   contents->SetOrientation(views::BoxLayout::Orientation::kVertical);
 
-  for (const auto& item : playlist_tab_helper->found_items()) {
+  for (const auto& item : items) {
     selected_views_.insert(contents->AddChildView(std::make_unique<ItemRow>(
         item.Clone(), base::BindRepeating(&AddBubble::OnItemPressed,
                                           base::Unretained(this)))));
@@ -357,8 +419,12 @@ AddBubble::AddBubble(Browser* browser,
 
   SetButtonLabel(ui::DialogButton::DIALOG_BUTTON_OK,
                  l10n_util::GetStringUTF16(IDS_PLAYLIST_ADD_SELECTED));
-  SetAcceptCallback(
-      base::BindRepeating(&AddBubble::AddSelected, base::Unretained(this)));
+
+  // This callback is called by itself, it's okay to pass Unretained(this).
+  SetAcceptCallback(base::BindOnce(&PlaylistActionBubbleView::WindowClosingImpl,
+                                   base::Unretained(this))
+                        .Then(base::BindOnce(&AddBubble::AddSelected,
+                                             base::Unretained(this))));
 }
 
 void AddBubble::AddSelected() {
@@ -403,20 +469,17 @@ END_METADATA
 // static
 void PlaylistActionBubbleView::ShowBubble(
     Browser* browser,
-    views::View* anchor,
+    PlaylistActionIconView* anchor,
     playlist::PlaylistTabHelper* playlist_tab_helper) {
-  DCHECK(!g_bubble);
-  DCHECK(playlist_tab_helper);
-
   if (playlist_tab_helper->saved_items().size()) {
-    g_bubble = new ConfirmBubble(browser, anchor, playlist_tab_helper);
+    ::ShowBubble(
+        std::make_unique<ConfirmBubble>(browser, anchor, playlist_tab_helper));
   } else if (playlist_tab_helper->found_items().size()) {
-    g_bubble = new AddBubble(browser, anchor, playlist_tab_helper);
+    ::ShowBubble(
+        std::make_unique<AddBubble>(browser, anchor, playlist_tab_helper));
   } else {
     NOTREACHED() << "Caller should filter this case";
   }
-  auto* widget = views::BubbleDialogDelegateView::CreateBubble(g_bubble);
-  widget->Show();
 }
 
 // static
@@ -437,19 +500,28 @@ PlaylistActionBubbleView* PlaylistActionBubbleView::GetBubble() {
 
 PlaylistActionBubbleView::PlaylistActionBubbleView(
     Browser* browser,
-    views::View* anchor,
+    PlaylistActionIconView* anchor,
     playlist::PlaylistTabHelper* playlist_tab_helper)
     : BubbleDialogDelegateView(anchor, views::BubbleBorder::Arrow::TOP_RIGHT),
       browser_(browser),
-      playlist_tab_helper_(playlist_tab_helper) {}
+      playlist_tab_helper_(playlist_tab_helper),
+      icon_view_(anchor) {}
 
 PlaylistActionBubbleView::~PlaylistActionBubbleView() = default;
 
 void PlaylistActionBubbleView::WindowClosing() {
   BubbleDialogDelegateView::WindowClosing();
 
-  DCHECK_EQ(g_bubble, this);
-  g_bubble = nullptr;
+  WindowClosingImpl();
+}
+
+void PlaylistActionBubbleView::WindowClosingImpl() {
+  // This method could be called multiple times during the closing process in
+  // order to show up a subsequent action bubble. So we should check if
+  // |g_bubble| is already filled up with a new bubble.
+  if (g_bubble == this) {
+    g_bubble = nullptr;
+  }
 }
 
 BEGIN_METADATA(PlaylistActionBubbleView, views::BubbleDialogDelegateView)

browser/ui/views/playlist/playlist_action_bubble_view.h

@@ -7,6 +7,7 @@
 #define BRAVE_BROWSER_UI_VIEWS_PLAYLIST_PLAYLIST_ACTION_BUBBLE_VIEW_H_
 
 class Browser;
+class PlaylistActionIconView;
 
 namespace playlist {
 class PlaylistTabHelper;
@@ -19,7 +20,7 @@ class PlaylistActionBubbleView : public views::BubbleDialogDelegateView {
   METADATA_HEADER(PlaylistActionBubbleView);
 
   static void ShowBubble(Browser* browser,
-                         views::View* anchor,
+                         PlaylistActionIconView* anchor,
                          playlist::PlaylistTabHelper* playlist_tab_helper);
   static bool IsShowingBubble();
   static void CloseBubble();
@@ -27,16 +28,21 @@ class PlaylistActionBubbleView : public views::BubbleDialogDelegateView {
 
   ~PlaylistActionBubbleView() override;
 
+  void WindowClosingImpl();
+
   // views::BubbleDialogDelegateView:
   void WindowClosing() override;
 
  protected:
   PlaylistActionBubbleView(Browser* browser,
-                           views::View* anchor,
+                           PlaylistActionIconView* anchor,
                            playlist::PlaylistTabHelper* playlist_tab_helper);
 
-  raw_ptr<Browser> browser_;
+  raw_ptr<Browser> browser_ = nullptr;
   raw_ptr<playlist::PlaylistTabHelper> playlist_tab_helper_ = nullptr;
+
+  // Our anchor.
+  raw_ptr<PlaylistActionIconView> icon_view_ = nullptr;
 };
 
 #endif  // BRAVE_BROWSER_UI_VIEWS_PLAYLIST_PLAYLIST_ACTION_BUBBLE_VIEW_H_

browser/ui/views/playlist/playlist_action_icon_view.cc

@@ -76,6 +76,10 @@ void PlaylistActionIconView::ShowPlaylistBubble() {
   PlaylistActionBubbleView::ShowBubble(browser_, this, playlist_tab_helper);
 }
 
+base::WeakPtr<PlaylistActionIconView> PlaylistActionIconView::GetWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 const gfx::VectorIcon& PlaylistActionIconView::GetVectorIcon() const {
   return state_ == State::kAdded ? kLeoProductPlaylistAddedIcon
                                  : kLeoProductPlaylistAddIcon;

browser/ui/views/playlist/playlist_action_icon_view.h

@@ -31,6 +31,8 @@ class PlaylistActionIconView : public PageActionIconView,
 
   void ShowPlaylistBubble();
 
+  base::WeakPtr<PlaylistActionIconView> GetWeakPtr();
+
   // PageActionIconView:
   void OnExecuting(ExecuteSource execute_source) override {}
   views::BubbleDialogDelegate* GetBubble() const override;