Commit b54154ac authored by Sebastien Seguin-Gagnon's avatar Sebastien Seguin-Gagnon Committed by Commit Bot

Revert "Make SaveCardBubble browsertest use the sync state"

This reverts commit 995f8099.

Reason for revert: Breaks browser tests on continues builders (consistent failures) See crbug.com/908787

Original change's description:
> Make SaveCardBubble browsertest use the sync state
> 
> The browsertests used to rely on the save manager being hardcoded to
> upload to Google if the observer_for_testing was set. This makes it
> impossible to test that the logic for showing the upload prompt works
> properly.
> 
> BUG=906630, 859761
> 
> Change-Id: Id7352a869388aac4ff36e1284750e22ac68c91b3
> Reviewed-on: https://chromium-review.googlesource.com/c/1348335
> Commit-Queue: Florian Uunk <feuunk@chromium.org>
> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
> Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#610866}

TBR=vasilii@chromium.org,sebsg@chromium.org,feuunk@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 906630, 859761
Change-Id: I145d2b56295af6014b6a94c44beb38b1bf6b9e01
Reviewed-on: https://chromium-review.googlesource.com/c/1352680Reviewed-by: default avatarSebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611844}
parent 3ee1c1d5
...@@ -10,10 +10,11 @@ ...@@ -10,10 +10,11 @@
#include <utility> #include <utility>
#include "base/bind.h" #include "base/bind.h"
#include "chrome/browser/signin/identity_manager_factory.h" #include "chrome/browser/signin/account_fetcher_service_factory.h"
#include "chrome/browser/signin/account_tracker_service_factory.h"
#include "chrome/browser/signin/fake_account_fetcher_service_builder.h"
#include "chrome/browser/signin/fake_signin_manager_builder.h"
#include "chrome/browser/signin/signin_manager_factory.h" #include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
#include "chrome/browser/ui/autofill/chrome_autofill_client.h" #include "chrome/browser/ui/autofill/chrome_autofill_client.h"
#include "chrome/browser/ui/autofill/save_card_bubble_controller_impl.h" #include "chrome/browser/ui/autofill/save_card_bubble_controller_impl.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
...@@ -29,19 +30,14 @@ ...@@ -29,19 +30,14 @@
#include "components/autofill/core/browser/credit_card_save_manager.h" #include "components/autofill/core/browser/credit_card_save_manager.h"
#include "components/autofill/core/browser/form_data_importer.h" #include "components/autofill/core/browser/form_data_importer.h"
#include "components/autofill/core/browser/payments/payments_client.h" #include "components/autofill/core/browser/payments/payments_client.h"
#include "components/browser_sync/profile_sync_service.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/network_session_configurator/common/network_switches.h" #include "components/network_session_configurator/common/network_switches.h"
#include "components/signin/core/browser/account_tracker_service.h" #include "components/signin/core/browser/account_tracker_service.h"
#include "components/signin/core/browser/fake_account_fetcher_service.h" #include "components/signin/core/browser/fake_account_fetcher_service.h"
#include "components/sync/test/fake_server/fake_server.h"
#include "components/sync/test/fake_server/fake_server_network_resources.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h" #include "content/public/test/test_navigation_observer.h"
#include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/test_url_fetcher_factory.h"
#include "services/device/public/cpp/test/scoped_geolocation_overrider.h" #include "services/device/public/cpp/test/scoped_geolocation_overrider.h"
#include "services/identity/public/cpp/identity_manager.h"
#include "services/identity/public/cpp/identity_test_utils.h"
#include "ui/events/base_event_utils.h" #include "ui/events/base_event_utils.h"
#include "ui/views/bubble/bubble_frame_view.h" #include "ui/views/bubble/bubble_frame_view.h"
#include "ui/views/controls/button/button.h" #include "ui/views/controls/button/button.h"
...@@ -75,38 +71,21 @@ const double kFakeGeolocationLongitude = 4.56; ...@@ -75,38 +71,21 @@ const double kFakeGeolocationLongitude = 4.56;
SaveCardBubbleViewsBrowserTestBase::SaveCardBubbleViewsBrowserTestBase( SaveCardBubbleViewsBrowserTestBase::SaveCardBubbleViewsBrowserTestBase(
const std::string& test_file_path) const std::string& test_file_path)
: SyncTest(SINGLE_CLIENT), test_file_path_(test_file_path) {} : test_file_path_(test_file_path) {}
SaveCardBubbleViewsBrowserTestBase::~SaveCardBubbleViewsBrowserTestBase() {} SaveCardBubbleViewsBrowserTestBase::~SaveCardBubbleViewsBrowserTestBase() {}
void SaveCardBubbleViewsBrowserTestBase::SetUpOnMainThread() { void SaveCardBubbleViewsBrowserTestBase::SetUpOnMainThread() {
SyncTest::SetUpOnMainThread();
// Set up the HTTPS server (uses the embedded_test_server). // Set up the HTTPS server (uses the embedded_test_server).
ASSERT_TRUE(embedded_test_server()->InitializeAndListen()); ASSERT_TRUE(embedded_test_server()->InitializeAndListen());
embedded_test_server()->ServeFilesFromSourceDirectory( embedded_test_server()->ServeFilesFromSourceDirectory(
"components/test/data/autofill"); "components/test/data/autofill");
embedded_test_server()->StartAcceptingConnections(); embedded_test_server()->StartAcceptingConnections();
ProfileSyncServiceFactory::GetForProfile(browser()->profile()) // Set up the URL fetcher. By providing an Impl, unexpected calls (sync, etc.)
->OverrideNetworkResourcesForTest( // won't cause the test to crash.
std::make_unique<fake_server::FakeServerNetworkResources>( url_fetcher_factory_ = std::make_unique<net::FakeURLFetcherFactory>(
GetFakeServer()->AsWeakPtr())); new net::URLFetcherImplFactory());
std::string username;
#if defined(OS_CHROMEOS)
// In ChromeOS browser tests, the profile may already by authenticated with
// stub account |user_manager::kStubUserEmail|.
AccountInfo info = IdentityManagerFactory::GetForProfile(browser()->profile())
->GetPrimaryAccountInfo();
username = info.email;
#endif
if (username.empty())
username = "user@gmail.com";
harness_ = ProfileSyncServiceHarness::Create(
browser()->profile(), username, "password",
ProfileSyncServiceHarness::SigninType::FAKE_SIGNIN);
// Set up the URL loader factory for the payments client so we can intercept // Set up the URL loader factory for the payments client so we can intercept
// those network requests too. // those network requests too.
...@@ -133,6 +112,8 @@ void SaveCardBubbleViewsBrowserTestBase::SetUpOnMainThread() { ...@@ -133,6 +112,8 @@ void SaveCardBubbleViewsBrowserTestBase::SetUpOnMainThread() {
// Set up the fake geolocation data. // Set up the fake geolocation data.
geolocation_overrider_ = std::make_unique<device::ScopedGeolocationOverrider>( geolocation_overrider_ = std::make_unique<device::ScopedGeolocationOverrider>(
kFakeGeolocationLatitude, kFakeGeolocationLongitude); kFakeGeolocationLatitude, kFakeGeolocationLongitude);
NavigateTo(test_file_path_);
} }
void SaveCardBubbleViewsBrowserTestBase::NavigateTo( void SaveCardBubbleViewsBrowserTestBase::NavigateTo(
...@@ -190,13 +171,54 @@ void SaveCardBubbleViewsBrowserTestBase::OnSCBCStrikeChangeComplete() { ...@@ -190,13 +171,54 @@ void SaveCardBubbleViewsBrowserTestBase::OnSCBCStrikeChangeComplete() {
event_waiter_->OnEvent(DialogEvent::STRIKE_CHANGE_COMPLETE); event_waiter_->OnEvent(DialogEvent::STRIKE_CHANGE_COMPLETE);
} }
void SaveCardBubbleViewsBrowserTestBase::SetAccountFullName( void SaveCardBubbleViewsBrowserTestBase::SetUpInProcessBrowserTestFixture() {
will_create_browser_context_services_subscription_ =
BrowserContextDependencyManager::GetInstance()
->RegisterWillCreateBrowserContextServicesCallbackForTesting(
base::BindRepeating(&SaveCardBubbleViewsBrowserTestBase::
OnWillCreateBrowserContextServices,
base::Unretained(this)));
}
void SaveCardBubbleViewsBrowserTestBase::OnWillCreateBrowserContextServices(
content::BrowserContext* context) {
// Replace the signin manager and account fetcher service with fakes.
SigninManagerFactory::GetInstance()->SetTestingFactory(
context, base::BindRepeating(&BuildFakeSigninManagerForTesting));
AccountFetcherServiceFactory::GetInstance()->SetTestingFactory(
context,
base::BindRepeating(&FakeAccountFetcherServiceBuilder::BuildForTests));
}
void SaveCardBubbleViewsBrowserTestBase::SignInWithFullName(
const std::string& full_name) { const std::string& full_name) {
identity::IdentityManager* identity_manager = // TODO(crbug.com/859761): Can this function be used to remove the
IdentityManagerFactory::GetForProfile(browser()->profile()); // observer_for_testing_ hack in
AccountInfo info = identity_manager->GetPrimaryAccountInfo(); // CreditCardSaveManager::IsCreditCardUploadEnabled()?
info.full_name = full_name; FakeSigninManagerForTesting* signin_manager =
identity::UpdateAccountInfoForAccount(identity_manager, info); static_cast<FakeSigninManagerForTesting*>(
SigninManagerFactory::GetInstance()->GetForProfile(
browser()->profile()));
// Note: Chrome OS tests seem to rely on these specific login values, so
// changing them is probably not recommended.
constexpr char kTestEmail[] = "stub-user@example.com";
constexpr char kTestGaiaId[] = "stub-user@example.com";
#if !defined(OS_CHROMEOS)
signin_manager->SignIn(kTestGaiaId, kTestEmail, "password");
#else
AccountTrackerService* account_tracker_service =
AccountTrackerServiceFactory::GetForProfile(browser()->profile());
signin_manager->SignIn(account_tracker_service->PickAccountIdForAccount(
kTestGaiaId, kTestEmail));
#endif
FakeAccountFetcherService* account_fetcher_service =
static_cast<FakeAccountFetcherService*>(
AccountFetcherServiceFactory::GetForProfile(browser()->profile()));
account_fetcher_service->FakeUserInfoFetchSuccess(
signin_manager->GetAuthenticatedAccountId(), kTestEmail, kTestGaiaId,
AccountTrackerService::kNoHostedDomainFound, full_name,
/*given_name=*/std::string(), "locale", "avatar.jpg");
} }
void SaveCardBubbleViewsBrowserTestBase::SubmitForm() { void SaveCardBubbleViewsBrowserTestBase::SubmitForm() {
......
...@@ -13,7 +13,6 @@ ...@@ -13,7 +13,6 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "chrome/browser/sync/test/integration/sync_test.h"
#include "chrome/browser/ui/autofill/save_card_bubble_controller_impl.h" #include "chrome/browser/ui/autofill/save_card_bubble_controller_impl.h"
#include "chrome/browser/ui/views/autofill/dialog_view_ids.h" #include "chrome/browser/ui/views/autofill/dialog_view_ids.h"
#include "chrome/browser/ui/views/autofill/save_card_bubble_views.h" #include "chrome/browser/ui/views/autofill/save_card_bubble_views.h"
...@@ -38,7 +37,7 @@ namespace autofill { ...@@ -38,7 +37,7 @@ namespace autofill {
// Base class for any interactive SaveCardBubbleViews browser test that will // Base class for any interactive SaveCardBubbleViews browser test that will
// need to show and interact with the offer-to-save bubble. // need to show and interact with the offer-to-save bubble.
class SaveCardBubbleViewsBrowserTestBase class SaveCardBubbleViewsBrowserTestBase
: public SyncTest, : public InProcessBrowserTest,
public CreditCardSaveManager::ObserverForTest, public CreditCardSaveManager::ObserverForTest,
public SaveCardBubbleControllerImpl::ObserverForTest { public SaveCardBubbleControllerImpl::ObserverForTest {
public: public:
...@@ -78,8 +77,14 @@ class SaveCardBubbleViewsBrowserTestBase ...@@ -78,8 +77,14 @@ class SaveCardBubbleViewsBrowserTestBase
void OnBubbleClosed() override; void OnBubbleClosed() override;
void OnSCBCStrikeChangeComplete() override; void OnSCBCStrikeChangeComplete() override;
// Sets the full name of the signed-in account to the provided |full_name|. // BrowserTestBase:
void SetAccountFullName(const std::string& full_name); void SetUpInProcessBrowserTestFixture() override;
// Sets up the ability to sign in the user.
void OnWillCreateBrowserContextServices(content::BrowserContext* context);
// Signs in the user with the provided |full_name|.
void SignInWithFullName(const std::string& full_name);
// Will call JavaScript to fill and submit the form in different ways. // Will call JavaScript to fill and submit the form in different ways.
void SubmitForm(); void SubmitForm();
...@@ -158,8 +163,6 @@ class SaveCardBubbleViewsBrowserTestBase ...@@ -158,8 +163,6 @@ class SaveCardBubbleViewsBrowserTestBase
base::test::ScopedFeatureList scoped_feature_list_; base::test::ScopedFeatureList scoped_feature_list_;
std::unique_ptr<ProfileSyncServiceHarness> harness_;
private: private:
std::unique_ptr<autofill::EventWaiter<DialogEvent>> event_waiter_; std::unique_ptr<autofill::EventWaiter<DialogEvent>> event_waiter_;
std::unique_ptr<net::FakeURLFetcherFactory> url_fetcher_factory_; std::unique_ptr<net::FakeURLFetcherFactory> url_fetcher_factory_;
......
...@@ -100,13 +100,10 @@ bool IsCreditCardUploadEnabled(const PrefService* pref_service, ...@@ -100,13 +100,10 @@ bool IsCreditCardUploadEnabled(const PrefService* pref_service,
// If the "allow all email domains" flag is off, restrict credit card upload // If the "allow all email domains" flag is off, restrict credit card upload
// only to Google Accounts with @googlemail, @gmail, @google, or @chromium // only to Google Accounts with @googlemail, @gmail, @google, or @chromium
// domains. // domains.
// example.com is on the list because ChromeOS tests rely on using this. That
// should be fine, since example.com is an IANA reserved domain.
if (!base::FeatureList::IsEnabled( if (!base::FeatureList::IsEnabled(
features::kAutofillUpstreamAllowAllEmailDomains) && features::kAutofillUpstreamAllowAllEmailDomains) &&
!(domain == "googlemail.com" || domain == "gmail.com" || !(domain == "googlemail.com" || domain == "gmail.com" ||
domain == "google.com" || domain == "chromium.org" || domain == "google.com" || domain == "chromium.org")) {
domain == "example.com")) {
return false; return false;
} }
......
...@@ -251,16 +251,12 @@ void CreditCardSaveManager::AttemptToOfferCardUploadSave( ...@@ -251,16 +251,12 @@ void CreditCardSaveManager::AttemptToOfferCardUploadSave(
} }
bool CreditCardSaveManager::IsCreditCardUploadEnabled() { bool CreditCardSaveManager::IsCreditCardUploadEnabled() {
#if defined(OS_IOS)
// If observer_for_testing_ is set, assume we are in a browsertest and // If observer_for_testing_ is set, assume we are in a browsertest and
// credit card upload should be enabled by default. // credit card upload should be enabled by default.
// TODO(crbug.com/859761): Remove dependency from iOS tests on this behavior. return observer_for_testing_ ||
if (observer_for_testing_) ::autofill::IsCreditCardUploadEnabled(
return true; client_->GetPrefs(), client_->GetSyncService(),
#endif // defined(OS_IOS) personal_data_manager_->GetAccountInfoForPaymentsServer().email);
return ::autofill::IsCreditCardUploadEnabled(
client_->GetPrefs(), client_->GetSyncService(),
personal_data_manager_->GetAccountInfoForPaymentsServer().email);
} }
bool CreditCardSaveManager::IsUploadEnabledForNetwork( bool CreditCardSaveManager::IsUploadEnabledForNetwork(
......
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