Commit 48246c46 authored by siyua's avatar siyua Committed by Commit Bot

[Autofill] Added browser tests for local card migration flow (part 1)

The previous browser tests CL got reverted twice due to flaky tests,
so I will try to break the browser tests into several CLs. Listed the
following tests to be added in the TODO in the browser tests file.


Bug: 897998
Change-Id: Ib3c99de27028530e2f8bce7b616d7d5a9dae4cd3
Reviewed-on: https://chromium-review.googlesource.com/c/1364695
Commit-Queue: Siyu An <siyua@chromium.org>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Reviewed-by: default avatarJared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#625435}
parent 6f4e2c0b
...@@ -8,17 +8,21 @@ ...@@ -8,17 +8,21 @@
#include "components/autofill/core/browser/field_types.h" #include "components/autofill/core/browser/field_types.h"
// This defines an enumeration of IDs that can uniquely identify a view within // This defines an enumeration of IDs that can uniquely identify a view within
// the scope of the local and upload credit card save bubbles. // the scope of the local and upload credit card save bubbles as well as the
// local card migration bubble and dialogs.
namespace autofill { namespace autofill {
enum DialogViewId : int { enum DialogViewId : int {
VIEW_ID_NONE = 0, VIEW_ID_NONE = 0,
// The following are the important containing views of the bubble. // The following views are contained in SaveCardBubbleViews.
MAIN_CONTENT_VIEW_LOCAL, // The main content view, for a local save bubble MAIN_CONTENT_VIEW_LOCAL, // The main content view for a local
MAIN_CONTENT_VIEW_UPLOAD, // The main content view, for an upload save bubble // save bubble
FOOTNOTE_VIEW, // Contains the legal messages for upload save MAIN_CONTENT_VIEW_UPLOAD, // The main content view for an upload
// save bubble
FOOTNOTE_VIEW, // The footnote view of either an upload
// save bubble or a manage cards view.
SIGN_IN_PROMO_VIEW, // Contains the sign-in promo view SIGN_IN_PROMO_VIEW, // Contains the sign-in promo view
MANAGE_CARDS_VIEW, // The manage cards view MANAGE_CARDS_VIEW, // The manage cards view
EXPIRATION_DATE_VIEW, // Contains the dropdowns for expiration date EXPIRATION_DATE_VIEW, // Contains the dropdowns for expiration date
...@@ -26,6 +30,12 @@ enum DialogViewId : int { ...@@ -26,6 +30,12 @@ enum DialogViewId : int {
// The sub-view that contains the sign-in button in the promo. // The sub-view that contains the sign-in button in the promo.
SIGN_IN_VIEW, SIGN_IN_VIEW,
// The main content view for a migration offer bubble.
MAIN_CONTENT_VIEW_MIGRATION_BUBBLE,
// The main content view for the main migration dialog.
MAIN_CONTENT_VIEW_MIGRATION_OFFER_DIALOG,
// The following are views::LabelButton objects (clickable). // The following are views::LabelButton objects (clickable).
OK_BUTTON, // Can say [Save], [Next], [Confirm], OK_BUTTON, // Can say [Save], [Next], [Confirm],
// or [Done] depending on context // or [Done] depending on context
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <stddef.h> #include <stddef.h>
#include <memory> #include <memory>
#include <utility>
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "build/build_config.h" #include "build/build_config.h"
...@@ -29,6 +30,7 @@ ...@@ -29,6 +30,7 @@
#include "ui/views/layout/box_layout.h" #include "ui/views/layout/box_layout.h"
#include "ui/views/layout/fill_layout.h" #include "ui/views/layout/fill_layout.h"
#include "ui/views/style/typography.h" #include "ui/views/style/typography.h"
#include "ui/views/window/dialog_client_view.h"
namespace autofill { namespace autofill {
...@@ -154,6 +156,7 @@ void LocalCardMigrationBubbleViews::Init() { ...@@ -154,6 +156,7 @@ void LocalCardMigrationBubbleViews::Init() {
explanatory_message->SetHorizontalAlignment(gfx::ALIGN_LEFT); explanatory_message->SetHorizontalAlignment(gfx::ALIGN_LEFT);
explanatory_message->SetMultiLine(true); explanatory_message->SetMultiLine(true);
AddChildView(explanatory_message); AddChildView(explanatory_message);
set_id(DialogViewId::MAIN_CONTENT_VIEW_MIGRATION_BUBBLE);
} }
} // namespace autofill } // namespace autofill
...@@ -47,6 +47,8 @@ class LocalCardMigrationBubbleViews : public LocalCardMigrationBubble, ...@@ -47,6 +47,8 @@ class LocalCardMigrationBubbleViews : public LocalCardMigrationBubble,
void WindowClosing() override; void WindowClosing() override;
private: private:
friend class LocalCardMigrationBrowserTest;
~LocalCardMigrationBubbleViews() override; ~LocalCardMigrationBubbleViews() override;
// views::BubbleDialogDelegateView: // views::BubbleDialogDelegateView:
......
...@@ -4,6 +4,10 @@ ...@@ -4,6 +4,10 @@
#include "chrome/browser/ui/views/autofill/local_card_migration_dialog_view.h" #include "chrome/browser/ui/views/autofill/local_card_migration_dialog_view.h"
#include <memory>
#include <string>
#include <vector>
#include "base/i18n/message_formatter.h" #include "base/i18n/message_formatter.h"
#include "base/location.h" #include "base/location.h"
#include "base/macros.h" #include "base/macros.h"
...@@ -311,6 +315,8 @@ class LocalCardMigrationOfferView : public views::View, ...@@ -311,6 +315,8 @@ class LocalCardMigrationOfferView : public views::View,
} }
private: private:
friend class LocalCardMigrationDialogView;
LocalCardMigrationDialogController* controller_; LocalCardMigrationDialogController* controller_;
views::View* card_list_view_ = nullptr; views::View* card_list_view_ = nullptr;
...@@ -467,6 +473,8 @@ void LocalCardMigrationDialogView::ConstructView() { ...@@ -467,6 +473,8 @@ void LocalCardMigrationDialogView::ConstructView() {
if (view_state == LocalCardMigrationDialogState::kOffered) { if (view_state == LocalCardMigrationDialogState::kOffered) {
offer_view_ = new LocalCardMigrationOfferView(controller_, this); offer_view_ = new LocalCardMigrationOfferView(controller_, this);
offer_view_->set_id(DialogViewId::MAIN_CONTENT_VIEW_MIGRATION_OFFER_DIALOG);
card_list_view_ = offer_view_->card_list_view_;
AddChildView(offer_view_); AddChildView(offer_view_);
} else { } else {
AddChildView(CreateFeedbackContentView(controller_, this).release()); AddChildView(CreateFeedbackContentView(controller_, this).release());
......
...@@ -51,7 +51,10 @@ class LocalCardMigrationDialogView : public LocalCardMigrationDialog, ...@@ -51,7 +51,10 @@ class LocalCardMigrationDialogView : public LocalCardMigrationDialog,
void UpdateLayout(); void UpdateLayout();
private: private:
friend class LocalCardMigrationBrowserTest;
void ConstructView(); void ConstructView();
base::string16 GetOkButtonLabel() const; base::string16 GetOkButtonLabel() const;
base::string16 GetCancelButtonLabel() const; base::string16 GetCancelButtonLabel() const;
...@@ -63,6 +66,10 @@ class LocalCardMigrationDialogView : public LocalCardMigrationDialog, ...@@ -63,6 +66,10 @@ class LocalCardMigrationDialogView : public LocalCardMigrationDialog,
// dialog is not in the 'offer' state. // dialog is not in the 'offer' state.
LocalCardMigrationOfferView* offer_view_ = nullptr; LocalCardMigrationOfferView* offer_view_ = nullptr;
// The view containing a list of cards. It is the content of the scroll bar.
// Owned by the LocalCardMigrationOfferView.
views::View* card_list_view_;
DISALLOW_COPY_AND_ASSIGN(LocalCardMigrationDialogView); DISALLOW_COPY_AND_ASSIGN(LocalCardMigrationDialogView);
}; };
......
...@@ -1539,6 +1539,7 @@ test("browser_tests") { ...@@ -1539,6 +1539,7 @@ test("browser_tests") {
"../browser/ui/views/autofill/autofill_popup_base_view_browsertest.cc", "../browser/ui/views/autofill/autofill_popup_base_view_browsertest.cc",
"../browser/ui/views/autofill/card_unmask_prompt_view_tester_views.cc", "../browser/ui/views/autofill/card_unmask_prompt_view_tester_views.cc",
"../browser/ui/views/autofill/card_unmask_prompt_view_tester_views.h", "../browser/ui/views/autofill/card_unmask_prompt_view_tester_views.h",
"../browser/ui/views/autofill/local_card_migration_browsertest.cc",
"../browser/ui/views/autofill/save_card_bubble_views_browsertest.cc", "../browser/ui/views/autofill/save_card_bubble_views_browsertest.cc",
"../browser/ui/views/autofill/save_card_bubble_views_browsertest_base.cc", "../browser/ui/views/autofill/save_card_bubble_views_browsertest_base.cc",
"../browser/ui/views/autofill/save_card_bubble_views_browsertest_base.h", "../browser/ui/views/autofill/save_card_bubble_views_browsertest_base.h",
......
...@@ -124,6 +124,7 @@ class CreditCardSaveManager { ...@@ -124,6 +124,7 @@ class CreditCardSaveManager {
private: private:
friend class CreditCardSaveManagerTest; friend class CreditCardSaveManagerTest;
friend class CreditCardSaveManagerTestObserverBridge; friend class CreditCardSaveManagerTestObserverBridge;
friend class LocalCardMigrationBrowserTest;
friend class TestCreditCardSaveManager; friend class TestCreditCardSaveManager;
friend class SaveCardBubbleViewsBrowserTestBase; friend class SaveCardBubbleViewsBrowserTestBase;
......
...@@ -145,6 +145,7 @@ class FormDataImporter { ...@@ -145,6 +145,7 @@ class FormDataImporter {
friend class AutofillMergeTest; friend class AutofillMergeTest;
friend class FormDataImporterTest; friend class FormDataImporterTest;
friend class FormDataImporterTestBase; friend class FormDataImporterTestBase;
friend class LocalCardMigrationBrowserTest;
friend class SaveCardBubbleViewsBrowserTestBase; friend class SaveCardBubbleViewsBrowserTestBase;
friend class SaveCardInfobarEGTestHelper; friend class SaveCardInfobarEGTestHelper;
FRIEND_TEST_ALL_PREFIXES(AutofillMergeTest, MergeProfiles); FRIEND_TEST_ALL_PREFIXES(AutofillMergeTest, MergeProfiles);
......
...@@ -88,6 +88,9 @@ void LocalCardMigrationManager::AttemptToOfferLocalCardMigration( ...@@ -88,6 +88,9 @@ void LocalCardMigrationManager::AttemptToOfferLocalCardMigration(
return; return;
migration_request_ = payments::PaymentsClient::MigrationRequestDetails(); migration_request_ = payments::PaymentsClient::MigrationRequestDetails();
if (observer_for_testing_)
observer_for_testing_->OnDecideToRequestLocalCardMigration();
payments_client_->GetUploadDetails( payments_client_->GetUploadDetails(
std::vector<AutofillProfile>(), GetDetectedValues(), std::vector<AutofillProfile>(), GetDetectedValues(),
/*active_experiments=*/std::vector<const char*>(), app_locale_, /*active_experiments=*/std::vector<const char*>(), app_locale_,
...@@ -145,15 +148,25 @@ bool LocalCardMigrationManager::IsCreditCardMigrationEnabled() { ...@@ -145,15 +148,25 @@ bool LocalCardMigrationManager::IsCreditCardMigrationEnabled() {
bool migration_experiment_enabled = bool migration_experiment_enabled =
features::GetLocalCardMigrationExperimentalFlag() != features::GetLocalCardMigrationExperimentalFlag() !=
features::LocalCardMigrationExperimentalFlag::kMigrationDisabled; features::LocalCardMigrationExperimentalFlag::kMigrationDisabled;
bool credit_card_upload_enabled = ::autofill::IsCreditCardUploadEnabled(
// If |observer_for_testing_| is set, assume we are in a browsertest and
// credit card upload should be enabled by default. Cannot get around this as
// Chrome OS testing requires an unsupported email domain (i.e.
// stub-user@example.com).
bool credit_card_upload_enabled =
observer_for_testing_ ||
::autofill::IsCreditCardUploadEnabled(
client_->GetPrefs(), client_->GetSyncService(), client_->GetPrefs(), client_->GetSyncService(),
client_->GetIdentityManager()->GetPrimaryAccountInfo().email); client_->GetIdentityManager()->GetPrimaryAccountInfo().email);
bool has_google_payments_account = bool has_google_payments_account =
(payments::GetBillingCustomerId(personal_data_manager_, (payments::GetBillingCustomerId(personal_data_manager_,
payments_client_->GetPrefService()) != 0); payments_client_->GetPrefService()) != 0);
bool sync_feature_enabled = bool sync_feature_enabled =
(personal_data_manager_->GetSyncSigninState() == (personal_data_manager_->GetSyncSigninState() ==
AutofillSyncSigninState::kSignedInAndSyncFeature); AutofillSyncSigninState::kSignedInAndSyncFeature);
return migration_experiment_enabled && credit_card_upload_enabled && return migration_experiment_enabled && credit_card_upload_enabled &&
has_google_payments_account && sync_feature_enabled; has_google_payments_account && sync_feature_enabled;
} }
...@@ -163,6 +176,9 @@ void LocalCardMigrationManager::OnDidGetUploadDetails( ...@@ -163,6 +176,9 @@ void LocalCardMigrationManager::OnDidGetUploadDetails(
AutofillClient::PaymentsRpcResult result, AutofillClient::PaymentsRpcResult result,
const base::string16& context_token, const base::string16& context_token,
std::unique_ptr<base::Value> legal_message) { std::unique_ptr<base::Value> legal_message) {
if (observer_for_testing_)
observer_for_testing_->OnReceivedGetUploadDetailsResponse();
if (result == AutofillClient::SUCCESS) { if (result == AutofillClient::SUCCESS) {
migration_request_.context_token = context_token; migration_request_.context_token = context_token;
legal_message_ = base::DictionaryValue::From(std::move(legal_message)); legal_message_ = base::DictionaryValue::From(std::move(legal_message));
...@@ -196,6 +212,9 @@ void LocalCardMigrationManager::OnDidMigrateLocalCards( ...@@ -196,6 +212,9 @@ void LocalCardMigrationManager::OnDidMigrateLocalCards(
AutofillClient::PaymentsRpcResult result, AutofillClient::PaymentsRpcResult result,
std::unique_ptr<std::unordered_map<std::string, std::string>> save_result, std::unique_ptr<std::unordered_map<std::string, std::string>> save_result,
const std::string& display_text) { const std::string& display_text) {
if (observer_for_testing_)
observer_for_testing_->OnReceivedMigrateCardsResponse();
if (!save_result) if (!save_result)
return; return;
...@@ -254,6 +273,9 @@ void LocalCardMigrationManager::OnDidGetMigrationRiskData( ...@@ -254,6 +273,9 @@ void LocalCardMigrationManager::OnDidGetMigrationRiskData(
// Send the migration request. Will call payments_client to create a new // Send the migration request. Will call payments_client to create a new
// PaymentsRequest. Also create a new callback function OnDidMigrateLocalCards. // PaymentsRequest. Also create a new callback function OnDidMigrateLocalCards.
void LocalCardMigrationManager::SendMigrateLocalCardsRequest() { void LocalCardMigrationManager::SendMigrateLocalCardsRequest() {
if (observer_for_testing_)
observer_for_testing_->OnSentMigrateCardsRequest();
migration_request_.app_locale = app_locale_; migration_request_.app_locale = app_locale_;
migration_request_.billing_customer_number = payments::GetBillingCustomerId( migration_request_.billing_customer_number = payments::GetBillingCustomerId(
personal_data_manager_, payments_client_->GetPrefService()); personal_data_manager_, payments_client_->GetPrefService());
......
...@@ -43,7 +43,7 @@ class MigratableCreditCard { ...@@ -43,7 +43,7 @@ class MigratableCreditCard {
FAILURE_ON_UPLOAD, FAILURE_ON_UPLOAD,
}; };
MigratableCreditCard(const CreditCard& credit_card); explicit MigratableCreditCard(const CreditCard& credit_card);
~MigratableCreditCard(); ~MigratableCreditCard();
CreditCard credit_card() const { return credit_card_; } CreditCard credit_card() const { return credit_card_; }
...@@ -66,6 +66,16 @@ class MigratableCreditCard { ...@@ -66,6 +66,16 @@ class MigratableCreditCard {
// Owned by FormDataImporter. // Owned by FormDataImporter.
class LocalCardMigrationManager { class LocalCardMigrationManager {
public: public:
// An observer class used by browsertests that gets notified whenever
// particular actions occur.
class ObserverForTest {
public:
virtual void OnDecideToRequestLocalCardMigration() = 0;
virtual void OnReceivedGetUploadDetailsResponse() = 0;
virtual void OnSentMigrateCardsRequest() = 0;
virtual void OnReceivedMigrateCardsResponse() = 0;
};
// The parameters should outlive the LocalCardMigrationManager. // The parameters should outlive the LocalCardMigrationManager.
LocalCardMigrationManager(AutofillClient* client, LocalCardMigrationManager(AutofillClient* client,
payments::PaymentsClient* payments_client, payments::PaymentsClient* payments_client,
...@@ -141,6 +151,7 @@ class LocalCardMigrationManager { ...@@ -141,6 +151,7 @@ class LocalCardMigrationManager {
payments::PaymentsClient* payments_client_; payments::PaymentsClient* payments_client_;
private: private:
friend class LocalCardMigrationBrowserTest;
FRIEND_TEST_ALL_PREFIXES(LocalCardMigrationManagerTest, FRIEND_TEST_ALL_PREFIXES(LocalCardMigrationManagerTest,
MigrateCreditCard_MigrationPermanentFailure); MigrateCreditCard_MigrationPermanentFailure);
FRIEND_TEST_ALL_PREFIXES(LocalCardMigrationManagerTest, FRIEND_TEST_ALL_PREFIXES(LocalCardMigrationManagerTest,
...@@ -161,6 +172,11 @@ class LocalCardMigrationManager { ...@@ -161,6 +172,11 @@ class LocalCardMigrationManager {
// Finalizes the migration request and calls PaymentsClient. // Finalizes the migration request and calls PaymentsClient.
void SendMigrateLocalCardsRequest(); void SendMigrateLocalCardsRequest();
// For testing.
void SetEventObserverForTesting(ObserverForTest* observer) {
observer_for_testing_ = observer;
}
std::unique_ptr<base::DictionaryValue> legal_message_; std::unique_ptr<base::DictionaryValue> legal_message_;
std::string app_locale_; std::string app_locale_;
...@@ -187,6 +203,9 @@ class LocalCardMigrationManager { ...@@ -187,6 +203,9 @@ class LocalCardMigrationManager {
// Record the triggering source of the local card migration. // Record the triggering source of the local card migration.
AutofillMetrics::LocalCardMigrationOrigin local_card_migration_origin_; AutofillMetrics::LocalCardMigrationOrigin local_card_migration_origin_;
// Initialized only during tests.
ObserverForTest* observer_for_testing_ = nullptr;
base::WeakPtrFactory<LocalCardMigrationManager> weak_ptr_factory_; base::WeakPtrFactory<LocalCardMigrationManager> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(LocalCardMigrationManager); DISALLOW_COPY_AND_ASSIGN(LocalCardMigrationManager);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment