Skip to content

GitLab

T
tangled
Commit 58450005 authored by Nick Burris's avatar Nick Burris Committed by Chromium LUCI CQ
Browse files
Options

Tentative fix for flaky PaymentRequest tests on Mac10.11 

* Both PaymentRequestContactInfoEditorTest and
  PaymentRequestShippingAddressEditorTest are flaky on Mac10.11. The
  flake output[1] shows that OnPersonalDataChanged is called twice, the
  first call is unexpected and happens before the profile is created, so
  the test fails.
* Local debugging shows that OnPersonalDataChanged is always called
  twice when the test runs, but only the second call is meant to be
  observed.
* A possible cause is the persistent observer between tests and not
  removing the observer from the PersonalDataManager. This is a
  tentative fix to make the observer local to each test and remove it
  after the event.
* Could not repro the flake with this fix after 7 tries on
  mac_chromium_10.11_rel_ng (note this bot consistently fails for
  unrelated tests).

[1]
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8863274960955528464/+/steps/browser_tests_on__none__GPU_on_Mac_on_Mac-10.11/0/logs/Deterministic_failure:_PaymentRequestShippingAddressEditorTest.AsyncData__status_FAILURE_/0

Bug: 1164438
Change-Id: Ia3e3f4f360f0b58a42a7816616aa8c697047dd60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2580303Reviewed-by: default avatarLiquan (Max) Gu <maxlg@chromium.org>
Commit-Queue: Nick Burris <nburris@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844600}
parent 98999604
Hide whitespace changes
Inline Side-by-side
Showing with 56 additions and 31 deletions
chrome/browser/ui/views/payments/contact_info_editor_view_controller_browsertest.cc
View file @ 58450005
......@@ -33,8 +33,6 @@ class PaymentRequestContactInfoEditorTest
: public PaymentRequestBrowserTestBase {
protected:
PaymentRequestContactInfoEditorTest() {}
PersonalDataLoadedObserverMock personal_data_observer_;
};
IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest, HappyPath) {
......@@ -48,16 +46,18 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest, HappyPath) {
SetEditorTextfieldValue(base::ASCIIToUTF16(kEmailAddress),
autofill::EMAIL_ADDRESS);
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
// Wait until the web database has been updated and the notification sent.
base::RunLoop data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&data_loop));
ClickOnDialogViewAndWait(DialogViewID::EDITOR_SAVE_BUTTON);
data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0];
DCHECK(profile);
......@@ -91,12 +91,13 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest,
SetEditorTextfieldValue(base::ASCIIToUTF16(kEmailAddress),
autofill::EMAIL_ADDRESS);
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
// Wait until the web database has been updated and the notification sent.
base::RunLoop data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&data_loop));
views::View* editor_sheet = dialog_view()->GetViewByID(
static_cast<int>(DialogViewID::CONTACT_INFO_EDITOR_SHEET));
......@@ -104,6 +105,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest,
ui::Accelerator(ui::VKEY_RETURN, ui::EF_NONE));
data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0];
DCHECK(profile);
......@@ -148,16 +150,18 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest, Validation) {
EXPECT_FALSE(IsEditorTextfieldInvalid(autofill::PHONE_HOME_WHOLE_NUMBER));
EXPECT_FALSE(IsEditorTextfieldInvalid(autofill::EMAIL_ADDRESS));
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
// Wait until the web database has been updated and the notification sent.
base::RunLoop data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&data_loop));
ClickOnDialogViewAndWait(DialogViewID::EDITOR_SAVE_BUTTON);
data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0];
DCHECK(profile);
......@@ -176,8 +180,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest, Validation) {
IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest, ModifyExisting) {
NavigateTo("/payment_request_contact_details_test.html");
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
autofill::AutofillProfile incomplete_profile;
incomplete_profile.SetInfo(autofill::NAME_FULL, base::ASCIIToUTF16(kNameFull),
......@@ -202,11 +207,12 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest, ModifyExisting) {
// Wait until the web database has been updated and the notification sent.
base::RunLoop save_data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&save_data_loop));
ClickOnDialogViewAndWait(DialogViewID::EDITOR_SAVE_BUTTON);
save_data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0];
DCHECK(profile);
......@@ -226,8 +232,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest, ModifyExisting) {
IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest,
ModifyExistingSelectsIt) {
NavigateTo("/payment_request_contact_details_test.html");
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
autofill::AutofillProfile incomplete_profile;
incomplete_profile.SetInfo(autofill::NAME_FULL, base::ASCIIToUTF16(kNameFull),
......@@ -259,11 +266,12 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest,
// Wait until the web database has been updated and the notification sent.
base::RunLoop save_data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&save_data_loop));
ClickOnDialogViewAndWait(DialogViewID::EDITOR_SAVE_BUTTON);
save_data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
autofill::AutofillProfile* profile =
request->state()->selected_contact_profile();
DCHECK(profile);
......@@ -294,12 +302,14 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestContactInfoEditorTest,
SetEditorTextfieldValue(base::ASCIIToUTF16(kEmailAddress),
autofill::EMAIL_ADDRESS);
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged()).Times(0);
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged()).Times(0);
ClickOnDialogViewAndWait(DialogViewID::EDITOR_SAVE_BUTTON);
personal_data_manager->RemoveObserver(&personal_data_observer);
// In incognito, the profile should be available in contact_profiles but it
// shouldn't be saved to the PersonalDataManager.
ASSERT_EQ(0UL, personal_data_manager->GetProfiles().size());
......
chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc
View file @ 58450005
......@@ -191,7 +191,6 @@ class PaymentRequestShippingAddressEditorTest
WaitForObservedEvent();
}
PersonalDataLoadedObserverMock personal_data_observer_;
autofill::TestRegionDataLoader test_region_data_loader_;
private:
......@@ -222,16 +221,18 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest, SyncData) {
DialogEvent::PROCESSING_SPINNER_HIDDEN});
// Verifying the data is in the DB.
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
// Wait until the web database has been updated and the notification sent.
base::RunLoop data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&data_loop));
ClickOnDialogViewAndWait(DialogViewID::SAVE_ADDRESS_BUTTON);
data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0];
DCHECK(profile);
......@@ -269,12 +270,13 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest,
DialogEvent::PROCESSING_SPINNER_HIDDEN});
// Verifying the data is in the DB.
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
// Wait until the web database has been updated and the notification sent.
base::RunLoop data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&data_loop));
views::View* editor_sheet = dialog_view()->GetViewByID(
static_cast<int>(DialogViewID::SHIPPING_ADDRESS_EDITOR_SHEET));
......@@ -282,6 +284,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest,
ui::Accelerator(ui::VKEY_RETURN, ui::EF_NONE));
data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0];
DCHECK(profile);
......@@ -319,16 +322,18 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest,
DialogEvent::PROCESSING_SPINNER_HIDDEN});
// Verifying the data is in the DB.
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
// Wait until the web database has been updated and the notification sent.
base::RunLoop data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&data_loop));
ClickOnDialogViewAndWait(DialogViewID::SAVE_ADDRESS_BUTTON);
data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0];
DCHECK(profile);
......@@ -477,16 +482,18 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest,
DialogEvent::PROCESSING_SPINNER_HIDDEN});
// Verifying the data is in the DB.
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
// Wait until the web database has been updated and the notification sent.
base::RunLoop data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&data_loop));
ClickOnDialogViewAndWait(DialogViewID::SAVE_ADDRESS_BUTTON);
data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0];
DCHECK(profile);
......@@ -520,16 +527,18 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest,
DialogEvent::PROCESSING_SPINNER_HIDDEN});
// Verifying the data is in the DB.
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
// Wait until the web database has been updated and the notification sent.
base::RunLoop data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&data_loop));
ClickOnDialogViewAndWait(DialogViewID::SAVE_ADDRESS_BUTTON);
data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0];
DCHECK(profile);
......@@ -580,8 +589,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest,
autofill::PHONE_HOME_WHOLE_NUMBER);
// Verifying the data is in the DB.
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
ResetEventWaiterForSequence({DialogEvent::PROCESSING_SPINNER_SHOWN,
DialogEvent::BACK_TO_PAYMENT_SHEET_NAVIGATION,
......@@ -589,11 +599,12 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest,
// Wait until the web database has been updated and the notification sent.
base::RunLoop data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&data_loop));
ClickOnDialogViewAndWait(DialogViewID::SAVE_ADDRESS_BUTTON);
data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* saved_profile =
personal_data_manager->GetProfiles()[0];
......@@ -766,16 +777,18 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest,
DialogEvent::PROCESSING_SPINNER_HIDDEN});
// Verifying the data is in the DB.
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
// Wait until the web database has been updated and the notification sent.
base::RunLoop data_loop;
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged())
.WillOnce(QuitMessageLoop(&data_loop));
ClickOnDialogViewAndWait(DialogViewID::SAVE_ADDRESS_BUTTON);
data_loop.Run();
personal_data_manager->RemoveObserver(&personal_data_observer);
ASSERT_EQ(1UL, personal_data_manager->GetProfiles().size());
autofill::AutofillProfile* profile = personal_data_manager->GetProfiles()[0];
DCHECK(profile);
......@@ -1250,12 +1263,14 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestShippingAddressEditorTest,
DialogEvent::PROCESSING_SPINNER_HIDDEN});
// Verifying the data is in the DB.
PersonalDataLoadedObserverMock personal_data_observer;
autofill::PersonalDataManager* personal_data_manager = GetDataManager();
personal_data_manager->AddObserver(&personal_data_observer_);
personal_data_manager->AddObserver(&personal_data_observer);
EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged()).Times(0);
EXPECT_CALL(personal_data_observer, OnPersonalDataChanged()).Times(0);
ClickOnDialogViewAndWait(DialogViewID::SAVE_ADDRESS_BUTTON);
personal_data_manager->RemoveObserver(&personal_data_observer);
// In incognito, the profile should be available in shipping_profiles but it
// shouldn't be saved to the PersonalDataManager.
ASSERT_EQ(0UL, personal_data_manager->GetProfiles().size());
......
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