Skip to content

Commit

Permalink
[VCN] Add image usage in VCN flow
Browse files Browse the repository at this point in the history 
Made some explorations and found the image fetcher works pretty well
without a disk cache in the AutofillTable.

listed three options https://docs.google.com/document/d/1tSz7puztdgn-AJExKg0RRu4yNfelqw0-024toE9L3RY/edit?resourcekey=0-kpj35lGmSEWOImPeDFjbMw

Now we are actually preferring option 2, which gets rid of the Autofill
Table storage completely. This would save much integration work and
will avoid unnecessary decoding of the image.

Previously submitted CL crrev.com/c/3048555 was reverted so figured
this would be a good time to redo the work.

Bug: 1196021
Change-Id: I7d1da29b39c43eee2b270295fdbda0ffb0950d67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054839
Reviewed-by: Jared Saul <jsaul@google.com>
Reviewed-by: Siddharth Shah <siashah@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Siyu An <siyua@chromium.org>
Cr-Commit-Position: refs/heads/master@{#906880}
  • Loading branch information
Siyu An authored and Chromium LUCI CQ committed Jul 29, 2021
1 parent 6f6d9c7 commit 57cc98b
Show file tree
Hide file tree
Showing 17 changed files with 277 additions and 210 deletions.
14 changes: 9 additions & 5 deletions components/autofill/core/browser/autofill_suggestion_generator.cc
View file
Expand Up @@ -176,16 +176,17 @@ Suggestion AutofillSuggestionGenerator::CreateCreditCardSuggestion(
suggestion.match = prefix_matched_suggestion ? Suggestion::PREFIX_MATCH
: Suggestion::SUBSTRING_MATCH;

std::string server_id_for_virtual_card_option;
GURL card_art_url_for_virtual_card_option;
if (virtual_card_option &&
credit_card.record_type() == CreditCard::MASKED_SERVER_CARD) {
server_id_for_virtual_card_option = credit_card.server_id();
card_art_url_for_virtual_card_option = credit_card.card_art_url();
} else if (virtual_card_option &&
credit_card.record_type() == CreditCard::LOCAL_CARD) {
const CreditCard* server_duplicate_card =
GetServerCardForLocalCard(&credit_card);
DCHECK(server_duplicate_card);
server_id_for_virtual_card_option = server_duplicate_card->server_id();
card_art_url_for_virtual_card_option =
server_duplicate_card->card_art_url();
backend_id = server_duplicate_card->guid();
}
suggestion.backend_id = backend_id;
Expand Down Expand Up @@ -251,9 +252,12 @@ Suggestion AutofillSuggestionGenerator::CreateCreditCardSuggestion(
feature_engagement::kIPHKeyboardAccessoryPaymentVirtualCardFeature.name;
#endif // OS_ANDROID

// TODO(crbug.com/1196021): Populate custom_icon with card art if available
// (use server_id_for_virtual_card_option).
suggestion.frontend_id = POPUP_ITEM_ID_VIRTUAL_CREDIT_CARD_ENTRY;

gfx::Image* image = personal_data_->GetCreditCardArtImageForUrl(
card_art_url_for_virtual_card_option);
if (image)
suggestion.custom_icon = *image;
}

return suggestion;
Expand Down
7 changes: 4 additions & 3 deletions components/autofill/core/browser/browser_autofill_manager.cc
View file
Expand Up @@ -1484,9 +1484,10 @@ void BrowserAutofillManager::OnCreditCardFetched(CreditCardFetchResult result,
// If synced down card is a virtual card, let the client know so that it can
// show the UI to help user to manually fill the form, if needed.
if (credit_card->record_type() == CreditCard::VIRTUAL_CARD) {
// TODO(crbug.com/1196021): Pass in real card image.
client()->OnVirtualCardDataAvailable(credit_card, cvc,
/*card_image=*/gfx::Image());
gfx::Image* card_art_image = personal_data_->GetCreditCardArtImageForUrl(
credit_card->card_art_url());
client()->OnVirtualCardDataAvailable(
credit_card, cvc, card_art_image ? *card_art_image : gfx::Image());
}

FillCreditCardForm(credit_card_query_id_, credit_card_form_,
Expand Down
9 changes: 2 additions & 7 deletions components/autofill/core/browser/data_model/credit_card_art_image.cc
View file
Expand Up @@ -8,13 +8,8 @@ namespace autofill {

CreditCardArtImage::CreditCardArtImage() = default;

CreditCardArtImage::CreditCardArtImage(const std::string& id,
int64_t instrument_id,
std::vector<uint8_t> card_art_image) {
this->id = id;
this->instrument_id = instrument_id;
this->card_art_image = std::move(card_art_image);
}
CreditCardArtImage::CreditCardArtImage(const CreditCardArtImage& other) =
default;

CreditCardArtImage::~CreditCardArtImage() = default;

Expand Down
23 changes: 8 additions & 15 deletions components/autofill/core/browser/data_model/credit_card_art_image.h
View file
Expand Up @@ -5,30 +5,23 @@
#ifndef COMPONENTS_AUTOFILL_CORE_BROWSER_DATA_MODEL_CREDIT_CARD_ART_IMAGE_H_
#define COMPONENTS_AUTOFILL_CORE_BROWSER_DATA_MODEL_CREDIT_CARD_ART_IMAGE_H_

#include <string>
#include <vector>
#include "ui/gfx/image/image.h"
#include "url/gurl.h"

namespace autofill {

// Represents an rich card art image for the server credit card.
// Represents an rich card art image for the card art url.
struct CreditCardArtImage {
public:
CreditCardArtImage();
CreditCardArtImage(const std::string& id,
int64_t instrument_id,
std::vector<uint8_t> card_art_image);
CreditCardArtImage(const CreditCardArtImage& other);
~CreditCardArtImage();
CreditCardArtImage(const CreditCardArtImage&) = delete;
CreditCardArtImage& operator=(const CreditCardArtImage&) = delete;

// The server id for the related credit card.
std::string id;
// The url to fetch the card art image.
GURL card_art_url;

// The instrument id for the related credit card.
int64_t instrument_id;

// The customized card art image. Stored as an raw PNG-encoded data.
std::vector<uint8_t> card_art_image;
// The customized card art image.
gfx::Image card_art_image;
};

} // namespace autofill
Expand Down
73 changes: 71 additions & 2 deletions components/autofill/core/browser/personal_data_manager.cc
View file
Expand Up @@ -34,6 +34,7 @@
#include "components/autofill/core/browser/autofill_metrics.h"
#include "components/autofill/core/browser/autofill_profile_save_strike_database.h"
#include "components/autofill/core/browser/data_model/autofill_profile_comparator.h"
#include "components/autofill/core/browser/data_model/credit_card_art_image.h"
#include "components/autofill/core/browser/data_model/phone_number.h"
#include "components/autofill/core/browser/form_structure.h"
#include "components/autofill/core/browser/geo/address_i18n.h"
Expand Down Expand Up @@ -152,6 +153,9 @@ class PersonalDatabaseHelper
explicit PersonalDatabaseHelper(PersonalDataManager* personal_data_manager)
: personal_data_manager_(personal_data_manager) {}

PersonalDatabaseHelper(const PersonalDatabaseHelper&) = delete;
PersonalDatabaseHelper& operator=(const PersonalDatabaseHelper&) = delete;

void Init(scoped_refptr<AutofillWebDataService> profile_database,
scoped_refptr<AutofillWebDataService> account_database) {
profile_database_ = profile_database;
Expand Down Expand Up @@ -252,8 +256,6 @@ class PersonalDatabaseHelper
scoped_refptr<AutofillWebDataService> server_database_;

PersonalDataManager* personal_data_manager_;

DISALLOW_COPY_AND_ASSIGN(PersonalDatabaseHelper);
};

PersonalDataManager::PersonalDataManager(
Expand Down Expand Up @@ -478,6 +480,7 @@ void PersonalDataManager::OnWebDataServiceRequestDone(
ReceiveLoadedDbValues(h, result.get(),
&pending_server_creditcards_query_,
&server_credit_cards_);
OnServerCreditCardsRefreshed();
}
break;
case AUTOFILL_CLOUDTOKEN_RESULT:
Expand Down Expand Up @@ -961,6 +964,7 @@ void PersonalDataManager::ClearAllServerData() {
payments_customer_data_.reset();
server_credit_card_cloud_token_data_.clear();
autofill_offer_data_.clear();
credit_card_art_images_.clear();
}

void PersonalDataManager::ClearAllLocalData() {
Expand Down Expand Up @@ -1227,6 +1231,27 @@ std::vector<AutofillOfferData*> PersonalDataManager::GetAutofillOffers() const {
return result;
}

gfx::Image* PersonalDataManager::GetCreditCardArtImageForUrl(
const GURL& card_art_url) const {
if (!IsAutofillWalletImportEnabled())
return nullptr;

if (!card_art_url.is_valid())
return nullptr;

auto images_iterator = credit_card_art_images_.find(card_art_url);

// Found an image and return it.
if (images_iterator != credit_card_art_images_.end()) {
gfx::Image* image = images_iterator->second.get();
if (!image->IsEmpty())
return image;
}

FetchImagesForUrls({card_art_url});
return nullptr;
}

void PersonalDataManager::Refresh() {
LoadProfiles();
LoadCreditCards();
Expand Down Expand Up @@ -1504,6 +1529,16 @@ const ProfileValidityMap& PersonalDataManager::GetProfileValidityByGUID(
return empty_validity_map;
}

void PersonalDataManager::FetchImagesForUrls(
const std::vector<GURL>& updated_urls) const {
if (!image_fetcher_)
return;

image_fetcher_->FetchImagesForUrls(
updated_urls, base::BindOnce(&PersonalDataManager::OnCardArtImagesFetched,
weak_factory_.GetWeakPtr()));
}

const std::string& PersonalDataManager::GetDefaultCountryCodeForNewAddress()
const {
if (default_country_code_.empty())
Expand Down Expand Up @@ -2099,6 +2134,16 @@ void PersonalDataManager::OnAutofillProfileChanged(
OnProfileChangeDone(guid);
}

void PersonalDataManager::OnCardArtImagesFetched(
std::vector<std::unique_ptr<CreditCardArtImage>> art_images) {
for (auto& art_image : art_images) {
if (!art_image->card_art_image.IsEmpty()) {
credit_card_art_images_[art_image->card_art_url] =
std::make_unique<gfx::Image>(art_image->card_art_image);
}
}
}

void PersonalDataManager::LogServerCardLinkClicked() const {
AutofillMetrics::LogServerCardLinkClicked(GetSyncSigninState());
}
Expand Down Expand Up @@ -2375,4 +2420,28 @@ scoped_refptr<AutofillWebDataService> PersonalDataManager::GetLocalDatabase() {
return database_helper_->GetLocalDatabase();
}

void PersonalDataManager::OnServerCreditCardsRefreshed() {
ProcessVirtualCardMetadataChanges();
}

void PersonalDataManager::ProcessVirtualCardMetadataChanges() {
std::vector<GURL> updated_urls;
for (auto& card : server_credit_cards_) {
// If this card is not enrolled for virtual cards or the url is not valid,
// continue.
if (card->virtual_card_enrollment_state() != CreditCard::ENROLLED ||
!card->card_art_url().is_valid()) {
continue;
}

// Otherwise try to find the old entry with the same url.
auto it = credit_card_art_images_.find(card->card_art_url());
// No existing entry found.
if (it == credit_card_art_images_.end())
updated_urls.emplace_back(card->card_art_url());
}
if (!updated_urls.empty())
FetchImagesForUrls(updated_urls);
}

} // namespace autofill
30 changes: 27 additions & 3 deletions components/autofill/core/browser/personal_data_manager.h
View file
Expand Up @@ -15,7 +15,6 @@
#include <vector>

#include "base/gtest_prod_util.h"
#include "base/macros.h"
#include "base/observer_list.h"
#include "base/scoped_observation.h"
#include "build/build_config.h"
Expand Down Expand Up @@ -55,6 +54,7 @@ class RemoveAutofillTester;
namespace autofill {
class AutofillImageFetcher;
class AutofillInteractiveTest;
struct CreditCardArtImage;
class PersonalDataManagerObserver;
class PersonalDataManagerFactory;
class PersonalDatabaseHelper;
Expand Down Expand Up @@ -85,6 +85,8 @@ class PersonalDataManager : public KeyedService,
explicit PersonalDataManager(const std::string& app_locale);
PersonalDataManager(const std::string& app_locale,
const std::string& country_code);
PersonalDataManager(const PersonalDataManager&) = delete;
PersonalDataManager& operator=(const PersonalDataManager&) = delete;
~PersonalDataManager() override;

// Kicks off asynchronous loading of profiles and credit cards.
Expand Down Expand Up @@ -286,6 +288,10 @@ class PersonalDataManager : public KeyedService,
// Returns autofill offer data, including card-linked and promo code offers.
virtual std::vector<AutofillOfferData*> GetAutofillOffers() const;

// Returns the customized credit card art image for the |card_art_url|.
virtual gfx::Image* GetCreditCardArtImageForUrl(
const GURL& card_art_url) const;

// Updates the validity states of |profiles| according to server validity map.
void UpdateProfilesServerValidityMapsIfNeeded(
const std::vector<AutofillProfile*>& profiles);
Expand Down Expand Up @@ -471,6 +477,8 @@ class PersonalDataManager : public KeyedService,
FRIEND_TEST_ALL_PREFIXES(PersonalDataManagerTest,
AddCreditCard_CrazyCharacters);
FRIEND_TEST_ALL_PREFIXES(PersonalDataManagerTest, AddCreditCard_Invalid);
FRIEND_TEST_ALL_PREFIXES(PersonalDataManagerTest,
AddAndGetCreditCardArtImage);
FRIEND_TEST_ALL_PREFIXES(PersonalDataManagerTest,
DedupeProfiles_ProfilesToDelete);
FRIEND_TEST_ALL_PREFIXES(PersonalDataManagerTest,
Expand Down Expand Up @@ -629,6 +637,9 @@ class PersonalDataManager : public KeyedService,
// Get the profiles fields validity map by |guid|.
const ProfileValidityMap& GetProfileValidityByGUID(const std::string& guid);

// Asks AutofillImageFetcher to fetch images.
virtual void FetchImagesForUrls(const std::vector<GURL>& updated_urls) const;

// Decides which database type to use for server and local cards.
std::unique_ptr<PersonalDatabaseHelper> database_helper_;

Expand Down Expand Up @@ -659,6 +670,9 @@ class PersonalDataManager : public KeyedService,
// Offer data for user's credit cards.
std::vector<std::unique_ptr<AutofillOfferData>> autofill_offer_data_;

// The customized card art images for the URL.
std::map<GURL, std::unique_ptr<gfx::Image>> credit_card_art_images_;

// When the manager makes a request from WebDataServiceBase, the database
// is queried on another sequence, we record the query handle until we
// get called back. We store handles for both profile and credit card queries
Expand Down Expand Up @@ -747,6 +761,11 @@ class PersonalDataManager : public KeyedService,
// Triggered when a profile is added/updated/removed on db.
void OnAutofillProfileChanged(const AutofillProfileDeepChange& change);

// Triggered when all the card art image fetches have been completed,
// regardless of whether all of them succeeded.
void OnCardArtImagesFetched(
std::vector<std::unique_ptr<CreditCardArtImage>> art_images);

// Look at the next profile change for profile with guid = |guid|, and handle
// it.
void HandleNextProfileChange(const std::string& guid);
Expand Down Expand Up @@ -774,6 +793,13 @@ class PersonalDataManager : public KeyedService,
// Returns the database that is used for storing local data.
scoped_refptr<AutofillWebDataService> GetLocalDatabase();

// Invoked when server credit card cache is refreshed.
void OnServerCreditCardsRefreshed();

// Checks whether any virtual card metadata for server cards is new and makes
// corresponding changes.
void ProcessVirtualCardMetadataChanges();

// Stores the |app_locale| supplied on construction.
const std::string app_locale_;

Expand Down Expand Up @@ -860,8 +886,6 @@ class PersonalDataManager : public KeyedService,
history_service_observation_{this};

base::WeakPtrFactory<PersonalDataManager> weak_factory_{this};

DISALLOW_COPY_AND_ASSIGN(PersonalDataManager);
};

} // namespace autofill
Expand Down

0 comments on commit 57cc98b

Please sign in to comment.