diff --git a/components/reading_list/core/dual_reading_list_model_unittest.cc b/components/reading_list/core/dual_reading_list_model_unittest.cc index 8cdfdb5376533..95dd565b203b5 100644 --- a/components/reading_list/core/dual_reading_list_model_unittest.cc +++ b/components/reading_list/core/dual_reading_list_model_unittest.cc @@ -13,6 +13,7 @@ #include "components/reading_list/core/reading_list_entry.h" #include "components/reading_list/core/reading_list_model_impl.h" #include "components/sync/base/storage_type.h" +#include "components/sync/model/client_tag_based_model_type_processor.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" @@ -189,11 +190,9 @@ class DualReadingListModelTest : public testing::Test { /*initial_account_entries_builders=*/{}); } - bool ResetStorageAndMimicSignedInSyncDisabled( + bool TriggerStorageLoadCompletionSignedInSyncDisabled( std::vector initial_local_entries_builders = {}, std::vector initial_account_entries_builders = {}) { - ResetStorage(); - auto metadata_batch = std::make_unique(); sync_pb::ModelTypeState state; state.set_initial_sync_state( @@ -217,6 +216,14 @@ class DualReadingListModelTest : public testing::Test { std::move(initial_account_entries), std::move(metadata_batch)); } + bool ResetStorageAndMimicSignedInSyncDisabled( + std::vector initial_local_entries_builders = {}, + std::vector initial_account_entries_builders = {}) { + ResetStorage(); + return TriggerStorageLoadCompletionSignedInSyncDisabled( + initial_local_entries_builders, initial_account_entries_builders); + } + bool ResetStorageAndMimicSyncEnabled( std::vector initial_syncable_entries_builders = {}) { ResetStorage(); @@ -276,6 +283,27 @@ TEST_F(DualReadingListModelTest, ModelLoadFailure) { EXPECT_FALSE(dual_model_->loaded()); } +TEST_F(DualReadingListModelTest, MetaDataClearedBeforeModelLoaded) { + ResetStorage(); + static_cast( + account_model_ptr_->GetSyncBridgeForTest()->change_processor()) + ->ClearMetadataWhileStopped(); + + EXPECT_CALL(observer_, ReadingListModelBeganBatchUpdates).Times(0); + EXPECT_CALL(observer_, ReadingListModelCompletedBatchUpdates).Times(0); + EXPECT_CALL(observer_, ReadingListWillRemoveEntry).Times(0); + EXPECT_CALL(observer_, ReadingListDidRemoveEntry).Times(0); + EXPECT_CALL(observer_, ReadingListDidApplyChanges).Times(0); + EXPECT_CALL(observer_, ReadingListModelLoaded); + TriggerStorageLoadCompletionSignedInSyncDisabled( + /*initial_local_entries_builders=*/{}, + /*initial_account_entries_builders=*/{ + TestEntryBuilder(kUrl, clock_.Now())}); + + EXPECT_EQ(0ul, account_model_ptr_->size()); + EXPECT_EQ(0ul, dual_model_->size()); +} + TEST_F(DualReadingListModelTest, ReturnAccountModelSize) { ASSERT_TRUE(ResetStorageAndTriggerLoadCompletion( /*initial_local_or_syncable_entries_builders=*/{}, diff --git a/components/reading_list/core/reading_list_model_impl.cc b/components/reading_list/core/reading_list_model_impl.cc index ca1c2222b6b1a..b429932452c7f 100644 --- a/components/reading_list/core/reading_list_model_impl.cc +++ b/components/reading_list/core/reading_list_model_impl.cc @@ -275,8 +275,11 @@ void ReadingListModelImpl::RemoveEntryByURLImpl(const GURL& url, if (!entry) return; - for (auto& observer : observers_) - observer.ReadingListWillRemoveEntry(this, url); + if (!suppress_deletions_batch_updates_notifications_) { + for (auto& observer : observers_) { + observer.ReadingListWillRemoveEntry(this, url); + } + } std::unique_ptr batch = storage_layer_->EnsureBatchCreated(); @@ -290,9 +293,11 @@ void ReadingListModelImpl::RemoveEntryByURLImpl(const GURL& url, entries_.erase(url); - for (auto& observer : observers_) { - observer.ReadingListDidRemoveEntry(this, url); - observer.ReadingListDidApplyChanges(this); + if (!suppress_deletions_batch_updates_notifications_) { + for (auto& observer : observers_) { + observer.ReadingListDidRemoveEntry(this, url); + observer.ReadingListDidApplyChanges(this); + } } } @@ -582,7 +587,8 @@ ReadingListModelImpl::BeginBatchUpdatesWithSyncMetadata() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); auto token = std::make_unique(this); ++current_batch_updates_count_; - if (current_batch_updates_count_ == 1) { + if (current_batch_updates_count_ == 1 && + !suppress_deletions_batch_updates_notifications_) { for (auto& observer : observers_) { observer.ReadingListModelBeganBatchUpdates(this); } @@ -646,8 +652,15 @@ void ReadingListModelImpl::StoreLoaded( DCHECK_EQ(read_entry_count_ + unread_entry_count_, entries_.size()); loaded_ = true; - sync_bridge_.ModelReadyToSync(/*model=*/this, - std::move(result_or_error.value().second)); + { + // In rare cases, ModelReadyToSync() leads to the deletion of all local + // entries. Such deletions should not be propagated to observers, because + // ReadingListModelLoaded hasn't been broadcasted yet. + base::AutoReset auto_reset_suppress_observer_notifications( + &suppress_deletions_batch_updates_notifications_, true); + sync_bridge_.ModelReadyToSync(/*model=*/this, + std::move(result_or_error.value().second)); + } base::UmaHistogramCounts1000("ReadingList.Unread.Count.OnModelLoaded", unread_entry_count_); @@ -664,7 +677,8 @@ void ReadingListModelImpl::EndBatchUpdates() { DCHECK(IsPerformingBatchUpdates()); DCHECK(current_batch_updates_count_ > 0); --current_batch_updates_count_; - if (current_batch_updates_count_ == 0) { + if (current_batch_updates_count_ == 0 && + !suppress_deletions_batch_updates_notifications_) { for (auto& observer : observers_) { observer.ReadingListModelCompletedBatchUpdates(this); } diff --git a/components/reading_list/core/reading_list_model_impl.h b/components/reading_list/core/reading_list_model_impl.h index 186362a1e9218..88ddb0ed80a73 100644 --- a/components/reading_list/core/reading_list_model_impl.h +++ b/components/reading_list/core/reading_list_model_impl.h @@ -179,6 +179,10 @@ class ReadingListModelImpl : public ReadingListModel { bool loaded_ = false; + // Used to suppress deletions and batch updates notifications when + // ReadingListModelLoaded is not broadcasted yet. + bool suppress_deletions_batch_updates_notifications_ = false; + std::map> entries_; size_t unread_entry_count_ = 0; size_t read_entry_count_ = 0;