[Sync] Fix flaky TwoClientAutofillSyncTest.ConflictingFields.

[Sync] Fix flaky TwoClientAutofillSyncTest.ConflictingFields.

Reworked how autofill_helper waits for PersonalDataManager. Expecting a
single OnPersonalDataChanged was extremely fragile due to race
conditions from remote changes being applied, and removing the
possibility for race conditions involved increasing the complexity of
test cases significantly. Instead, replaced the single expected call by
completely removing waits from modifications, and changing the wait on
read to post a task to the DB thread and wait for it to be run. While
the PersonalDataManager does sometimes cancel these query tasks, this
is safe because we block the UI thread which is where the
PersonalDataManager operates.

Removed obsolete MakeABookmarkChange method. A while back the nudge
handler was changed to schedule commits almost immediately for
integration tests, which was essentially what MakeABookmarkChange was
trying to achieve.

Removed unused SyncTest::notifications_enabled_ field, removed
AllProfilesMatch() method, and other minor refactoring changes.

BUG=701814
Review-Url: https://codereview.chromium.org/2791793002
Cr-Commit-Position: refs/heads/master@{#461889}
Committed: https://chromium.googlesource.com/chromium/src/+/fce24c540141bc39ca8aa0b29b4a16debfbdb5d4

[skym, 2017-03-31 22:40:55]

The CQ bit was checked by skym@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 22:41:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 23:21:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 23:21:15]

Dry run: This issue passed the CQ dry run.

[skym, 2017-04-03 22:55:53]

Description was changed from

==========
[Sync] Fix flaky TwoClientAutofillSyncTest.ConflictingFields.

BUG=701814
==========

to

==========
[Sync] Fix flaky TwoClientAutofillSyncTest.ConflictingFields.

Reworked how autofill_helper waits for PersonalDataManager. Expecting a
single OnPersonalDataChanged was extremely fragile due to race
conditions from remote changes being applied, and removing the
possibility for race conditions involved increasing the complexity of
test cases significantly. Instead, replaced the single expected call by
completely removing waits from modifications, and changing the wait on
read to post a task to the DB thread and wait for it to be run. While
the PersonalDataManager does sometimes cancel these query tasks, this
is safe because we block the UI thread which is where the
PersonalDataManager operates.

Removed obsolete MakeABookmarkChange method. A while back the nudge
handler was changed to schedule commits almost immediately for
integration tests, which was essentially what MakeABookmarkChange was
trying to achieve.

Removed unused SyncTest::notifications_enabled_ field.

BUG=701814
==========

[skym, 2017-04-03 22:55:53]

skym@chromium.org changed reviewers:
+ pavely@chromium.org

[skym, 2017-04-03 22:59:11]

The CQ bit was checked by skym@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-03 22:59:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 23:28:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-03 23:28:43]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[skym, 2017-04-04 16:06:13]

The CQ bit was checked by skym@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 16:06:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[skym, 2017-04-04 16:09:24]

Description was changed from

==========
[Sync] Fix flaky TwoClientAutofillSyncTest.ConflictingFields.

Reworked how autofill_helper waits for PersonalDataManager. Expecting a
single OnPersonalDataChanged was extremely fragile due to race
conditions from remote changes being applied, and removing the
possibility for race conditions involved increasing the complexity of
test cases significantly. Instead, replaced the single expected call by
completely removing waits from modifications, and changing the wait on
read to post a task to the DB thread and wait for it to be run. While
the PersonalDataManager does sometimes cancel these query tasks, this
is safe because we block the UI thread which is where the
PersonalDataManager operates.

Removed obsolete MakeABookmarkChange method. A while back the nudge
handler was changed to schedule commits almost immediately for
integration tests, which was essentially what MakeABookmarkChange was
trying to achieve.

Removed unused SyncTest::notifications_enabled_ field.

BUG=701814
==========

to

==========
[Sync] Fix flaky TwoClientAutofillSyncTest.ConflictingFields.

Reworked how autofill_helper waits for PersonalDataManager. Expecting a
single OnPersonalDataChanged was extremely fragile due to race
conditions from remote changes being applied, and removing the
possibility for race conditions involved increasing the complexity of
test cases significantly. Instead, replaced the single expected call by
completely removing waits from modifications, and changing the wait on
read to post a task to the DB thread and wait for it to be run. While
the PersonalDataManager does sometimes cancel these query tasks, this
is safe because we block the UI thread which is where the
PersonalDataManager operates.

Removed obsolete MakeABookmarkChange method. A while back the nudge
handler was changed to schedule commits almost immediately for
integration tests, which was essentially what MakeABookmarkChange was
trying to achieve.

Removed unused SyncTest::notifications_enabled_ field and AllProfilesMatch()
method.

BUG=701814
==========

[commit-bot: I haz the power, 2017-04-04 16:54:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-04 16:54:45]

Dry run: This issue passed the CQ dry run.

[skym, 2017-04-04 17:29:11]

The CQ bit was checked by skym@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 17:29:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 18:08:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-04 18:08:43]

Dry run: This issue passed the CQ dry run.

[skym, 2017-04-04 18:11:34]

Description was changed from

==========
[Sync] Fix flaky TwoClientAutofillSyncTest.ConflictingFields.

Reworked how autofill_helper waits for PersonalDataManager. Expecting a
single OnPersonalDataChanged was extremely fragile due to race
conditions from remote changes being applied, and removing the
possibility for race conditions involved increasing the complexity of
test cases significantly. Instead, replaced the single expected call by
completely removing waits from modifications, and changing the wait on
read to post a task to the DB thread and wait for it to be run. While
the PersonalDataManager does sometimes cancel these query tasks, this
is safe because we block the UI thread which is where the
PersonalDataManager operates.

Removed obsolete MakeABookmarkChange method. A while back the nudge
handler was changed to schedule commits almost immediately for
integration tests, which was essentially what MakeABookmarkChange was
trying to achieve.

Removed unused SyncTest::notifications_enabled_ field and AllProfilesMatch()
method.

BUG=701814
==========

to

==========
[Sync] Fix flaky TwoClientAutofillSyncTest.ConflictingFields.

Reworked how autofill_helper waits for PersonalDataManager. Expecting a
single OnPersonalDataChanged was extremely fragile due to race
conditions from remote changes being applied, and removing the
possibility for race conditions involved increasing the complexity of
test cases significantly. Instead, replaced the single expected call by
completely removing waits from modifications, and changing the wait on
read to post a task to the DB thread and wait for it to be run. While
the PersonalDataManager does sometimes cancel these query tasks, this
is safe because we block the UI thread which is where the
PersonalDataManager operates.

Removed obsolete MakeABookmarkChange method. A while back the nudge
handler was changed to schedule commits almost immediately for
integration tests, which was essentially what MakeABookmarkChange was
trying to achieve.

Removed unused SyncTest::notifications_enabled_ field, removed
AllProfilesMatch() method, and other minor refactoring changes.

BUG=701814
==========

[skym, 2017-04-04 18:14:07]

The CQ bit was checked by skym@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 18:15:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 19:05:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-04 19:05:12]

Dry run: This issue passed the CQ dry run.

[skym, 2017-04-04 19:42:36]

PTAL

[pavely, 2017-04-04 22:32:03]

lgtm % one nit.

https://codereview.chromium.org/2791793002/diff/120001/chrome/browser/sync/te...
File chrome/browser/sync/test/integration/autofill_helper.cc (right):

https://codereview.chromium.org/2791793002/diff/120001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/autofill_helper.cc:341: // a task to the DB
thread and wait for the it run. This will ensure that our
the it => it

[skym, 2017-04-04 22:37:50]

https://codereview.chromium.org/2791793002/diff/120001/chrome/browser/sync/te...
File chrome/browser/sync/test/integration/autofill_helper.cc (right):

https://codereview.chromium.org/2791793002/diff/120001/chrome/browser/sync/te...
chrome/browser/sync/test/integration/autofill_helper.cc:341: // a task to the DB
thread and wait for the it run. This will ensure that our
On 2017/04/04 22:32:03, pavely wrote:
> the it => it

Done.

[skym, 2017-04-04 22:37:56]

The CQ bit was checked by skym@chromium.org

[skym, 2017-04-04 22:37:57]

The patchset sent to the CQ was uploaded after l-g-t-m from pavely@chromium.org
Link to the patchset: https://codereview.chromium.org/2791793002/#ps140001
(title: "Updated for Pavel's comment.")

[commit-bot: I haz the power, 2017-04-04 22:38:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 23:21:41]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1491345476888150,
"parent_rev": "891f1b7f9fa54c80afd7414730ab57b30bd4d791", "commit_rev":
"fce24c540141bc39ca8aa0b29b4a16debfbdb5d4"}

[commit-bot: I haz the power, 2017-04-04 23:22:32]

Description was changed from

==========
[Sync] Fix flaky TwoClientAutofillSyncTest.ConflictingFields.

Reworked how autofill_helper waits for PersonalDataManager. Expecting a
single OnPersonalDataChanged was extremely fragile due to race
conditions from remote changes being applied, and removing the
possibility for race conditions involved increasing the complexity of
test cases significantly. Instead, replaced the single expected call by
completely removing waits from modifications, and changing the wait on
read to post a task to the DB thread and wait for it to be run. While
the PersonalDataManager does sometimes cancel these query tasks, this
is safe because we block the UI thread which is where the
PersonalDataManager operates.

Removed obsolete MakeABookmarkChange method. A while back the nudge
handler was changed to schedule commits almost immediately for
integration tests, which was essentially what MakeABookmarkChange was
trying to achieve.

Removed unused SyncTest::notifications_enabled_ field, removed
AllProfilesMatch() method, and other minor refactoring changes.

BUG=701814
==========

to

==========
[Sync] Fix flaky TwoClientAutofillSyncTest.ConflictingFields.

Reworked how autofill_helper waits for PersonalDataManager. Expecting a
single OnPersonalDataChanged was extremely fragile due to race
conditions from remote changes being applied, and removing the
possibility for race conditions involved increasing the complexity of
test cases significantly. Instead, replaced the single expected call by
completely removing waits from modifications, and changing the wait on
read to post a task to the DB thread and wait for it to be run. While
the PersonalDataManager does sometimes cancel these query tasks, this
is safe because we block the UI thread which is where the
PersonalDataManager operates.

Removed obsolete MakeABookmarkChange method. A while back the nudge
handler was changed to schedule commits almost immediately for
integration tests, which was essentially what MakeABookmarkChange was
trying to achieve.

Removed unused SyncTest::notifications_enabled_ field, removed
AllProfilesMatch() method, and other minor refactoring changes.

BUG=701814

Review-Url: https://codereview.chromium.org/2791793002
Cr-Commit-Position: refs/heads/master@{#461889}
Committed:
https://chromium.googlesource.com/chromium/src/+/fce24c540141bc39ca8aa0b29b4a...
==========

[commit-bot: I haz the power, 2017-04-04 23:22:33]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/fce24c540141bc39ca8aa0b29b4a...