Chrome · Bookmarks
CVE-2026-113039
UAF in Bookmarks
Overview
Low
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
target_parentchrome/browser/ui/bookmarks/bookmark_ui_operations_helper.cc |
modified | |
ifchrome/browser/ui/bookmarks/bookmark_ui_operations_helper.cc |
modified | |
forchrome/browser/ui/bookmarks/bookmark_ui_operations_helper.cc |
modified | |
parent_folderchrome/browser/ui/bookmarks/bookmark_ui_operations_helper.cc |
modified | |
BookmarkNodechrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.h |
modified | |
BookmarkBarUIControllerInjectorchrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.h |
modified | |
BookmarkBarUIControllerImplchrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.h |
modified |
Files Changed
chrome/browser/ui/views/bookmarks/bookmark_bar_view_browsertest.ccchrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.ccchrome/browser/ui/views/bookmarks/BUILD.gnchrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl_unittest.ccchrome/browser/ui/bookmarks/bookmark_ui_operations_helper.ccchrome/browser/ui/bookmarks/bookmark_ui_operations_helper.hchrome/browser/ui/bookmarks/bookmark_ui_operations_helper_unittest.ccchrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.h
Patch
From 780e8286a917a64ee44080d9ce15783557402bf4 Mon Sep 17 00:00:00 2001
From: David Yeung <dayeung@chromium.org>
Date: Mon, 28 Sep 2026 10:42:22 -0700
Subject: [PATCH] Resolve bookmark nodes by ID after async paste checks
Clipboard paste checks are asynchronous, meaning a target bookmark node
could be deleted before the check completes. This CL updates two places
to track targets by BookmarkNodeId instead of raw pointers to prevent
dangling pointers or operations on removed folders:
* BookmarkUIOperationsHelper: Store BookmarkNodeId in TargetParent and
resolve the folder dynamically, safely aborting if the folder was
removed.
* BookmarkBarUIControllerImpl: Pass BookmarkNodeId across
CanPasteFromClipboard() and resolve target nodes in
OnPasteCheckComplete(), skipping the context menu if deleted.
Bug: 566069559
Change-Id: If238eb2d01f0f706accb14ce92aada55f1b8d7f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8464438
Reviewed-by: Thomas Lukaszewicz <tluk@chromium.org>
Commit-Queue: David Yeung <dayeung@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1706254}
---
diff --git a/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper.cc b/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper.cc
index 589f740e..675a05c 100644
--- a/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper.cc
@@ -22,6 +22,7 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/bookmarks/bookmark_drag_drop.h"
#include "chrome/browser/ui/bookmarks/bookmark_stats.h"
+#include "chrome/browser/ui/bookmarks/bookmark_utils.h"
#include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/browser/bookmark_node.h"
#include "components/bookmarks/browser/bookmark_node_data.h"
@@ -479,7 +480,7 @@
const internal::BookmarkUIOperationsHelper::TargetParent*
BookmarkUIOperationsHelperNonMergedSurfaces::target_parent() const {
- if (!target_parent_) {
+ if (!target_parent_ || !target_parent_->parent_node()) {
return nullptr;
}
return target_parent_.get();
@@ -508,39 +509,58 @@
BookmarkUIOperationsHelperMergedSurfaces::TargetParent::TargetParent(
BookmarkMergedSurfaceService* merged_surface_service,
- BookmarkParentFolder parent)
- : merged_surface_service_(merged_surface_service), parent_(parent) {}
+ const BookmarkParentFolder& parent)
+ : merged_surface_service_(merged_surface_service),
+ parent_id_(chrome::ToNodeId(parent)) {}
BookmarkUIOperationsHelperMergedSurfaces::TargetParent::~TargetParent() =
default;
-const BookmarkParentFolder&
+std::optional<BookmarkParentFolder>
BookmarkUIOperationsHelperMergedSurfaces::TargetParent::parent_folder() const {
- return parent_;
+ if (std::holds_alternative<int64_t>(parent_id_)) {
+ const BookmarkNode* node = bookmarks::GetBookmarkNodeByID(
+ merged_surface_service_->bookmark_model(),
+ std::get<int64_t>(parent_id_));
+ if (!node || !node->is_folder() || node->is_root()) {
+ return std::nullopt;
+ }
+ return BookmarkParentFolder::FromFolderNode(node);
+ }
+ return chrome::ToFolder(parent_id_,
+ merged_surface_service_->bookmark_model());
}
bool BookmarkUIOperationsHelperMergedSurfaces::TargetParent::IsManaged() const {
- return merged_surface_service_->IsParentFolderManaged(parent_);
+ std::optional<BookmarkParentFolder> folder = parent_folder();
+ return folder.has_value() &&
+ merged_surface_service_->IsParentFolderManaged(*folder);
}
bool BookmarkUIOperationsHelperMergedSurfaces::TargetParent::IsPermanentNode()
const {
- return !parent_.HoldsNonPermanentFolder();
+ std::optional<BookmarkParentFolder> folder = parent_folder();
+ return folder.has_value() && !folder->HoldsNonPermanentFolder();
}
bool BookmarkUIOperationsHelperMergedSurfaces::TargetParent::IsDirectChild(
const bookmarks::BookmarkNode* node) const {
- return parent_.HasDirectChildNode(node);
+ std::optional<BookmarkParentFolder> folder = parent_folder();
+ return folder.has_value() && folder->HasDirectChildNode(node);
}
bookmarks::BookmarkNode::Type
BookmarkUIOperationsHelperMergedSurfaces::TargetParent::GetType() const {
- if (parent_.HoldsNonPermanentFolder()) {
- return parent_.as_non_permanent_folder()->type();
+ std::optional<BookmarkParentFolder> folder = parent_folder();
+ if (!folder.has_value()) {
+ return bookmarks::BookmarkNode::Type::URL;
+ }
+ if (folder->HoldsNonPermanentFolder()) {
+ return folder->as_non_permanent_folder()->type();
}
using PermanentFolderType = BookmarkParentFolder::PermanentFolderType;
- switch (*parent_.as_permanent_folder()) {
+ switch (*folder->as_permanent_folder()) {
case PermanentFolderType::kBookmarkBarNode:
return bookmarks::BookmarkNode::Type::BOOKMARK_BAR;
case PermanentFolderType::kOtherNode:
@@ -558,14 +578,18 @@
const bookmarks::BookmarkNode*
BookmarkUIOperationsHelperMergedSurfaces::TargetParent::GetNodeAtIndex(
size_t index) const {
+ std::optional<BookmarkParentFolder> folder = parent_folder();
+ CHECK(folder.has_value());
CHECK_LE(index, GetChildrenCount());
- return merged_surface_service_->GetNodeAtIndex(parent_, index);
+ return merged_surface_service_->GetNodeAtIndex(*folder, index);
}
size_t
BookmarkUIOperationsHelperMergedSurfaces::TargetParent::GetChildrenCount()
const {
- return merged_surface_service_->GetChildrenCount(parent_);
+ std::optional<BookmarkParentFolder> folder = parent_folder();
+ return folder.has_value() ? merged_surface_service_->GetChildrenCount(*folder)
+ : 0;
}
// BookmarkUIOperationsHelperMergedSurfaces:
@@ -586,14 +610,15 @@
const BookmarkNode*
BookmarkUIOperationsHelperMergedSurfaces::GetDefaultParentForNonMergedSurfaces()
const {
- if (!target_parent_) {
+ std::optional<BookmarkParentFolder> folder = parent_folder();
+ if (!folder.has_value()) {
return nullptr;
}
if (target_parent_->IsManaged()) {
- return merged_surface_service_->GetParentForManagedNode(parent_folder());
+ return merged_surface_service_->GetParentForManagedNode(*folder);
}
- return merged_surface_service_->GetDefaultParentForNewNodes(parent_folder());
+ return merged_surface_service_->GetDefaultParentForNewNodes(*folder);
}
bookmarks::BookmarkModel* BookmarkUIOperationsHelperMergedSurfaces::model() {
@@ -603,12 +628,13 @@
void BookmarkUIOperationsHelperMergedSurfaces::AddNodesAsCopiesOfNodeData(
const bookmarks::BookmarkNodeData& data,
size_t index_to_add_at) {
- if (!target_parent_) {
+ std::optional<BookmarkParentFolder> folder = parent_folder();
+ if (!folder.has_value()) {
return;
}
bookmarks::ScopedGroupBookmarkActions group_drops(model());
- merged_surface_service_->AddNodesAsCopiesOfNodeData(
- data.elements, parent_folder(), index_to_add_at);
+ merged_surface_service_->AddNodesAsCopiesOfNodeData(data.elements, *folder,
+ index_to_add_at);
}
void BookmarkUIOperationsHelperMergedSurfaces::MoveBookmarkNodeData(
@@ -616,7 +642,8 @@
const base::FilePath& profile_path,
size_t index_to_add_at,
BrowserWindowInterface* browser) {
- if (!target_parent_) {
+ std::optional<BookmarkParentFolder> folder = parent_folder();
+ if (!folder.has_value()) {
return;
}
CHECK_GE(data.size(), 1u);
@@ -626,7 +653,7 @@
for (const auto& moved_node : moved_nodes) {
CHECK(!model()->client()->IsNodeManaged(moved_node));
- merged_surface_service_->Move(moved_node, parent_folder(), index_to_add_at,
+ merged_surface_service_->Move(moved_node, *folder, index_to_add_at,
browser);
index_to_add_at++;
}
@@ -634,14 +661,16 @@
const internal::BookmarkUIOperationsHelper::TargetParent*
BookmarkUIOperationsHelperMergedSurfaces::target_parent() const {
- if (!target_parent_) {
+ if (!target_parent_ || !target_parent_->parent_folder().has_value()) {
return nullptr;
}
return target_parent_.get();
}
-const BookmarkParentFolder&
+std::optional<BookmarkParentFolder>
BookmarkUIOperationsHelperMergedSurfaces::parent_folder() const {
- CHECK(target_parent_);
+ if (!target_parent_) {
+ return std::nullopt;
+ }
return target_parent_->parent_folder();
}
diff --git a/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper.h b/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper.h
index 630b3f0f..0078743 100644
--- a/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper.h
+++ b/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper.h
@@ -244,10 +244,10 @@
const BookmarkParentFolder* parent);
TargetParent(BookmarkMergedSurfaceService* merged_surface_service,
- BookmarkParentFolder parent);
+ const BookmarkParentFolder& parent);
~TargetParent() override;
- const BookmarkParentFolder& parent_folder() const;
+ std::optional<BookmarkParentFolder> parent_folder() const;
// internal::BookmarkUIOperationsHelper::TargetParent
bool IsManaged() const override;
@@ -259,10 +259,10 @@
private:
const raw_ptr<BookmarkMergedSurfaceService> merged_surface_service_;
- const BookmarkParentFolder parent_;
+ const bookmarks::BookmarkNodeId parent_id_;
};
- const BookmarkParentFolder& parent_folder() const;
+ std::optional<BookmarkParentFolder> parent_folder() const;
const raw_ptr<BookmarkMergedSurfaceService> merged_surface_service_;
const std::unique_ptr<TargetParent> target_parent_;
diff --git a/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.cc b/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.cc
index 66e71c5..be3c9003 100644
--- a/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.cc
+++ b/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.cc
@@ -159,16 +159,22 @@
model_adapter->CanPasteFromClipboard(
&parent_folder,
base::BindOnce(&BookmarkBarUIControllerImpl::OnPasteCheckComplete,
- weak_ptr_factory_.GetWeakPtr(), std::move(nodes), point,
- source_type, auto_close.Release()));
+ weak_ptr_factory_.GetWeakPtr(), target, point, source_type,
+ auto_close.Release()));
}
void BookmarkBarUIControllerImpl::OnPasteCheckComplete(
- std::vector<const bookmarks::BookmarkNode*> selection,
+ bookmarks::BookmarkNodeId target,
const gfx::Point& point,
ui::mojom::MenuSourceType source_type,
base::OnceClosure on_close,
bool can_paste) {
+ base::ScopedClosureRunner auto_close(std::move(on_close));
+ std::vector<const bookmarks::BookmarkNode*> selection =
+ ResolveTargetNodes(injector_->GetModelAdapter(), target);
+ if (selection.empty()) {
+ return;
+ }
injector_->GetActionAdapter()->ShowContextMenu(
- selection, point, source_type, can_paste, std::move(on_close));
+ selection, point, source_type, can_paste, auto_close.Release());
}
diff --git a/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.h b/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.h
index 8180acc..f11fd8e 100644
--- a/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.h
+++ b/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl.h
@@ -9,10 +9,6 @@
#include "base/memory/weak_ptr.h"
#include "chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller.h"
-namespace bookmarks {
-class BookmarkNode;
-}
-
class BookmarkBarUIControllerInjector;
class BookmarkBarUIControllerImpl : public BookmarkBarUIController {
@@ -40,12 +36,11 @@
void OnAppsPageShortcutVisibilityPrefChanged();
void OnTabGroupsVisibilityPrefChanged();
void OnShowManagedBookmarksPrefChanged();
- void OnPasteCheckComplete(
- std::vector<const bookmarks::BookmarkNode*> selection,
- const gfx::Point& point,
- ui::mojom::MenuSourceType source_type,
- base::OnceClosure on_close,
- bool can_paste);
+ void OnPasteCheckComplete(bookmarks::BookmarkNodeId target,
+ const gfx::Point& point,
+ ui::mojom::MenuSourceType source_type,
+ base::OnceClosure on_close,
+ bool can_paste);
std::unique_ptr<BookmarkBarUIControllerInjector> injector_;
raw_ptr<BookmarkBarUIClient> client_ = nullptr;
diff --git a/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper_unittest.cc b/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper_unittest.cc
index 6009b7e5..2bc7bba 100644
--- a/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper_unittest.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper_unittest.cc
@@ -103,9 +103,10 @@
model_.get(), parent);
} else if (std::is_same<T,
BookmarkUIOperationsHelperMergedSurfaces>::value) {
- parent_folder_ = BookmarkParentFolder::FromFolderNode(parent);
+ BookmarkParentFolder parent_folder =
+ BookmarkParentFolder::FromFolderNode(parent);
helper_ = std::make_unique<BookmarkUIOperationsHelperMergedSurfaces>(
- bookmark_merged_surface_service_.get(), &parent_folder_.value());
+ bookmark_merged_surface_service_.get(), &parent_folder);
}
return helper_.get();
}
@@ -124,7 +125,6 @@
raw_ptr<bookmarks::BookmarkModel> model_ = nullptr;
raw_ptr<BookmarkMergedSurfaceService> bookmark_merged_surface_service_ =
nullptr;
- std::optional<BookmarkParentFolder> parent_folder_;
std::unique_ptr<internal::BookmarkUIOperationsHelper> helper_;
};
@@ -630,6 +630,35 @@
EXPECT_EQ(url, bar_folder->children()[0]->url().spec());
}
+TYPED_TEST(BookmarkUIOperationsHelperTest,
+ PasteFromClipboardAfterParentFolderRemoved) {
+ BookmarkModel* model = this->model();
+ const BookmarkNode* folder =
+ model->AddFolder(model->bookmark_bar_node(), 0, u"Folder");
+ model->AddURL(folder, 0, u"existing", GURL("https://existing.example/"));
+
+ {
+ ui::ScopedClipboardWriter clipboard_writer(ui::ClipboardBuffer::kCopyPaste);
+ clipboard_writer.WriteText(u"https://www.google.com/");
+ }
+
+ internal::BookmarkUIOperationsHelper* helper = this->CreateHelper(folder);
+ EXPECT_TRUE(this->CanPasteFromClipboardSync(helper));
+
+ // Remove the target folder before PasteFromClipboard completes.
+ model->Remove(folder, bookmarks::metrics::BookmarkEditSource::kOther,
+ FROM_HERE);
+
+ EXPECT_FALSE(this->CanPasteFromClipboardSync(helper));
+
+ {
+ base::test::TestFuture<void> future;
+ helper->PasteFromClipboard(1, future.GetCallback());
+ EXPECT_TRUE(future.Wait());
+ }
+ EXPECT_TRUE(model->bookmark_bar_node()->children().empty());
+}
+
#endif // !BUILDFLAG(IS_MAC)
TEST(BookmarkUIOperationsHelperMergedSurfacesTest,
diff --git a/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl_unittest.cc b/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl_unittest.cc
index 109f1ce..0cdbb36 100644
--- a/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl_unittest.cc
+++ b/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl_unittest.cc
@@ -270,7 +270,8 @@
ui::mojom::MenuSourceType source_type = ui::mojom::MenuSourceType::kMouse;
EXPECT_CALL(mock_model_adapter_, GetNodeById(node->id()))
- .WillOnce(testing::Return(node));
+ .Times(2)
+ .WillRepeatedly(testing::Return(node));
EXPECT_CALL(mock_model_adapter_,
CanPasteFromClipboard(testing::_, testing::_))
.WillOnce(base::test::RunOnceCallback<1>(true));
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bar_view_browsertest.cc b/chrome/browser/ui/views/bookmarks/bookmark_bar_view_browsertest.cc
index aa4d068..f5d615d 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_bar_view_browsertest.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_bar_view_browsertest.cc
@@ -4,6 +4,7 @@
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
+#include "base/memory/raw_ptr.h"
#include "base/task/sequenced_task_runner.h"
#include "base/test/run_until.h"
#include "base/test/scoped_feature_list.h"
@@ -28,15 +29,19 @@
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view_test_helper.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_context_menu.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/undo/bookmark_undo_service_factory.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
... (file diff truncated)
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper_unittest.cc b/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper_unittest.cc
index 6009b7e5..2bc7bba 100644
--- a/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper_unittest.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_ui_operations_helper_unittest.cc
@@ -103,9 +103,10 @@
model_.get(), parent);
} else if (std::is_same<T,
BookmarkUIOperationsHelperMergedSurfaces>::value) {
- parent_folder_ = BookmarkParentFolder::FromFolderNode(parent);
+ BookmarkParentFolder parent_folder =
+ BookmarkParentFolder::FromFolderNode(parent);
helper_ = std::make_unique<BookmarkUIOperationsHelperMergedSurfaces>(
- bookmark_merged_surface_service_.get(), &parent_folder_.value());
+ bookmark_merged_surface_service_.get(), &parent_folder);
}
return helper_.get();
}
@@ -124,7 +125,6 @@
raw_ptr<bookmarks::BookmarkModel> model_ = nullptr;
raw_ptr<BookmarkMergedSurfaceService> bookmark_merged_surface_service_ =
nullptr;
- std::optional<BookmarkParentFolder> parent_folder_;
std::unique_ptr<internal::BookmarkUIOperationsHelper> helper_;
};
@@ -630,6 +630,35 @@
EXPECT_EQ(url, bar_folder->children()[0]->url().spec());
}
+TYPED_TEST(BookmarkUIOperationsHelperTest,
+ PasteFromClipboardAfterParentFolderRemoved) {
+ BookmarkModel* model = this->model();
+ const BookmarkNode* folder =
+ model->AddFolder(model->bookmark_bar_node(), 0, u"Folder");
+ model->AddURL(folder, 0, u"existing", GURL("https://existing.example/"));
+
+ {
+ ui::ScopedClipboardWriter clipboard_writer(ui::ClipboardBuffer::kCopyPaste);
+ clipboard_writer.WriteText(u"https://www.google.com/");
+ }
+
+ internal::BookmarkUIOperationsHelper* helper = this->CreateHelper(folder);
+ EXPECT_TRUE(this->CanPasteFromClipboardSync(helper));
+
+ // Remove the target folder before PasteFromClipboard completes.
+ model->Remove(folder, bookmarks::metrics::BookmarkEditSource::kOther,
+ FROM_HERE);
+
+ EXPECT_FALSE(this->CanPasteFromClipboardSync(helper));
+
+ {
+ base::test::TestFuture<void> future;
+ helper->PasteFromClipboard(1, future.GetCallback());
+ EXPECT_TRUE(future.Wait());
+ }
+ EXPECT_TRUE(model->bookmark_bar_node()->children().empty());
+}
+
#endif // !BUILDFLAG(IS_MAC)
TEST(BookmarkUIOperationsHelperMergedSurfacesTest,
diff --git a/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl_unittest.cc b/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl_unittest.cc
index 109f1ce..0cdbb36 100644
--- a/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl_unittest.cc
+++ b/chrome/browser/ui/bookmarks/controllers/bookmark_bar_ui_controller_impl_unittest.cc
@@ -270,7 +270,8 @@
ui::mojom::MenuSourceType source_type = ui::mojom::MenuSourceType::kMouse;
EXPECT_CALL(mock_model_adapter_, GetNodeById(node->id()))
- .WillOnce(testing::Return(node));
+ .Times(2)
+ .WillRepeatedly(testing::Return(node));
EXPECT_CALL(mock_model_adapter_,
CanPasteFromClipboard(testing::_, testing::_))
.WillOnce(base::test::RunOnceCallback<1>(true));
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bar_view_browsertest.cc b/chrome/browser/ui/views/bookmarks/bookmark_bar_view_browsertest.cc
index aa4d068..f5d615d 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_bar_view_browsertest.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_bar_view_browsertest.cc
@@ -4,6 +4,7 @@
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
+#include "base/memory/raw_ptr.h"
#include "base/task/sequenced_task_runner.h"
#include "base/test/run_until.h"
#include "base/test/scoped_feature_list.h"
@@ -28,15 +29,19 @@
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view_test_helper.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_context_menu.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/undo/bookmark_undo_service_factory.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/bookmarks/browser/bookmark_model.h"
+#include "components/bookmarks/common/bookmark_metrics.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
#include "components/bookmarks/test/bookmark_test_helpers.h"
#include "components/page_load_metrics/browser/navigation_handle_user_data.h"
#include "components/prefs/pref_service.h"
+#include "components/undo/bookmark_undo_service.h"
+#include "components/undo/undo_manager.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/test/browser_test.h"
@@ -52,6 +57,7 @@
#include "services/network/public/cpp/features.h"
#include "ui/accessibility/ax_action_data.h"
#include "ui/base/clipboard/test/test_clipboard.h"
+#include "ui/base/data_transfer_policy/data_transfer_endpoint.h"
#include "ui/base/page_transition_types.h"
#include "ui/events/base_event_utils.h"
#include "ui/events/test/test_event.h"
@@ -1168,6 +1174,96 @@
namespace {
+// A TestClipboard that holds on to ReadText() requests until the test
+// explicitly completes them, so tests can make changes while an async
+// clipboard read is pending.
+class DeferredReadTextClipboard : public ui::TestClipboard {
+ public:
+ bool HasPendingReadText() const { return !pending_callback_.is_null(); }
+
+ void CompletePendingReadText() {
+ CHECK(HasPendingReadText());
+ ui::TestClipboard::ReadText(pending_buffer_, pending_data_dst_,
+ std::move(pending_callback_));
+ }
+
+ // ui::TestClipboard:
+ void ReadText(ui::ClipboardBuffer buffer,
+ const std::optional<ui::DataTransferEndpoint>& data_dst,
+ ReadTextCallback callback) const override {
+ pending_buffer_ = buffer;
+ pending_data_dst_ = data_dst;
+ pending_callback_ = std::move(callback);
+ }
+
+ private:
+ mutable ui::ClipboardBuffer pending_buffer_ = ui::ClipboardBuffer::kCopyPaste;
+ mutable std::optional<ui::DataTransferEndpoint> pending_data_dst_;
+ mutable ReadTextCallback pending_callback_;
+};
+
+class BookmarkBarContextMenuPasteCheckTest : public BookmarkBarTest {
+ public:
+ void SetUpOnMainThread() override {
+ BookmarkBarTest::SetUpOnMainThread();
+ // Replace the default TestClipboard with one that defers reads.
+ ui::Clipboard::DestroyClipboardForCurrentThread();
+ auto clipboard = std::make_unique<DeferredReadTextClipboard>();
+ clipboard_ = clipboard.get();
+ ui::Clipboard::SetClipboardForCurrentThread(std::move(clipboard));
+ }
+
+ void TearDownOnMainThread() override {
+ clipboard_ = nullptr;
+ BookmarkBarTest::TearDownOnMainThread();
+ }
+
+ protected:
+ raw_ptr<DeferredReadTextClipboard> clipboard_ = nullptr;
+};
+
+} // namespace
+
+// Verifies that removing a bookmark folder while the context menu's async
+// paste check is pending does not show a menu for the removed folder.
+IN_PROC_BROWSER_TEST_F(BookmarkBarContextMenuPasteCheckTest,
+ FolderRemovedDuringPasteCheck) {
+ CreateBookmarkFolder();
+ views::View* button = GetBookmarkButton(0);
+ ASSERT_TRUE(button);
+
+ gfx::Point point;
+ views::View::ConvertPointToScreen(button, &point);
+ bookmark_bar()->ShowContextMenuForViewImpl(button, point,
+ ui::mojom::MenuSourceType::kMouse);
+ ASSERT_TRUE(base::test::RunUntil([&]() {
+ return clipboard_->HasPendingReadText();
+ })) << "The context menu paste check did not reach the clipboard read.";
+
+ bookmarks::BookmarkModel* model =
+ BookmarkModelFactory::GetForBrowserContext(browser()->GetProfile());
+ const bookmarks::BookmarkNode* folder =
+ model->bookmark_bar_node()->children()[0].get();
+ ASSERT_TRUE(folder->is_folder());
+
+ // Suspend undo tracking so the removed node is deleted immediately rather
+ // than kept alive by the undo stack.
+ UndoManager* undo_manager =
+ BookmarkUndoServiceFactory::GetForProfile(browser()->GetProfile())
+ ->undo_manager();
+ undo_manager->SuspendUndoTracking();
+ model->Remove(folder, bookmarks::metrics::BookmarkEditSource::kOther,
+ FROM_HERE);
+ undo_manager->ResumeUndoTracking();
+ RunScheduledLayouts();
+
+ clipboard_->CompletePendingReadText();
+
+ EXPECT_FALSE(views::MenuController::GetActiveInstance());
+}
+
+namespace {
+
ui::AXActionData CreateAXActionDataWithAction(ax::mojom::Action action) {
ui::AXActionData data;
data.action = action;
Loading diff…
Original Bug Report
The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.
References
On This Page