New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 689662 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 694261


Show other hotlists

Hotlists containing this issue:
Hotlist-1


Sign in to add a comment

sync_integration_test failures on ChromeOS

Project Member Reported by s...@chromium.org, Feb 7 2017

Issue description

../../chrome/browser/sync/test/integration/two_client_uss_sync_test.cc:242: Failure
Value of: clients_.size()
 Actual: 3
Expected: 2U
Which is: 2

Seems to work fine on Linux. Working theory is some chrome os specific issue is causing us problems.
 

Comment 1 by s...@chromium.org, Feb 7 2017

Also lets see if we can update try bots to run sync_integration_tests so that we don't regress.

Comment 2 by s...@chromium.org, Feb 8 2017

Current failures seem to be:

SingleClientSessionsSyncTest.NoSessions
TwoClientUssSyncTest.Error
TwoClientArcPackageSyncTest.StartWithDifferentPackages
TwoClientUssSyncTest.Encryption
TwoClientUssSyncTest.Sanity
TwoClientAppListSyncTest.Move
TwoClientArcPackageSyncTest.Install
EnableDisableSingleClientTest.DisableOneAtATime
SingleClientSessionsSyncTest.CookieJarMismatch
SingleClientSessionsSyncTest.ResponseCodeIsPreserved
SingleClientArcPackageSyncTest.ArcPackageInstallSomePackages
TwoClientArcPackageSyncTest.OneClientHasPackagesAnotherHasSubSet
SyncErrorTest.ErrorWhileSettingUp
SingleClientSessionsSyncTest.TimestampMatchesHistory
EnableDisableSingleClientTest.EnableOneAtATime
TwoClientArcPackageSyncTest.StartWithSamePackages
TwoClientUssSyncTest.ConflictResolution
TwoClientArcPackageSyncTest.OneClientHasPackagesAnotherHasNone
SingleClientArcPackageSyncTest.ArcPackageEmpty
TwoClientArcPackageSyncTest.StartWithNoPackages
TwoClientArcPackageSyncTest.InstallDifferent
TwoClientUssSyncTest.DisableEnable
SingleClientSessionsSyncTest.Sanity
TwoClientAppListSyncTest.DisableApps
SingleClientSessionsSyncTest.ChromeHistory
TwoClientArcPackageSyncTest.Uninstall

Comment 3 by s...@chromium.org, Feb 8 2017

Owner: gangwu@chromium.org

Comment 4 by s...@chromium.org, Feb 21 2017

Blockedon: 694261
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e3c16997af716ba961740d9598b6942b70e59ccc

commit e3c16997af716ba961740d9598b6942b70e59ccc
Author: gangwu <gangwu@chromium.org>
Date: Wed Feb 22 18:08:02 2017

[USS] Fix USS in sync_integration_tests in chromeos

Chrome OS will force loading of signin profile, so we need to ignore one more client.
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/chrome_browser_main_chromeos.cc?type=cs&q=ChromeBrowserMainPartsChromeos::PostProfileInit()&l=672

The Tests fixed are
TwoClientUssSyncTest.Error
TwoClientUssSyncTest.Encryption
TwoClientUssSyncTest.Sanity
TwoClientUssSyncTest.ConflictResolution
TwoClientUssSyncTest.DisableEnable
SingleClientSessionsSyncTest.Sanity
SingleClientSessionsSyncTest.NoSessions
SingleClientSessionsSyncTest.ChromeHistory
SingleClientSessionsSyncTest.ResponseCodeIsPreserved
SingleClientSessionsSyncTest.TimestampMatchesHistory

BUG= 689662 

Review-Url: https://codereview.chromium.org/2694503002
Cr-Commit-Position: refs/heads/master@{#452126}

[modify] https://crrev.com/e3c16997af716ba961740d9598b6942b70e59ccc/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc
[modify] https://crrev.com/e3c16997af716ba961740d9598b6942b70e59ccc/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc

After https://codereview.chromium.org/2711033002/, right now the failures seem to be:
TwoClientAppListSyncTest.Move
EnableDisableSingleClientTest.DisableOneAtATime
SingleClientSessionsSyncTest.CookieJarMismatch
SingleClientPrintersSyncTest.AddBeforeSetup
TwoClientPrintersSyncTest.SimpleMerge
SyncErrorTest.ErrorWhileSettingUp
EnableDisableSingleClientTest.EnableOneAtATime
TwoClientAppListSyncTest.DisableApps
This is on Windows today

[ RUN      ] SingleClientSessionsSyncTest.CookieJarMismatch
[2456:6636:0317/055625.250:WARNING:chrome_browser_main_win.cc(452)] Command line too long for RegisterApplicationRestart:  --brave-new-test-launcher --gtest_also_run_disabled_tests --gtest_filter=SingleClientSessionsSyncTest.CookieJarMismatch --single_process --test-launcher-bot-mode --test-launcher-summary-output="e:\swarm_slave\w\iopcjbln\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir3552_15933\d3552_14680" --disable-offline-auto-reload --disable-background-networking --sync-short-initial-retry-override --sync-short-nudge-delay-for-test --disable-features=NetworkPrediction --no-first-run --no-default-browser-check --enable-logging=stderr --log-level=0 --safebrowsing-disable-auto-update --disable-default-apps --wm-window-animations-disabled --disable-component-update --test-type=browser --disable-zero-browsers-open-for-tests --ipc-connection-timeout=30 --allow-file-access-from-files --dom-automation --log-gpu-control-list-decisions --disable-backgrounding-occluded-windows --disable-gl-drawing-for-tests --override-use-software-gl-for-tests --flag-switches-begin --flag-switches-end --restore-last-session about:blank
[2456:6100:0317/055625.671:WARNING:syncer_proto_util.cc(338)] Error posting from syncer: Response Code (bogus on error): -1 Content-Length (bogus on error): -1 Server Status: SYNC_AUTH_ERROR
[2456:6100:0317/055625.674:WARNING:sync_encryption_handler_impl.cc(971)] Nigori had empty encryption keybag.
[2456:6100:0317/055625.674:WARNING:sync_encryption_handler_impl.cc(971)] Nigori had empty encryption keybag.
[2456:6100:0317/055625.694:WARNING:sync_encryption_handler_impl.cc(347)] Ignoring new implicit passphrase. Keystore migration already performed.
c:\c\win\src\chromerowser\sync	est\integration\single_client_sessions_sync_test.cc(61): error: Value of: samples->TotalCount()
  Actual: 2
Expected: sample_count
Which is: 1
c:\c\win\srcase	est\histogram_tester.cc(170): error: Value of: actual_count
  Actual: 1
Expected: expected_count
Which is: 0
Histogram "Sync.CookieJarEmptyOnMismatch" does not have the right total number of samples (0). It has (1).
[2456:6100:0317/055626.099:WARNING:syncer_proto_util.cc(338)] Error posting from syncer: Response Code (bogus on error): -1 Content-Length (bogus on error): -1 Server Status: CONNECTION_UNAVAILABLE
[2456:6100:0317/055626.099:ERROR:get_updates_processor.cc(244)] PostClientToServerMessage() failed during GetUpdates
[2456:6636:0317/055626.105:ERROR:shared_change_processor.cc(210)] Sessions datatype error was encountered: Change processor disconnected.
[  FAILED  ] SingleClientSessionsSyncTest.CookieJarMismatch, where TypeParam =  and GetParam() =  (1101 ms)
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0ec5a56f12d2bdb426f1aae99f7bb26153ffa1a

commit a0ec5a56f12d2bdb426f1aae99f7bb26153ffa1a
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Fri Mar 17 13:37:44 2017

Disable SingleClientSessionsSyncTest.CookieJarMismatch on Win

BUG= 689662 
TBR=gangwu@chromium.org

Review-Url: https://codereview.chromium.org/2757833004 .
Cr-Commit-Position: refs/heads/master@{#457750}

[modify] https://crrev.com/a0ec5a56f12d2bdb426f1aae99f7bb26153ffa1a/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc

Comment 10 by s...@chromium.org, Mar 17 2017

Cc: pnoland@chromium.org s...@chromium.org
SingleClientSessionsSyncTest.CookieJarMismatch flaking has really been a saga. Not sure if this is the right bug to track it on non-chromeos, some other relevant bugs  issue 695241   issue 663547   issue 603794 .

It looks like instead of having 1+ cookie jar matches, we had 1 match and 1 mismatch... While the GE checks allow for multiple sync cycles to affect each scoped histogram_tester, it doesn't look like we handle the case when one of the extra sync cycles escaped the top scope and affects the bottom scope. WaitForURLOnServer() only blocks for the first cycle.

I think it would be unacceptably lax to allow the second scope to additionally contain a mismatch. Instead we should probably add better blocking calls to force correct synchronization.

Bizarrely, adding a 1 second sleep (on Linux) to the beginning of a commit causes the second scope to only see a mismatch, which doesn't make sense to me yet.

Comment 11 by s...@chromium.org, Mar 17 2017

Okay, the odd failure was my mistake. The sleep I added blocked the sync thread. This meant the UI thread post to sync thread to update cookie jar fields was blocked as well. Unlike the sessions changes which only need the directory lock and could be updated in parallel.

This brings me to a separate threading issue that this test was ignoring. Updating the cookie jar isn't synchronous, but no where does it attempt wait for it to happen correctly.
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4d5e4f5dc10d0203717fbfcce22e546876b793ff

commit 4d5e4f5dc10d0203717fbfcce22e546876b793ff
Author: skym <skym@chromium.org>
Date: Wed Mar 22 16:35:55 2017

[Sync] Try to fix race conditions in CookieJarMismatch.

When we navigate to a URL in sync integration tests, sometimes, usually
when this is the first navigation, we see two separate commits. The
first has most of the data correctly set, but lacks a title. Later on
the title is updated through a second commit. This test case has
previously been updated in an attempt to resilient to this quirk, but
it was not enough.

In the flake in attached bug, what appeared to have happened was that
the first navigation created two commits, but only the first one was
needed to satisfy the WaitForURLOnServer check. Then we call
OnGaiaAccountsInCookieUpdated but the affect is asynchronous. Before
the posted task to update cookie information reaches the sync thread,
but after the second histogram checker is created, a second commit from
navigating to kURL1 executes and we get histogram data that we're not
expecting, leading to a failure.

I was not able to repro locally, so it is unclear if both of the
following fixes were actually needed. Regardless, this should make us
significantly more resilient to races and flakiness.

First, changed the way blocking for sessions changes works, to only
consider data that has a title set. When used in a way, this should
force both commits to happen (if there are going to be two), and less
likely the later will leak out.

Second, added a callback to the mechanism that sets cookie information.
This allows us to truly block until the sync thread has been updated.
It is a significant amount of boilerplate for a very simple concept,
unfortunately.

BUG= 689662 

Review-Url: https://codereview.chromium.org/2751333007
Cr-Commit-Position: refs/heads/master@{#458773}

[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc
[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/components/browser_sync/profile_sync_service.h
[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/components/sync/driver/glue/sync_backend_host_core.cc
[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/components/sync/driver/glue/sync_backend_host_core.h
[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/components/sync/driver/glue/sync_backend_host_impl.cc
[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/components/sync/driver/glue/sync_backend_host_impl.h
[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/components/sync/engine/fake_sync_engine.cc
[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/components/sync/engine/fake_sync_engine.h
[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/components/sync/engine/sync_engine.h
[modify] https://crrev.com/4d5e4f5dc10d0203717fbfcce22e546876b793ff/components/sync/test/fake_server/fake_server_verifier.cc

Summary: sync_integration_test failures on ChromeOS (was: TwoClientUssSyncTest.Sanity failing on ChromeOS)
Project Member

Comment 15 by bugdroid1@chromium.org, Mar 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9b2a322544c613890a49f8f41c277f1ef19e609

commit f9b2a322544c613890a49f8f41c277f1ef19e609
Author: gangwu <gangwu@chromium.org>
Date: Thu Mar 23 00:11:08 2017

[Sync] Disable/Enable APP_LIST by APPS

BUG= 689662 

Review-Url: https://codereview.chromium.org/2771593002
Cr-Commit-Position: refs/heads/master@{#458943}

[modify] https://crrev.com/f9b2a322544c613890a49f8f41c277f1ef19e609/chrome/browser/sync/test/integration/two_client_app_list_sync_test.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Apr 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/864d4cde3b40817cf6831a4a2e6711a4f94413b9

commit 864d4cde3b40817cf6831a4a2e6711a4f94413b9
Author: gangwu <gangwu@chromium.org>
Date: Fri Apr 07 00:47:21 2017

Enable printers sync_integration_tests on chromeos

Fix printer flaky bugs by adding another wait.
The bug is flaky because tests only wait for UI tasks to be completed,
but since PrintersManagerFactory uses
content::BrowserThread::GetBlockingPool() to schedule ModelTypeStore
initialization, and then reply result to UI thread, so we need to wait
on content::BrowserThread::GetBlockingPool() as well.

Remove unnecessary changes from
https://codereview.chromium.org/2758643002 since that did not really
fix this waiting issue.

BUG= 689662 ,  701999 

Review-Url: https://codereview.chromium.org/2799103002
Cr-Commit-Position: refs/heads/master@{#462716}

[modify] https://crrev.com/864d4cde3b40817cf6831a4a2e6711a4f94413b9/chrome/browser/sync/test/integration/printers_helper.cc
[modify] https://crrev.com/864d4cde3b40817cf6831a4a2e6711a4f94413b9/chrome/browser/sync/test/integration/single_client_printers_sync_test.cc
[modify] https://crrev.com/864d4cde3b40817cf6831a4a2e6711a4f94413b9/chrome/browser/sync/test/integration/two_client_printers_sync_test.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Apr 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/03a0356e4e8839701627209ce25c0edaeca3a10f

commit 03a0356e4e8839701627209ce25c0edaeca3a10f
Author: gangwu <gangwu@chromium.org>
Date: Fri Apr 07 21:01:32 2017

[Sync] Fix TwoClientAppListSyncTest.Move

After sync, the test did not install pending apps on the synced client.

BUG= 689662 

Review-Url: https://codereview.chromium.org/2799783007
Cr-Commit-Position: refs/heads/master@{#463000}

[modify] https://crrev.com/03a0356e4e8839701627209ce25c0edaeca3a10f/chrome/browser/sync/test/integration/two_client_app_list_sync_test.cc

TwoClientUssSyncTest.ConflictResolution also seems to be failing on Linux Tests (dbg)

https://uberchromegw.corp.google.com/i/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29%2832%29/builds/45940
Labels: SyncHandoff2018
Status: Available (was: Assigned)
Owner: ----

Comment 21 by treib@chromium.org, Apr 27 2018

Status: Fixed (was: Available)
I think this umbrella bug can be closed. A few sync_integration_tests are still flaky, but those are tracked in individual bugs.

Comment 22 by treib@chromium.org, Apr 27 2018

I think this umbrella bug can be closed. A few sync_integration_tests are still flaky, but those are tracked in individual bugs.

Sign in to add a comment