From e33f23f1de15eb1f65e6b41f2bcb92fe4bae431e Mon Sep 17 00:00:00 2001
From: Boris Sazonov <bsazonov@google.com>
Date: Mon, 15 May 2023 10:21:10 +0000
Subject: [PATCH] [Bookmark] Clear account BookmarkModel on sign-out
BookmarkModelTypeProcessor::OnSyncStopping is called with
CLEAR_METADATA upon sign-out. After this CL,
BookmarkModelTypeProcessor can be configured to call
RemoveAllUserBookmarks at that moment.
Bug: 1444662
Change-Id: Ida7bba5e80940bd9ad81e80dc1062f283b8fab12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4527935
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1143978}
---
.../sync/bookmark_sync_service_factory.cc | 3 +-
.../bookmark_model_type_processor.cc | 41 +++++++-----
.../bookmark_model_type_processor.h | 28 ++++++---
.../bookmark_model_type_processor_unittest.cc | 62 ++++++++++++++++---
.../sync_bookmarks/bookmark_sync_service.cc | 7 ++-
.../sync_bookmarks/bookmark_sync_service.h | 10 ++-
.../account_bookmark_sync_service_factory.cc | 4 +-
..._syncable_bookmark_sync_service_factory.cc | 4 +-
8 files changed, 116 insertions(+), 43 deletions(-)
diff --git a/chrome/browser/sync/bookmark_sync_service_factory.cc b/chrome/browser/sync/bookmark_sync_service_factory.cc
index 3a0a187a39c40..548fb4b69b76e 100644
--- a/chrome/browser/sync/bookmark_sync_service_factory.cc
+++ b/chrome/browser/sync/bookmark_sync_service_factory.cc
@@ -38,5 +38,6 @@ KeyedService* BookmarkSyncServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
Profile* profile = Profile::FromBrowserContext(context);
return new sync_bookmarks::BookmarkSyncService(
- BookmarkUndoServiceFactory::GetForProfileIfExists(profile));
+ BookmarkUndoServiceFactory::GetForProfileIfExists(profile),
+ /*wipe_model_on_stopping_sync_with_clear_data=*/false);
}
diff --git a/components/sync_bookmarks/bookmark_model_type_processor.cc b/components/sync_bookmarks/bookmark_model_type_processor.cc
index ab84151c1d8d8..cf9d32dd70643 100644
--- a/components/sync_bookmarks/bookmark_model_type_processor.cc
+++ b/components/sync_bookmarks/bookmark_model_type_processor.cc
@@ -51,7 +51,7 @@ constexpr size_t kDefaultMaxBookmarksTillSyncEnabled = 100000;
class ScopedRemoteUpdateBookmarks {
public:
- // |bookmark_model|, |bookmark_undo_service| and |observer| must not be null
+ // `bookmark_model`, `bookmark_undo_service` and `observer` must not be null
// and must outlive this object.
ScopedRemoteUpdateBookmarks(bookmarks::BookmarkModel* bookmark_model,
BookmarkUndoService* bookmark_undo_service,
@@ -123,8 +123,11 @@ size_t CountSyncableBookmarksFromModel(bookmarks::BookmarkModel* model) {
} // namespace
BookmarkModelTypeProcessor::BookmarkModelTypeProcessor(
- BookmarkUndoService* bookmark_undo_service)
+ BookmarkUndoService* bookmark_undo_service,
+ bool wipe_model_on_stopping_sync_with_clear_data)
: bookmark_undo_service_(bookmark_undo_service),
+ wipe_model_on_stopping_sync_with_clear_data_(
+ wipe_model_on_stopping_sync_with_clear_data),
max_bookmarks_till_sync_enabled_(kDefaultMaxBookmarksTillSyncEnabled) {}
BookmarkModelTypeProcessor::~BookmarkModelTypeProcessor() {
@@ -141,7 +144,7 @@ void BookmarkModelTypeProcessor::ConnectSync(
worker_ = std::move(worker);
- // |bookmark_tracker_| is instantiated only after initial sync is done.
+ // `bookmark_tracker_` is instantiated only after initial sync is done.
if (bookmark_tracker_) {
NudgeForCommitIfNeeded();
}
@@ -164,7 +167,7 @@ void BookmarkModelTypeProcessor::GetLocalChanges(
GetLocalChangesCallback callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Processor should never connect if
- // |last_initial_merge_remote_updates_exceeded_limit_| is set.
+ // `last_initial_merge_remote_updates_exceeded_limit_` is set.
DCHECK(!last_initial_merge_remote_updates_exceeded_limit_);
BookmarkLocalChangesBuilder builder(bookmark_tracker_.get(), bookmark_model_);
std::move(callback).Run(builder.BuildCommitRequests(max_entries));
@@ -176,7 +179,7 @@ void BookmarkModelTypeProcessor::OnCommitCompleted(
const syncer::FailedCommitResponseDataList& error_response_list) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- // |error_response_list| is ignored, because all errors are treated as
+ // `error_response_list` is ignored, because all errors are treated as
// transient and the processor with eventually retry.
for (const syncer::CommitResponseData& response : committed_response_list) {
const SyncedBookmarkTrackerEntity* entity =
@@ -205,10 +208,10 @@ void BookmarkModelTypeProcessor::OnUpdateReceived(
DCHECK(syncer::IsInitialSyncDone(model_type_state.initial_sync_state()));
DCHECK(start_callback_.is_null());
// Processor should never connect if
- // |last_initial_merge_remote_updates_exceeded_limit_| is set.
+ // `last_initial_merge_remote_updates_exceeded_limit_` is set.
DCHECK(!last_initial_merge_remote_updates_exceeded_limit_);
- // TODO(crbug.com/1356900): validate incoming updates, e.g. |gc_directive|
+ // TODO(crbug.com/1356900): validate incoming updates, e.g. `gc_directive`
// must be empty for Bookmarks.
syncer::LogUpdatesReceivedByProcessorHistogram(
@@ -285,7 +288,7 @@ bool BookmarkModelTypeProcessor::IsConnectedForTest() const {
std::string BookmarkModelTypeProcessor::EncodeSyncMetadata() const {
std::string metadata_str;
if (bookmark_tracker_) {
- // |last_initial_merge_remote_updates_exceeded_limit_| is only set in error
+ // `last_initial_merge_remote_updates_exceeded_limit_` is only set in error
// cases where the tracker would not be initialized.
DCHECK(!last_initial_merge_remote_updates_exceeded_limit_);
@@ -343,7 +346,7 @@ void BookmarkModelTypeProcessor::ModelReadyToSync(
syncer::kSyncEnforceBookmarksCountLimit)) {
// Report error if remote updates fetched last time during initial merge
// exceeded limit. Note that here we are only setting
- // |last_initial_merge_remote_updates_exceeded_limit_|, the actual error
+ // `last_initial_merge_remote_updates_exceeded_limit_`, the actual error
// would be reported in ConnectIfReady().
last_initial_merge_remote_updates_exceeded_limit_ = true;
} else {
@@ -354,8 +357,8 @@ void BookmarkModelTypeProcessor::ModelReadyToSync(
if (bookmark_tracker_) {
StartTrackingMetadata();
} else if (!metadata_str.empty()) {
- // Even if the field |last_initial_merge_remote_updates_exceeded_limit| is
- // set and the feature toggle |kSyncEnforceBookmarksCountLimit| not
+ // Even if the field `last_initial_merge_remote_updates_exceeded_limit` is
+ // set and the feature toggle `kSyncEnforceBookmarksCountLimit` not
// enabled, making the metadata_str non-empty, scheduling a save shouldn't
// cause any problem.
DLOG(WARNING)
@@ -398,7 +401,7 @@ void BookmarkModelTypeProcessor::OnSyncStarting(
StartCallback start_callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(start_callback);
- // |favicon_service_| should have been set by now.
+ // `favicon_service_` should have been set by now.
DCHECK(favicon_service_);
DVLOG(1) << "Sync is starting for Bookmarks";
@@ -428,7 +431,7 @@ void BookmarkModelTypeProcessor::ConnectIfReady() {
// Report error if remote updates fetched last time during initial merge
// exceeded limit.
if (last_initial_merge_remote_updates_exceeded_limit_) {
- // |last_initial_merge_remote_updates_exceeded_limit_| is only set in error
+ // `last_initial_merge_remote_updates_exceeded_limit_` is only set in error
// case and thus tracker should be empty.
DCHECK(!bookmark_tracker_);
start_callback_.Reset();
@@ -510,6 +513,14 @@ void BookmarkModelTypeProcessor::OnSyncStopping(
StopTrackingMetadataAndResetTracker();
}
last_initial_merge_remote_updates_exceeded_limit_ = false;
+ if (wipe_model_on_stopping_sync_with_clear_data_) {
+ // `CLEAR_METADATA` indicates sync is permanently disabled. Since
+ // `wipe_model_on_stopping_sync_with_clear_data_` is `true`, the
+ // lifetime of local data (bookmarks) is coupled with sync metadata's,
+ // which means disabling sync requires that bookmarks in local storage
+ // are deleted.
+ bookmark_model_->RemoveAllUserBookmarks();
+ }
schedule_save_closure_.Run();
break;
}
@@ -524,7 +535,7 @@ void BookmarkModelTypeProcessor::NudgeForCommitIfNeeded() {
DCHECK(bookmark_tracker_);
// Issue error and stop sync if the number of local bookmarks exceed limit.
- // If |error_handler_| is not set, the check is ignored because this gets
+ // If `error_handler_` is not set, the check is ignored because this gets
// re-evaluated in ConnectIfReady().
if (error_handler_ &&
bookmark_tracker_->TrackedBookmarksCount() >
@@ -564,7 +575,7 @@ void BookmarkModelTypeProcessor::OnInitialUpdateReceived(
TRACE_EVENT0("sync", "BookmarkModelTypeProcessor::OnInitialUpdateReceived");
- // |updates| can contain an additional root folder. The server may or may not
+ // `updates` can contain an additional root folder. The server may or may not
// deliver a root node - it is not guaranteed, but this works as an
// approximated safeguard.
const size_t max_initial_updates_count = max_bookmarks_till_sync_enabled_ + 1;
diff --git a/components/sync_bookmarks/bookmark_model_type_processor.h b/components/sync_bookmarks/bookmark_model_type_processor.h
index 1416e086a7ae3..d14469f0989ad 100644
--- a/components/sync_bookmarks/bookmark_model_type_processor.h
+++ b/components/sync_bookmarks/bookmark_model_type_processor.h
@@ -35,9 +35,12 @@ class BookmarkModelObserverImpl;
class BookmarkModelTypeProcessor : public syncer::ModelTypeProcessor,
public syncer::ModelTypeControllerDelegate {
public:
- // |bookmark_undo_service| must not be nullptr and must outlive this object.
- explicit BookmarkModelTypeProcessor(
- BookmarkUndoService* bookmark_undo_service);
+ // `bookmark_undo_service` must not be nullptr and must outlive this object.
+ // If `wipe_model_on_stopping_sync_with_clear_data` is `true`, then calling
+ // `OnSyncStopping` with `CLEAR_METADATA` will trigger the removal of all user
+ // bookmarks from the corresponding `BookmarkModel`.
+ BookmarkModelTypeProcessor(BookmarkUndoService* bookmark_undo_service,
+ bool wipe_model_on_stopping_sync_with_clear_data);
BookmarkModelTypeProcessor(const BookmarkModelTypeProcessor&) = delete;
BookmarkModelTypeProcessor& operator=(const BookmarkModelTypeProcessor&) =
@@ -78,10 +81,10 @@ class BookmarkModelTypeProcessor : public syncer::ModelTypeProcessor,
std::string EncodeSyncMetadata() const;
// It mainly decodes a BookmarkModelMetadata proto serialized in
- // |metadata_str|, and uses it to fill in the tracker and the model type state
- // objects. |model| must not be null and must outlive this object. It is used
+ // `metadata_str`, and uses it to fill in the tracker and the model type state
+ // objects. `model` must not be null and must outlive this object. It is used
// to the retrieve the local node ids, and is stored in the processor to be
- // used for further model operations. |schedule_save_closure| is a repeating
+ // used for further model operations. `schedule_save_closure` is a repeating
// closure used to schedule a save of the bookmark model together with the
// metadata.
void ModelReadyToSync(const std::string& metadata_str,
@@ -90,7 +93,7 @@ class BookmarkModelTypeProcessor : public syncer::ModelTypeProcessor,
// Sets the favicon service used when processing remote updates. It must be
// called before the processor is ready to receive remote updates, and hence
- // before OnSyncStarting() is called. |favicon_service| must not be null.
+ // before OnSyncStarting() is called. `favicon_service` must not be null.
void SetFaviconService(favicon::FaviconService* favicon_service);
// Returns the estimate of dynamically allocated memory in bytes.
@@ -136,9 +139,9 @@ class BookmarkModelTypeProcessor : public syncer::ModelTypeProcessor,
void StopTrackingMetadataAndResetTracker();
// Creates a DictionaryValue for local and remote debugging information about
- // |node| and appends it to |all_nodes|. It does the same for child nodes
- // recursively. |index| is the index of |node| within its parent. |index|
- // could computed from |node|, however it's much cheaper to pass from outside
+ // `node` and appends it to `all_nodes`. It does the same for child nodes
+ // recursively. `index` is the index of `node` within its parent. `index`
+ // could computed from `node`, however it's much cheaper to pass from outside
// since we iterate over child nodes already in the calling sites.
void AppendNodeAndChildrenForDebugging(const bookmarks::BookmarkNode* node,
int index,
@@ -163,6 +166,11 @@ class BookmarkModelTypeProcessor : public syncer::ModelTypeProcessor,
// Used to suspend bookmark undo when processing remote changes.
const raw_ptr<BookmarkUndoService, DanglingUntriaged> bookmark_undo_service_;
+ // Controls whether bookmarks should be wiped when sync is stopped. Contains
+ // `true` for Account `BookmarkModel` and `false` for LocalOrSyncable
+ // `BookmarkModel`.
+ const bool wipe_model_on_stopping_sync_with_clear_data_;
+
// The callback used to schedule the persistence of bookmark model as well as
// the metadata to a file during which latest metadata should also be pulled
// via EncodeSyncMetadata. Processor should invoke it upon changes in the
diff --git a/components/sync_bookmarks/bookmark_model_type_processor_unittest.cc b/components/sync_bookmarks/bookmark_model_type_processor_unittest.cc
index c6e353274d8a6..f8950df2b0c38 100644
--- a/components/sync_bookmarks/bookmark_model_type_processor_unittest.cc
+++ b/components/sync_bookmarks/bookmark_model_type_processor_unittest.cc
@@ -50,6 +50,7 @@ using testing::IsEmpty;
using testing::IsNull;
using testing::NiceMock;
using testing::NotNull;
+using testing::Pointer;
using testing::UnorderedElementsAre;
const char kBookmarkBarTag[] = "bookmark_bar";
@@ -244,7 +245,8 @@ class BookmarkModelTypeProcessorTest : public testing::Test {
public:
BookmarkModelTypeProcessorTest()
: processor_(std::make_unique<BookmarkModelTypeProcessor>(
- &bookmark_undo_service_)),
+ &bookmark_undo_service_,
+ /*wipe_model_on_stopping_sync_with_clear_data=*/false)),
bookmark_model_(bookmarks::TestBookmarkClient::CreateModel()) {
processor_->SetFaviconService(&favicon_service_);
}
@@ -306,9 +308,10 @@ class BookmarkModelTypeProcessorTest : public testing::Test {
}
// Simulate browser restart.
- void ResetModelTypeProcessor() {
- processor_ =
- std::make_unique<BookmarkModelTypeProcessor>(&bookmark_undo_service_);
+ void ResetModelTypeProcessor(
+ bool wipe_model_on_stopping_sync_with_clear_data = false) {
+ processor_ = std::make_unique<BookmarkModelTypeProcessor>(
+ &bookmark_undo_service_, wipe_model_on_stopping_sync_with_clear_data);
processor_->SetFaviconService(&favicon_service_);
}
@@ -517,7 +520,9 @@ TEST_F(BookmarkModelTypeProcessorTest, ShouldDecodeSyncMetadata) {
CreateNodeMetadata(bookmarknode, kNodeId);
// Create a new processor and init it with the metadata str.
- BookmarkModelTypeProcessor new_processor(bookmark_undo_service());
+ BookmarkModelTypeProcessor new_processor(
+ bookmark_undo_service(),
+ /*wipe_model_on_stopping_sync_with_clear_data=*/false);
std::string metadata_str;
model_metadata.SerializeToString(&metadata_str);
@@ -546,7 +551,9 @@ TEST_F(BookmarkModelTypeProcessorTest, ShouldDecodeEncodedSyncMetadata) {
SimulateModelReadyToSyncWithInitialSyncDone();
// Create a new processor and init it with the same metadata str.
- BookmarkModelTypeProcessor new_processor(bookmark_undo_service());
+ BookmarkModelTypeProcessor new_processor(
+ bookmark_undo_service(),
+ /*wipe_model_on_stopping_sync_with_clear_data=*/false);
new_processor.ModelReadyToSync(processor()->EncodeSyncMetadata(),
base::DoNothing(), bookmark_model());
@@ -579,7 +586,9 @@ TEST_F(BookmarkModelTypeProcessorTest,
bookmark_metadata->mutable_metadata()->set_server_id(kBookmarkBarId);
// Create a new processor and init it with the metadata str.
- BookmarkModelTypeProcessor new_processor(bookmark_undo_service());
+ BookmarkModelTypeProcessor new_processor(
+ bookmark_undo_service(),
+ /*wipe_model_on_stopping_sync_with_clear_data=*/false);
// A save should be scheduled.
NiceMock<base::MockCallback<base::RepeatingClosure>>
@@ -606,7 +615,9 @@ TEST_F(BookmarkModelTypeProcessorTest,
/*server_id=*/kBookmarkBarId);
// Create a new processor and init it with the metadata str.
- BookmarkModelTypeProcessor new_processor(bookmark_undo_service());
+ BookmarkModelTypeProcessor new_processor(
+ bookmark_undo_service(),
+ /*wipe_model_on_stopping_sync_with_clear_data=*/false);
// A save should be scheduled.
NiceMock<base::MockCallback<base::RepeatingClosure>>
@@ -1573,6 +1584,41 @@ TEST_F(BookmarkModelTypeProcessorTest,
"Sync.ClearMetadataWhileStopped.ImmediateClear", 0);
}
+TEST_F(BookmarkModelTypeProcessorTest,
+ ShouldWipeBookmarksIfStoppedWithClearMetadata) {
+ ResetModelTypeProcessor(/*wipe_model_on_stopping_sync_with_clear_data=*/true);
+
+ const GURL kUrl("http://www.example.com");
+ bookmark_model()->AddURL(bookmark_model()->bookmark_bar_node(), /*index=*/0,
+ u"foo", kUrl);
+ const bookmarks::BookmarkNode* folder = bookmark_model()->AddFolder(
+ bookmark_model()->mobile_node(), /*index=*/0, u"folder");
+ bookmark_model()->AddURL(folder, /*index=*/0, u"bar", kUrl);
+
+ SimulateModelReadyToSyncWithInitialSyncDone();
+ SimulateOnSyncStarting();
+ ASSERT_FALSE(bookmark_model()->HasNoUserCreatedBookmarksOrFolders());
+
+ processor()->OnSyncStopping(syncer::CLEAR_METADATA);
+ EXPECT_TRUE(bookmark_model()->HasNoUserCreatedBookmarksOrFolders());
+}
+
+TEST_F(BookmarkModelTypeProcessorTest,
+ ShouldNotWipeBookmarksIfStoppedWithKeepMetadata) {
+ ResetModelTypeProcessor(/*wipe_model_on_stopping_sync_with_clear_data=*/true);
+
+ const GURL kUrl("http://www.example.com");
+ const bookmarks::BookmarkNode* node = bookmark_model()->AddURL(
+ bookmark_model()->mobile_node(), /*index=*/0, u"foo", kUrl);
+
+ SimulateModelReadyToSyncWithInitialSyncDone();
+ SimulateOnSyncStarting();
+
+ processor()->OnSyncStopping(syncer::KEEP_METADATA);
+ EXPECT_THAT(bookmark_model()->mobile_node()->children(),
+ ElementsAre(Pointer(Eq(node))));
+}
+
TEST_F(BookmarkModelTypeProcessorTest,
ShouldNotClearMetadataWhileStoppedWithoutMetadataInitially) {
SimulateModelReadyToSyncWithoutLocalMetadata();
diff --git a/components/sync_bookmarks/bookmark_sync_service.cc b/components/sync_bookmarks/bookmark_sync_service.cc
index afeafd5ee9730..0105eddd797c4 100644
--- a/components/sync_bookmarks/bookmark_sync_service.cc
+++ b/components/sync_bookmarks/bookmark_sync_service.cc
@@ -10,8 +10,11 @@
namespace sync_bookmarks {
BookmarkSyncService::BookmarkSyncService(
- BookmarkUndoService* bookmark_undo_service)
- : bookmark_model_type_processor_(bookmark_undo_service) {}
+ BookmarkUndoService* bookmark_undo_service,
+ bool wipe_model_on_stopping_sync_with_clear_data)
+ : bookmark_model_type_processor_(
+ bookmark_undo_service,
+ wipe_model_on_stopping_sync_with_clear_data) {}
BookmarkSyncService::~BookmarkSyncService() = default;
diff --git a/components/sync_bookmarks/bookmark_sync_service.h b/components/sync_bookmarks/bookmark_sync_service.h
index 7544836ad52e3..94edb706c298b 100644
--- a/components/sync_bookmarks/bookmark_sync_service.h
+++ b/components/sync_bookmarks/bookmark_sync_service.h
@@ -34,8 +34,12 @@ class BookmarkModelTypeProcessor;
// This service owns the BookmarkModelTypeProcessor.
class BookmarkSyncService : public KeyedService {
public:
- // |bookmark_undo_service| must not be null and must outlive this object.
- explicit BookmarkSyncService(BookmarkUndoService* bookmark_undo_service);
+ // If `wipe_model_on_stopping_sync_with_clear_data` is `true`, then the
+ // `bookmark_undo_service` must not be null and must outlive this object.
+ // lifetime of bookmarks in the associated storage is coupled with sync
+ // metadata's, so disabling sync will delete bookmarks in the storage.
+ BookmarkSyncService(BookmarkUndoService* bookmark_undo_service,
+ bool wipe_model_on_stopping_sync_with_clear_data);
BookmarkSyncService(const BookmarkSyncService&) = delete;
BookmarkSyncService& operator=(const BookmarkSyncService&) = delete;
@@ -51,7 +55,7 @@ class BookmarkSyncService : public KeyedService {
bookmarks::BookmarkModel* model);
// Returns the ModelTypeControllerDelegate for syncer::BOOKMARKS.
- // |favicon_service| is the favicon service used when processing updates in
+ // `favicon_service` is the favicon service used when processing updates in
// the underlying processor. It could have been a separate a setter in
// BookmarkSyncService instead of passing it as a parameter to
// GetBookmarkSyncControllerDelegate(). However, this would incur the risk of
diff --git a/ios/chrome/browser/bookmarks/account_bookmark_sync_service_factory.cc b/ios/chrome/browser/bookmarks/account_bookmark_sync_service_factory.cc
index 177d7f7214bc6..b71073e5f4d89 100644
--- a/ios/chrome/browser/bookmarks/account_bookmark_sync_service_factory.cc
+++ b/ios/chrome/browser/bookmarks/account_bookmark_sync_service_factory.cc
@@ -51,8 +51,8 @@ AccountBookmarkSyncServiceFactory::BuildServiceInstanceFor(
ChromeBrowserState::FromBrowserState(context);
std::unique_ptr<sync_bookmarks::BookmarkSyncService> bookmark_sync_service(
new sync_bookmarks::BookmarkSyncService(
- BookmarkUndoServiceFactory::GetForBrowserStateIfExists(
- browser_state)));
+ BookmarkUndoServiceFactory::GetForBrowserStateIfExists(browser_state),
+ /*wipe_model_on_stopping_sync_with_clear_data=*/true));
return bookmark_sync_service;
}
diff --git a/ios/chrome/browser/bookmarks/local_or_syncable_bookmark_sync_service_factory.cc b/ios/chrome/browser/bookmarks/local_or_syncable_bookmark_sync_service_factory.cc
index fb2ec3f8f604d..06a67adaaeddf 100644
--- a/ios/chrome/browser/bookmarks/local_or_syncable_bookmark_sync_service_factory.cc
+++ b/ios/chrome/browser/bookmarks/local_or_syncable_bookmark_sync_service_factory.cc
@@ -46,8 +46,8 @@ LocalOrSyncableBookmarkSyncServiceFactory::BuildServiceInstanceFor(
ChromeBrowserState::FromBrowserState(context);
std::unique_ptr<sync_bookmarks::BookmarkSyncService> bookmark_sync_service(
new sync_bookmarks::BookmarkSyncService(
- BookmarkUndoServiceFactory::GetForBrowserStateIfExists(
- browser_state)));
+ BookmarkUndoServiceFactory::GetForBrowserStateIfExists(browser_state),
+ /*wipe_model_on_stopping_sync_with_clear_data=*/false));
return bookmark_sync_service;
}