[B4B] Update default parents for new node
For a recently created profile, BookmarkCurrentTab will always save
new bookmarks to the local Other folder.
This CL allows BookmarkCurrentTab to save to account folders if
available.
Bug: 401522073
Change-Id: Iee57adea58a7e6f181c653dfbca556f83ad64384
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6445850
Reviewed-by: Emily Shack <[email protected]>
Reviewed-by: Ted Choc <[email protected]>
Commit-Queue: Ryan Sultanem <[email protected]>
Reviewed-by: Ryan Sultanem <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1447222}
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 31bd948d..87e31ef8 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -1093,6 +1093,10 @@
inline constexpr char kDefaultSearchProviderChoiceScreenShuffleMilestone[] =
"default_search_provider.choice_screen_shuffle_milestone";
+// Deprecated 04/2025.
+inline constexpr char kAddedBookmarkSincePowerBookmarksLaunch[] =
+ "bookmarks.added_since_power_bookmarks_launch";
+
// Register local state used only for migration (clearing or moving to a new
// key).
void RegisterLocalStatePrefsForMigration(PrefRegistrySimple* registry) {
@@ -1529,6 +1533,9 @@
// Deprecated 04/2025.
registry->RegisterIntegerPref(
kDefaultSearchProviderChoiceScreenShuffleMilestone, 0);
+
+ // Deprecated 04/2025.
+ registry->RegisterBooleanPref(kAddedBookmarkSincePowerBookmarksLaunch, false);
}
} // namespace
@@ -2811,6 +2818,9 @@
// Added 04/2025.
profile_prefs->ClearPref(kDefaultSearchProviderChoiceScreenShuffleMilestone);
+ // Added 04/2025.
+ profile_prefs->ClearPref(kAddedBookmarkSincePowerBookmarksLaunch);
+
// Please don't delete the following line. It is used by PRESUBMIT.py.
// END_MIGRATE_OBSOLETE_PROFILE_PREFS
diff --git a/chrome/browser/ui/browser_commands.cc b/chrome/browser/ui/browser_commands.cc
index bb39e0df..64adc672 100644
--- a/chrome/browser/ui/browser_commands.cc
+++ b/chrome/browser/ui/browser_commands.cc
@@ -1444,15 +1444,6 @@
return;
}
bool was_bookmarked_by_user = bookmarks::IsBookmarkedByUser(model, url);
-#if !BUILDFLAG(IS_ANDROID)
- PrefService* prefs = browser->profile()->GetPrefs();
- if (!prefs->GetBoolean(
- bookmarks::prefs::kAddedBookmarkSincePowerBookmarksLaunch)) {
- bookmarks::AddIfNotBookmarked(model, url, title, model->other_node());
- prefs->SetBoolean(bookmarks::prefs::kAddedBookmarkSincePowerBookmarksLaunch,
- true);
- }
-#endif
bookmarks::AddIfNotBookmarked(model, url, title);
bool is_bookmarked_by_user = bookmarks::IsBookmarkedByUser(model, url);
// Make sure the model actually added a bookmark before showing the star. A
diff --git a/components/bookmarks/browser/bookmark_utils.cc b/components/bookmarks/browser/bookmark_utils.cc
index af0eff8f..e197e85 100644
--- a/components/bookmarks/browser/bookmark_utils.cc
+++ b/components/bookmarks/browser/bookmark_utils.cc
@@ -27,6 +27,7 @@
#include "build/build_config.h"
#include "components/bookmarks/browser/bookmark_client.h"
#include "components/bookmarks/browser/bookmark_model.h"
+#include "components/bookmarks/browser/bookmark_node.h"
#include "components/bookmarks/browser/scoped_group_bookmark_actions.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
#include "components/pref_registry/pref_registry_syncable.h"
@@ -468,8 +469,6 @@
registry->RegisterBooleanPref(
prefs::kShowManagedBookmarksInBookmarkBar, true,
user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
- registry->RegisterBooleanPref(prefs::kAddedBookmarkSincePowerBookmarksLaunch,
- false);
RegisterManagedBookmarksPrefs(registry);
}
@@ -495,8 +494,7 @@
const BookmarkNode* AddIfNotBookmarked(BookmarkModel* model,
const GURL& url,
- const std::u16string& title,
- const BookmarkNode* parent) {
+ const std::u16string& title) {
// Nothing to do, a user bookmark with that url already exists.
if (IsBookmarkedByUser(model, url)) {
return nullptr;
@@ -504,8 +502,7 @@
base::RecordAction(base::UserMetricsAction("BookmarkAdded"));
- const auto* parent_to_use =
- parent ? parent : GetParentForNewNodes(model, url);
+ const BookmarkNode* parent_to_use = GetParentForNewNodes(model, url);
return model->AddNewURL(parent_to_use, parent_to_use->children().size(),
title, url);
}
@@ -563,10 +560,16 @@
return parent;
}
+ // Return the last modified folder if there is no save location suggestion.
std::vector<const BookmarkNode*> nodes =
GetMostRecentlyModifiedUserFolders(model);
- CHECK(!nodes.empty());
- return nodes[0];
+ if (!nodes.empty()) {
+ return nodes[0];
+ }
+
+ // Default save location.
+ return model->account_other_node() ? model->account_other_node()
+ : model->other_node();
}
bool PruneFoldersForDisplay(const BookmarkModel* model,
diff --git a/components/bookmarks/browser/bookmark_utils.h b/components/bookmarks/browser/bookmark_utils.h
index 06105f75..a4ae448 100644
--- a/components/bookmarks/browser/bookmark_utils.h
+++ b/components/bookmarks/browser/bookmark_utils.h
@@ -186,8 +186,7 @@
// If there are no user bookmarks for url, a bookmark is created.
const BookmarkNode* AddIfNotBookmarked(BookmarkModel* model,
const GURL& url,
- const std::u16string& title,
- const BookmarkNode* parent = nullptr);
+ const std::u16string& title);
// Removes all bookmarks for the given `url`.
void RemoveAllBookmarks(BookmarkModel* model,
diff --git a/components/bookmarks/browser/bookmark_utils_unittest.cc b/components/bookmarks/browser/bookmark_utils_unittest.cc
index b5b84d8..d416fe81 100644
--- a/components/bookmarks/browser/bookmark_utils_unittest.cc
+++ b/components/bookmarks/browser/bookmark_utils_unittest.cc
@@ -256,17 +256,24 @@
std::unique_ptr<BookmarkModel> model(
TestBookmarkClient::CreateModelWithClient(std::move(client)));
+#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS)
+ EXPECT_EQ(model->mobile_node(), GetParentForNewNodes(model.get(), GURL()));
+#else
+ EXPECT_EQ(model->bookmark_bar_node(),
+ GetParentForNewNodes(model.get(), GURL()));
+ model->CreateAccountPermanentFolders();
+ EXPECT_EQ(model->account_other_node(),
+ GetParentForNewNodes(model.get(), GURL()));
+#endif
+
const BookmarkNode* folder_to_suggest =
model->AddFolder(model->bookmark_bar_node(), 0, u"Suggested");
const BookmarkNode* folder1 =
model->AddFolder(model->bookmark_bar_node(), 1, u"Folder 1");
-
- ASSERT_EQ(folder1, GetParentForNewNodes(model.get(), GURL()));
+ EXPECT_EQ(folder1, GetParentForNewNodes(model.get(), GURL()));
client_ptr->SetSuggestedSaveLocation(folder_to_suggest);
-
- ASSERT_EQ(folder_to_suggest, GetParentForNewNodes(model.get(), GURL()));
-
+ EXPECT_EQ(folder_to_suggest, GetParentForNewNodes(model.get(), GURL()));
client_ptr = nullptr;
}
diff --git a/components/bookmarks/common/bookmark_pref_names.h b/components/bookmarks/common/bookmark_pref_names.h
index 9f05d11..3a84d45 100644
--- a/components/bookmarks/common/bookmark_pref_names.h
+++ b/components/bookmarks/common/bookmark_pref_names.h
@@ -9,10 +9,6 @@
namespace bookmarks::prefs {
-// Boolean which specifies whether the user has added any new bookmarks
-// following the launch of the power bookmarks feature.
-inline constexpr char kAddedBookmarkSincePowerBookmarksLaunch[] =
- "bookmarks.added_since_power_bookmarks_launch";
// Boolean which specifies the ids of the bookmark nodes that are expanded in
// the bookmark editor.
inline constexpr char kBookmarkEditorExpandedNodes[] =
diff --git a/components/power_bookmarks/core/bookmark_client_base_unittest.cc b/components/power_bookmarks/core/bookmark_client_base_unittest.cc
index 2a8022b7..88a8df0 100644
--- a/components/power_bookmarks/core/bookmark_client_base_unittest.cc
+++ b/components/power_bookmarks/core/bookmark_client_base_unittest.cc
@@ -276,8 +276,8 @@
// Save another bookmark to the suggested folder explicitly, even though the
// system wouldn't normally suggest it.
const GURL normal_bookmark_url1 = GURL("http://example.com/normal_1");
- bookmarks::AddIfNotBookmarked(model(), normal_bookmark_url1, u"bookmark 1",
- suggested_folder);
+ model()->AddNewURL(suggested_folder, suggested_folder->children().size(),
+ u"bookmark 1", normal_bookmark_url1);
node = model()->GetMostRecentlyAddedUserNodeForURL(normal_bookmark_url1);
ASSERT_EQ(node->parent(), suggested_folder);
