Don't add as a separate shortcut when bulitin provide related item

fix brave/brave-browser#21124

When bookmarks manager is loaded in active tab, user can add it
as a shortcut in sidebar. However, sidebar provides bookmarks panel
as a default items. So, we should add bookmarks builtin item
instead of shortcut if it's removed.
This can apply also to all other builtin items.(talk, wallet).

browser/ui/sidebar/BUILD.gn

@@ -89,6 +89,7 @@ source_set("unit_tests") {
       "//base",
       "//brave/browser/ui",
       "//brave/browser/ui/sidebar",
+      "//brave/common",
       "//brave/components/sidebar",
       "//chrome/test:test_support",
       "//components/prefs",

browser/ui/sidebar/sidebar_unittest.cc

@@ -9,12 +9,17 @@
 #include "base/test/scoped_feature_list.h"
 #include "brave/browser/ui/sidebar/sidebar_model.h"
 #include "brave/browser/ui/sidebar/sidebar_service_factory.h"
+#include "brave/browser/ui/sidebar/sidebar_utils.h"
+#include "brave/common/webui_url_constants.h"
+#include "brave/components/sidebar/constants.h"
 #include "brave/components/sidebar/sidebar_service.h"
+#include "chrome/common/url_constants.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/prefs/pref_service.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 namespace sidebar {
 
@@ -118,4 +123,32 @@ TEST_F(SidebarModelTest, ItemsChangedTest) {
   EXPECT_EQ(3, model()->active_index());
 }
 
+TEST_F(SidebarModelTest, CanUseNotAddedBuiltInItemInsteadOfTest) {
+  GURL talk("https://talk.brave.com/1Ar1vHfLBWX2sAdi");
+  // False because builtin talk item is already added.
+  EXPECT_FALSE(CanUseNotAddedBuiltInItemInsteadOf(service(), talk));
+
+  // Remove builtin talk item and check builtin talk item will be used
+  // instead of adding |talk| url.
+  service()->RemoveItemAt(0);
+  EXPECT_TRUE(CanUseNotAddedBuiltInItemInsteadOf(service(), talk));
+}
+
+TEST(SidebarUtilTest, ConvertURLToBuiltInItemURLTest) {
+  EXPECT_EQ(GURL(kBraveTalkURL),
+            ConvertURLToBuiltInItemURL(GURL("https://talk.brave.com")));
+  EXPECT_EQ(GURL(kBraveTalkURL),
+            ConvertURLToBuiltInItemURL(
+                GURL("https://talk.brave.com/1Ar1vHfLBWX2sAdi")));
+  EXPECT_EQ(GURL(kSidebarBookmarksURL),
+            ConvertURLToBuiltInItemURL(GURL(chrome::kChromeUIBookmarksURL)));
+  EXPECT_EQ(
+      GURL(kBraveUIWalletPageURL),
+      ConvertURLToBuiltInItemURL(GURL("chrome://wallet/crypto/onboarding")));
+
+  // Not converted for url that doesn't relavant builtin item.
+  GURL brave_com("https://www.brave.com/");
+  EXPECT_EQ(brave_com, ConvertURLToBuiltInItemURL(brave_com));
+}
+
 }  // namespace sidebar

browser/ui/sidebar/sidebar_utils.cc

@@ -6,15 +6,19 @@
 #include "brave/browser/ui/sidebar/sidebar_utils.h"
 
 #include "brave/browser/ui/sidebar/sidebar_service_factory.h"
+#include "brave/common/webui_url_constants.h"
+#include "brave/components/sidebar/constants.h"
 #include "brave/components/sidebar/sidebar_item.h"
 #include "brave/components/sidebar/sidebar_service.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.h"
 #include "chrome/browser/ui/webui/ntp/new_tab_ui.h"
+#include "chrome/common/webui_url_constants.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/url_constants.h"
 
 namespace sidebar {
 
@@ -24,24 +28,22 @@ SidebarService* GetSidebarService(Browser* browser) {
   return SidebarServiceFactory::GetForProfile(browser->profile());
 }
 
-bool IsActiveTabNTP(Browser* browser) {
-  auto* web_contents = browser->tab_strip_model()->GetActiveWebContents();
+bool IsActiveTabNTP(content::WebContents* active_web_contents) {
   content::NavigationEntry* entry =
-      web_contents->GetController().GetLastCommittedEntry();
+      active_web_contents->GetController().GetLastCommittedEntry();
   if (!entry)
-    entry = web_contents->GetController().GetVisibleEntry();
+    entry = active_web_contents->GetController().GetVisibleEntry();
   if (!entry)
     return false;
   const GURL url = entry->GetURL();
   return NewTabUI::IsNewTab(url) || NewTabPageUI::IsNewTabPageOrigin(url) ||
-         search::NavEntryIsInstantNTP(web_contents, entry);
+         search::NavEntryIsInstantNTP(active_web_contents, entry);
 }
 
-bool IsActiveTabAlreadyAddedToSidebar(Browser* browser) {
-  auto* web_contents = browser->tab_strip_model()->GetActiveWebContents();
-  const GURL url = web_contents->GetVisibleURL();
-  for (const auto& item : GetSidebarService(browser)->items()) {
-    if (item.url == url)
+bool IsURLAlreadyAddedToSidebar(SidebarService* service, const GURL& url) {
+  const GURL converted_url = ConvertURLToBuiltInItemURL(url);
+  for (const auto& item : service->items()) {
+    if (item.url == converted_url)
       return true;
   }
 
@@ -50,20 +52,59 @@ bool IsActiveTabAlreadyAddedToSidebar(Browser* browser) {
 
 }  // namespace
 
+bool CanUseNotAddedBuiltInItemInsteadOf(SidebarService* service,
+                                        const GURL& url) {
+  const auto not_added_default_items =
+      service->GetNotAddedDefaultSidebarItems();
+  if (not_added_default_items.empty())
+    return false;
+  const GURL converted_url = ConvertURLToBuiltInItemURL(url);
+  for (const auto& item : not_added_default_items) {
+    if (item.url == converted_url)
+      return true;
+  }
+  return false;
+}
+
 bool CanUseSidebar(Browser* browser) {
   DCHECK(browser);
   return browser->is_type_normal();
 }
 
+// If url is relavant with bulitin items, use builtin item's url.
+// Ex, we don't need to add bookmarks manager as a sidebar shortcut
+// if sidebar panel already has bookmarks item.
+GURL ConvertURLToBuiltInItemURL(const GURL& url) {
+  if (url == GURL(chrome::kChromeUIBookmarksURL))
+    return GURL(kSidebarBookmarksURL);
+
+  if (url.host() == "talk.brave.com")
+    return GURL(kBraveTalkURL);
+
+  if (url.SchemeIs(content::kChromeUIScheme) && url.host() == kWalletPageHost) {
+    return GURL(kBraveUIWalletPageURL);
+  }
+  return url;
+}
+
 bool CanAddCurrentActiveTabToSidebar(Browser* browser) {
-  auto* web_contents = browser->tab_strip_model()->GetActiveWebContents();
-  if (!web_contents)
+  auto* active_web_contents =
+      browser->tab_strip_model()->GetActiveWebContents();
+  if (!active_web_contents)
+    return false;
+
+  if (IsActiveTabNTP(active_web_contents))
+    return false;
+
+  const GURL url = active_web_contents->GetLastCommittedURL();
+  if (!url.is_valid())
     return false;
 
-  if (IsActiveTabNTP(browser))
+  auto* service = GetSidebarService(browser);
+  if (IsURLAlreadyAddedToSidebar(service, url))
     return false;
 
-  if (IsActiveTabAlreadyAddedToSidebar(browser))
+  if (CanUseNotAddedBuiltInItemInsteadOf(service, url))
     return false;
 
   return true;

browser/ui/sidebar/sidebar_utils.h

@@ -7,12 +7,20 @@
 #define BRAVE_BROWSER_UI_SIDEBAR_SIDEBAR_UTILS_H_
 
 class Browser;
+class GURL;
 
 namespace sidebar {
 
+class SidebarService;
+
 bool CanUseSidebar(Browser* browser);
 bool CanAddCurrentActiveTabToSidebar(Browser* browser);
 
+// Exported for testing.
+bool CanUseNotAddedBuiltInItemInsteadOf(SidebarService* service,
+                                        const GURL& url);
+GURL ConvertURLToBuiltInItemURL(const GURL& url);
+
 }  // namespace sidebar
 
 #endif  // BRAVE_BROWSER_UI_SIDEBAR_SIDEBAR_UTILS_H_

components/sidebar/constants.h

@@ -19,6 +19,8 @@ constexpr char kSidebarItemOpenInPanelKey[] = "open_in_panel";
 constexpr char kSidebarBookmarksURL[] =
     "chrome://sidebar-bookmarks.top-chrome/";
 
+constexpr char kBraveTalkURL[] = "https://talk.brave.com/widget";
+
 }  // namespace sidebar
 
 #endif  // BRAVE_COMPONENTS_SIDEBAR_CONSTANTS_H_

components/sidebar/sidebar_service.cc

@@ -27,7 +27,7 @@ SidebarItem GetBuiltInItemForType(SidebarItem::BuiltInItemType type) {
   switch (type) {
     case SidebarItem::BuiltInItemType::kBraveTalk:
       return SidebarItem::Create(
-          GURL("https://talk.brave.com/widget"),
+          GURL(kBraveTalkURL),
           l10n_util::GetStringUTF16(IDS_SIDEBAR_BRAVE_TALK_ITEM_TITLE),
           SidebarItem::Type::kTypeBuiltIn,
           SidebarItem::BuiltInItemType::kBraveTalk, false);

components/sidebar/sidebar_service_unittest.cc

@@ -171,7 +171,7 @@ TEST_F(SidebarServiceTest, BuiltInItemUpdateTestWithBuiltInItemTypeKey) {
 
   // Check service has updated built-in item. Previously url was deprecated.xxx.
   EXPECT_EQ(1UL, service_->items().size());
-  EXPECT_EQ("https://talk.brave.com/widget", service_->items()[0].url);
+  EXPECT_EQ(GURL(kBraveTalkURL), service_->items()[0].url);
 
   // Simulate re-launch and check service has still updated items.
   service_->RemoveObserver(this);
@@ -181,7 +181,7 @@ TEST_F(SidebarServiceTest, BuiltInItemUpdateTestWithBuiltInItemTypeKey) {
 
   // Check service has updated built-in item. Previously url was deprecated.xxx.
   EXPECT_EQ(1UL, service_->items().size());
-  EXPECT_EQ("https://talk.brave.com/widget", service_->items()[0].url);
+  EXPECT_EQ(GURL(kBraveTalkURL), service_->items()[0].url);
 }
 
 TEST_F(SidebarServiceTest, BuiltInItemDoesntHaveHistoryItem) {