Skip to content

Commit

Permalink
Clean up unused function in wallet sync
Browse files Browse the repository at this point in the history
Bug: 1249665
Change-Id: I678f9f8d2f31baa20cbb03a3f5737a01525b9218
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3547502
Reviewed-by: Matthias Körber <koerber@google.com>
Commit-Queue: Siyu An <siyua@chromium.org>
Cr-Commit-Position: refs/heads/main@{#985471}
  • Loading branch information
Siyu An authored and Chromium LUCI CQ committed Mar 25, 2022
1 parent 24262cd commit 084fc44
Show file tree
Hide file tree
Showing 9 changed files with 6 additions and 66 deletions.
Expand Up @@ -356,9 +356,9 @@ bool AutofillWalletSyncBridge::SetWalletCards(
ComputeAutofillWalletDiff(existing_cards, wallet_cards);

if (!diff.IsEmpty()) {
// Check if there is any update on cards' virtual card metadata. If so, make
// changes to the credit card art image table and log it.
ProcessVirtualCardMetadataChanges(existing_cards, wallet_cards);
// Check if there is any update on cards' virtual card metadata. If so log
// it.
LogVirtualCardMetadataChanges(existing_cards, wallet_cards);

table->SetServerCardsData(wallet_cards);

Expand Down Expand Up @@ -541,10 +541,9 @@ void AutofillWalletSyncBridge::LoadMetadata() {
change_processor()->ModelReadyToSync(std::move(batch));
}

void AutofillWalletSyncBridge::ProcessVirtualCardMetadataChanges(
void AutofillWalletSyncBridge::LogVirtualCardMetadataChanges(
const std::vector<std::unique_ptr<CreditCard>>& old_data,
const std::vector<CreditCard>& new_data) {
std::vector<std::string> updated_server_ids;
for (const CreditCard& new_card : new_data) {
// Try to find the old card with same server id.
auto old_data_iterator =
Expand All @@ -555,7 +554,6 @@ void AutofillWalletSyncBridge::ProcessVirtualCardMetadataChanges(

// No existing card with the same ID found.
if (old_data_iterator == old_data.end()) {
updated_server_ids.push_back(new_card.server_id());
// log the newly-synced card.
AutofillMetrics::LogVirtualCardMetadataSynced(/*existing_card=*/false);
continue;
Expand All @@ -566,15 +564,9 @@ void AutofillWalletSyncBridge::ProcessVirtualCardMetadataChanges(
if ((*old_data_iterator)->virtual_card_enrollment_state() !=
new_card.virtual_card_enrollment_state() ||
(*old_data_iterator)->card_art_url() != new_card.card_art_url()) {
updated_server_ids.push_back(new_card.server_id());
AutofillMetrics::LogVirtualCardMetadataSynced(/*existing_card=*/true);
}
}

// After traversing all the new cards, notify the observer the ids of card
// whose image should be updated.
if (!updated_server_ids.empty() && web_data_backend_)
web_data_backend_->NotifyOfCreditCardArtImagesChanged(updated_server_ids);
}

} // namespace autofill
Expand Up @@ -138,11 +138,8 @@ class AutofillWalletSyncBridge : public base::SupportsUserData::Data,
// processor so that it can start tracking changes.
void LoadMetadata();

// TODO(crbug/com/1196021): Clean up duplicate functions and use it for
// logging only.
// Checks whether any virtual card metadata for new_data is new and make
// corresponding changes.
void ProcessVirtualCardMetadataChanges(
// Logs virtual card metadata changes.
void LogVirtualCardMetadataChanges(
const std::vector<std::unique_ptr<CreditCard>>& old_data,
const std::vector<CreditCard>& new_data);

Expand Down
Expand Up @@ -1095,7 +1095,6 @@ TEST_F(AutofillWalletSyncBridgeTest, SetWalletCards_LogVirtualMetadataSynced) {

std::vector<std::string> server_ids = {"card2_server_id", "card3_server_id",
"card4_server_id"};
EXPECT_CALL(*backend(), NotifyOfCreditCardArtImagesChanged(server_ids));

// Trigger sync.
base::HistogramTester histogram_tester;
Expand Down
Expand Up @@ -67,13 +67,6 @@ class AutofillWebDataBackend {
// 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 the credit cards with
// |server_ids| have new card art images. NOTE: This method is intended to be
// called from the DB sequence. The UI sequence notifications are
// asynchronous.
virtual void NotifyOfCreditCardArtImagesChanged(
const std::vector<std::string>& server_ids) = 0;
};

} // namespace autofill
Expand Down
Expand Up @@ -75,13 +75,6 @@ void AutofillWebDataBackendImpl::SetAutofillProfileChangedCallback(
on_autofill_profile_changed_cb_ = std::move(change_cb);
}

void AutofillWebDataBackendImpl::SetCardArtImagesChangedCallback(
base::RepeatingCallback<void(const std::vector<std::string>&)>
on_card_art_image_change_callback) {
on_card_art_image_change_callback_ =
std::move(on_card_art_image_change_callback);
}

WebDatabase* AutofillWebDataBackendImpl::GetDatabase() {
DCHECK(owning_task_runner()->RunsTasksInCurrentSequence());
return web_database_backend_->database();
Expand Down Expand Up @@ -154,19 +147,6 @@ void AutofillWebDataBackendImpl::NotifyThatSyncHasStarted(
FROM_HERE, base::BindOnce(on_sync_started_callback_, model_type));
}

void AutofillWebDataBackendImpl::NotifyOfCreditCardArtImagesChanged(
const std::vector<std::string>& server_ids) {
DCHECK(owning_task_runner()->RunsTasksInCurrentSequence());

if (on_card_art_image_change_callback_.is_null())
return;

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

base::SupportsUserData* AutofillWebDataBackendImpl::GetDBUserData() {
DCHECK(owning_task_runner()->RunsTasksInCurrentSequence());
if (!user_data_)
Expand Down
Expand Up @@ -64,9 +64,6 @@ class AutofillWebDataBackendImpl
void SetAutofillProfileChangedCallback(
base::RepeatingCallback<void(const AutofillProfileDeepChange&)>
change_cb);
void SetCardArtImagesChangedCallback(
base::RepeatingCallback<void(const std::vector<std::string>&)>
on_card_art_image_change_callback);

// AutofillWebDataBackend implementation.
void AddObserver(
Expand All @@ -80,8 +77,6 @@ class AutofillWebDataBackendImpl
void NotifyOfMultipleAutofillChanges() override;
void NotifyOfAddressConversionCompleted() override;
void NotifyThatSyncHasStarted(syncer::ModelType model_type) override;
void NotifyOfCreditCardArtImagesChanged(
const std::vector<std::string>& server_ids) override;
void CommitChanges() override;

// Returns a SupportsUserData object that may be used to store data accessible
Expand Down Expand Up @@ -272,8 +267,6 @@ class AutofillWebDataBackendImpl
base::RepeatingCallback<void(syncer::ModelType)> on_sync_started_callback_;
base::RepeatingCallback<void(const AutofillProfileDeepChange&)>
on_autofill_profile_changed_cb_;
base::RepeatingCallback<void(const std::vector<std::string>&)>
on_card_art_image_change_callback_;
};

} // namespace autofill
Expand Down
Expand Up @@ -123,13 +123,6 @@ void AutofillWebDataService::SetAutofillProfileChangedCallback(
autofill_backend_->SetAutofillProfileChangedCallback(std::move(change_cb));
}

void AutofillWebDataService::SetCardArtImagesChangedCallback(
base::RepeatingCallback<void(const std::vector<std::string>&)>
on_card_art_image_change_callback) {
autofill_backend_->SetCardArtImagesChangedCallback(
std::move(on_card_art_image_change_callback));
}

void AutofillWebDataService::UpdateAutofillProfile(
const AutofillProfile& profile) {
wdbs_->ScheduleDBTask(
Expand Down
Expand Up @@ -113,9 +113,6 @@ class AutofillWebDataService : public WebDataServiceBase {
void SetAutofillProfileChangedCallback(
base::RepeatingCallback<void(const AutofillProfileDeepChange&)>
change_cb);
void SetCardArtImagesChangedCallback(
base::RepeatingCallback<void(const std::vector<std::string>&)>
on_card_art_image_change_callback);

// Schedules a task to add credit card to the web database.
void AddCreditCard(const CreditCard& credit_card);
Expand Down
Expand Up @@ -50,10 +50,6 @@ class MockAutofillWebDataBackend : public AutofillWebDataBackend {
NotifyThatSyncHasStarted,
(syncer::ModelType model_type),
(override));
MOCK_METHOD(void,
NotifyOfCreditCardArtImagesChanged,
(const std::vector<std::string>& server_ids),
(override));
};

} // namespace autofill
Expand Down

0 comments on commit 084fc44

Please sign in to comment.