Skip to content

Commit

Permalink
Cleanup OnSyncUpdatesReceived and SyncStarted
Browse files Browse the repository at this point in the history
Both AutofillWebDataServiceObserverOnUISequence::OnSyncUpdatesReceived
and AutofillWebDataServiceObserverOnUISequence::SyncStarted are not
used anymore.

Bug: 1477292
Change-Id: Ic0d4ff2c701ab9e802f4ab30625d72bc3f6eba27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4916043
Reviewed-by: Florian Leimgruber <fleimgruber@google.com>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Vidhan Jain <vidhanj@google.com>
Cr-Commit-Position: refs/heads/main@{#1209392}
  • Loading branch information
Vidhan authored and Chromium LUCI CQ committed Oct 13, 2023
1 parent b6c88c2 commit 8cb1a8d
Show file tree
Hide file tree
Showing 15 changed files with 4 additions and 131 deletions.
8 changes: 0 additions & 8 deletions components/autofill/core/browser/personal_data_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -663,14 +663,6 @@ void PersonalDataManager::AutofillAddressConversionCompleted() {
Refresh();
}

void PersonalDataManager::SyncStarted(syncer::ModelType model_type) {
personal_data_manager_cleaner_->SyncStarted(model_type);
}

void PersonalDataManager::OnSyncUpdatesReceived(syncer::ModelType model_type) {
// TODO(crbug.com/1477292): Need to be implemented.
}

void PersonalDataManager::OnStateChanged(syncer::SyncService* sync_service) {
DCHECK_EQ(sync_service_, sync_service);

Expand Down
2 changes: 0 additions & 2 deletions components/autofill/core/browser/personal_data_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,6 @@ class PersonalDataManager : public KeyedService,
// AutofillWebDataServiceObserverOnUISequence:
void AutofillMultipleChangedBySync(syncer::ModelType model_type) override;
void AutofillAddressConversionCompleted() override;
void SyncStarted(syncer::ModelType model_type) override;
void OnSyncUpdatesReceived(syncer::ModelType model_type) override;

// SyncServiceObserver:
void OnStateChanged(syncer::SyncService* sync) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,6 @@ optional<syncer::ModelError> AutocompleteSyncBridge::MergeFullSyncData(
change_processor()));

web_data_backend_->CommitChanges();
web_data_backend_->NotifyThatSyncHasStarted(syncer::AUTOFILL);
return {};
}

Expand All @@ -369,7 +368,6 @@ optional<ModelError> AutocompleteSyncBridge::ApplyIncrementalSyncChanges(
change_processor()));

web_data_backend_->CommitChanges();
web_data_backend_->NotifyOnSyncUpdatesReceived(syncer::AUTOFILL);
return {};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,6 @@ class AutocompleteSyncBridgeTest : public testing::Test {
}

void ApplyChanges(EntityChangeList changes) {
EXPECT_CALL(*backend(), NotifyOnSyncUpdatesReceived(syncer::AUTOFILL));
const auto error = bridge()->ApplyIncrementalSyncChanges(
bridge()->CreateMetadataChangeList(), std::move(changes));
EXPECT_FALSE(error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ optional<syncer::ModelError> AutofillProfileSyncBridge::MergeFullSyncData(
FlushSyncTracker(std::move(metadata_change_list), &initial_sync_tracker));

web_data_backend_->CommitChanges();
web_data_backend_->NotifyThatSyncHasStarted(syncer::AUTOFILL_PROFILE);
return absl::nullopt;
}

Expand Down Expand Up @@ -159,7 +158,6 @@ optional<ModelError> AutofillProfileSyncBridge::ApplyIncrementalSyncChanges(
RETURN_IF_ERROR(FlushSyncTracker(std::move(metadata_change_list), &tracker));

web_data_backend_->CommitChanges();
web_data_backend_->NotifyOnSyncUpdatesReceived(syncer::AUTOFILL_PROFILE);
return absl::nullopt;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,6 @@ class AutofillProfileSyncBridgeTest : public testing::Test {
}

void ApplyIncrementalSyncChanges(EntityChangeList changes) {
EXPECT_CALL(*backend(),
NotifyOnSyncUpdatesReceived(syncer::AUTOFILL_PROFILE));
const absl::optional<syncer::ModelError> error =
bridge()->ApplyIncrementalSyncChanges(
bridge()->CreateMetadataChangeList(), std::move(changes));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,6 @@ class AutofillWebDataBackend {
// NOTE: This method is intended to be called from the DB sequence. The UI
// sequence notifications are asynchronous.
virtual void NotifyOfAddressConversionCompleted() = 0;

// Notifies listeners on the UI sequence when sync has first been
// enabled for |model_type|. (NOT called on subsequent browser startups!)
// NOTE: This method is intended to be called from the DB sequence. The UI
// sequence notifications are asynchronous.
virtual void NotifyThatSyncHasStarted(syncer::ModelType model_type) = 0;

// Notifies listeners on the UI sequence that sync has been running for
// |model_type|.
// NOTE: This method is intended to be called from the DB sequence. The UI
// sequence notifications are asynchronous.
virtual void NotifyOnSyncUpdatesReceived(syncer::ModelType model_type) = 0;
};

} // namespace autofill
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,20 +128,14 @@ AutofillWebDataBackendImpl::AutofillWebDataBackendImpl(
scoped_refptr<base::SequencedTaskRunner> ui_task_runner,
scoped_refptr<base::SequencedTaskRunner> db_task_runner,
const base::RepeatingCallback<void(syncer::ModelType)>& on_changed_callback,
const base::RepeatingClosure& on_address_conversion_completed_callback,
const base::RepeatingCallback<void(syncer::ModelType)>&
on_sync_started_callback,
const base::RepeatingCallback<void(syncer::ModelType)>&
on_sync_updates_received_callback)
const base::RepeatingClosure& on_address_conversion_completed_callback)
: base::RefCountedDeleteOnSequence<AutofillWebDataBackendImpl>(
std::move(db_task_runner)),
ui_task_runner_(ui_task_runner),
web_database_backend_(web_database_backend),
on_changed_callback_(on_changed_callback),
on_address_conversion_completed_callback_(
on_address_conversion_completed_callback),
on_sync_started_callback_(on_sync_started_callback),
on_sync_updates_received_callback_(on_sync_updates_received_callback) {}
on_address_conversion_completed_callback) {}

void AutofillWebDataBackendImpl::AddObserver(
AutofillWebDataServiceObserverOnDBSequence* observer) {
Expand Down Expand Up @@ -230,32 +224,6 @@ void AutofillWebDataBackendImpl::NotifyOfAddressConversionCompleted() {
on_address_conversion_completed_callback_);
}

void AutofillWebDataBackendImpl::NotifyThatSyncHasStarted(
syncer::ModelType model_type) {
DCHECK(owning_task_runner()->RunsTasksInCurrentSequence());

if (on_sync_started_callback_.is_null())
return;

// UI sequence notification.
ui_task_runner_->PostTask(
FROM_HERE, base::BindOnce(on_sync_started_callback_, model_type));
}

void AutofillWebDataBackendImpl::NotifyOnSyncUpdatesReceived(
syncer::ModelType model_type) {
DCHECK(owning_task_runner()->RunsTasksInCurrentSequence());

if (on_sync_updates_received_callback_.is_null()) {
return;
}

// UI sequence notification.
ui_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(on_sync_updates_received_callback_, model_type));
}

base::SupportsUserData* AutofillWebDataBackendImpl::GetDBUserData() {
DCHECK(owning_task_runner()->RunsTasksInCurrentSequence());
if (!user_data_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,7 @@ class AutofillWebDataBackendImpl
scoped_refptr<base::SequencedTaskRunner> db_task_runner,
const base::RepeatingCallback<void(syncer::ModelType)>&
on_changed_callback,
const base::RepeatingClosure& on_address_conversion_completed_callback,
const base::RepeatingCallback<void(syncer::ModelType)>&
on_sync_started_callback,
const base::RepeatingCallback<void(syncer::ModelType)>&
on_sync_updates_received_callback);
const base::RepeatingClosure& on_address_conversion_completed_callback);

AutofillWebDataBackendImpl(const AutofillWebDataBackendImpl&) = delete;
AutofillWebDataBackendImpl& operator=(const AutofillWebDataBackendImpl&) =
Expand All @@ -80,8 +76,6 @@ class AutofillWebDataBackendImpl
void NotifyOfCreditCardChanged(const CreditCardChange& change) override;
void NotifyOfMultipleAutofillChanges(syncer::ModelType model_type) override;
void NotifyOfAddressConversionCompleted() override;
void NotifyThatSyncHasStarted(syncer::ModelType model_type) override;
void NotifyOnSyncUpdatesReceived(syncer::ModelType model_type) override;
void CommitChanges() override;

// Returns a SupportsUserData object that may be used to store data accessible
Expand Down Expand Up @@ -297,9 +291,6 @@ class AutofillWebDataBackendImpl

base::RepeatingCallback<void(syncer::ModelType)> on_changed_callback_;
base::RepeatingClosure on_address_conversion_completed_callback_;
base::RepeatingCallback<void(syncer::ModelType)> on_sync_started_callback_;
base::RepeatingCallback<void(syncer::ModelType)>
on_sync_updates_received_callback_;
base::RepeatingCallback<void(const AutofillProfileDeepChange&)>
on_autofill_profile_changed_cb_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,9 @@ AutofillWebDataService::AutofillWebDataService(
&AutofillWebDataService::
NotifyAutofillAddressConversionCompletedOnUISequence,
weak_ptr_factory_.GetWeakPtr());
base::RepeatingCallback<void(syncer::ModelType)> on_sync_started_callback =
base::BindRepeating(
&AutofillWebDataService::NotifySyncStartedOnUISequence,
weak_ptr_factory_.GetWeakPtr());
base::RepeatingCallback<void(syncer::ModelType)>
on_sync_updates_received_callback = base::BindRepeating(
&AutofillWebDataService::NotifyOnSyncUpdatesReceivedOnUISequence,
weak_ptr_factory_.GetWeakPtr());
autofill_backend_ = new AutofillWebDataBackendImpl(
wdbs_->GetBackend(), ui_task_runner_, db_task_runner_,
on_changed_callback, on_address_conversion_completed_callback,
on_sync_started_callback, on_sync_updates_received_callback);
on_changed_callback, on_address_conversion_completed_callback);
}

AutofillWebDataService::AutofillWebDataService(
Expand All @@ -68,8 +59,6 @@ AutofillWebDataService::AutofillWebDataService(
ui_task_runner_,
db_task_runner_,
base::NullCallback(),
base::NullCallback(),
base::NullCallback(),
base::NullCallback())) {}

void AutofillWebDataService::ShutdownOnUISequence() {
Expand Down Expand Up @@ -477,19 +466,4 @@ void AutofillWebDataService::
ui_observer.AutofillAddressConversionCompleted();
}

void AutofillWebDataService::NotifySyncStartedOnUISequence(
syncer::ModelType model_type) {
DCHECK(ui_task_runner_->RunsTasksInCurrentSequence());
for (auto& ui_observer : ui_observer_list_)
ui_observer.SyncStarted(model_type);
}

void AutofillWebDataService::NotifyOnSyncUpdatesReceivedOnUISequence(
syncer::ModelType model_type) {
CHECK(ui_task_runner_->RunsTasksInCurrentSequence());
for (auto& ui_observer : ui_observer_list_) {
ui_observer.OnSyncUpdatesReceived(model_type);
}
}

} // namespace autofill
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,6 @@ class AutofillWebDataService : public WebDataServiceBase {

void NotifyAutofillMultipleChangedOnUISequence(syncer::ModelType model_type);
void NotifyAutofillAddressConversionCompletedOnUISequence();
void NotifySyncStartedOnUISequence(syncer::ModelType model_type);
void NotifyOnSyncUpdatesReceivedOnUISequence(syncer::ModelType model_type);

base::WeakPtr<AutofillWebDataService> AsWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,6 @@ class AutofillWebDataServiceObserverOnUISequence {

virtual void AutofillAddressConversionCompleted() {}

virtual void AutofillProfileChanged(const AutofillProfileChange& change) {}

// Called on the UI sequence when sync has first been enabled for
// |model_type|. (NOT called on subsequent browser startups!)
virtual void SyncStarted(syncer::ModelType /* model_type */) {}

// Called after call to
// `AutofillWebDataServiceObserverOnDBSequence::AutofillProfileChanged` on UI
// sequence on any incremental updates when sync has been running and the
// changes have been committed to DB.
// Note, there is a possibility that PDM::Refresh is not finished yet, thus
// cleanups are run on stale data, due to asynchronous calls.
// TODO(crbug.com/1477292): Should also be called for `model_type` related to
// payments.
virtual void OnSyncUpdatesReceived(syncer::ModelType model_type) {}

protected:
virtual ~AutofillWebDataServiceObserverOnUISequence() {}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ absl::optional<syncer::ModelError> ContactInfoSyncBridge::MergeFullSyncData(
std::move(entity_data))) {
return error;
}
web_data_backend_->NotifyThatSyncHasStarted(syncer::CONTACT_INFO);
return absl::nullopt;
}

Expand Down Expand Up @@ -134,8 +133,6 @@ ContactInfoSyncBridge::ApplyIncrementalSyncChanges(
// currently doesn't provide a way to detect such cases, we don't distinguish.
if (!entity_changes.empty())
web_data_backend_->NotifyOfMultipleAutofillChanges(syncer::CONTACT_INFO);

web_data_backend_->NotifyOnSyncUpdatesReceived(syncer::CONTACT_INFO);
return absl::nullopt;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ TEST_F(ContactInfoSyncBridgeTest, MergeFullSyncData) {
EXPECT_CALL(mock_processor(), Delete).Times(0);
EXPECT_CALL(backend(), CommitChanges);
EXPECT_CALL(backend(), NotifyOfMultipleAutofillChanges);
EXPECT_CALL(backend(), NotifyThatSyncHasStarted(syncer::CONTACT_INFO));

EXPECT_TRUE(StartSyncing({remote1, remote2}));

Expand Down Expand Up @@ -219,7 +218,6 @@ TEST_F(ContactInfoSyncBridgeTest, ApplyIncrementalSyncChanges) {
EXPECT_CALL(mock_processor(), Put).Times(0);
EXPECT_CALL(backend(), CommitChanges());
EXPECT_CALL(backend(), NotifyOfMultipleAutofillChanges);
EXPECT_CALL(backend(), NotifyOnSyncUpdatesReceived(syncer::CONTACT_INFO));

// `ApplyIncrementalSyncChanges()` returns an error if it fails.
EXPECT_FALSE(bridge().ApplyIncrementalSyncChanges(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,6 @@ class MockAutofillWebDataBackend : public AutofillWebDataBackend {
(syncer::ModelType model_type),
(override));
MOCK_METHOD(void, NotifyOfAddressConversionCompleted, (), (override));
MOCK_METHOD(void,
NotifyThatSyncHasStarted,
(syncer::ModelType model_type),
(override));
MOCK_METHOD(void,
NotifyOnSyncUpdatesReceived,
(syncer::ModelType model_type),
(override));
};

} // namespace autofill
Expand Down

0 comments on commit 8cb1a8d

Please sign in to comment.