"org.chromium.chrome.browser.sync.ui.PassphraseActivityTest#testCallbackAfterBackgrounded" is flaky |
||||
Issue description"org.chromium.chrome.browser.sync.ui.PassphraseActivityTest#testCallbackAfterBackgrounded" is flaky. This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label. We have detected 3 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyYwsSBUZsYWtlIlhvcmcuY2hyb21pdW0uY2hyb21lLmJyb3dzZXIuc3luYy51aS5QYXNzcGhyYXNlQWN0aXZpdHlUZXN0I3Rlc3RDYWxsYmFja0FmdGVyQmFja2dyb3VuZGVkDA. Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
,
Jan 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49ff8037f6ca8b3924769bce5fb484d798c69cc7 commit 49ff8037f6ca8b3924769bce5fb484d798c69cc7 Author: Mike Wittman <wittman@chromium.org> Date: Mon Jan 08 21:59:19 2018 Disable org.chromium.chrome.browser.sync.ui.PassphraseActivityTest#testCallbackAfterBackgrounded Test has been flakily crashing. See https://luci-milo.appspot.com/buildbot/tryserver.chromium.android/linux_android_rel_ng/462719 I 790.983s run_tests_on_device(060ddd7af0ea8220) detected failure in org.chromium.chrome.browser.sync.ui.PassphraseActivityTest#testCallbackAfterBackgrounded. raw output: I 790.983s run_tests_on_device(060ddd7af0ea8220) INSTRUMENTATION_STATUS: numtests=1 I 790.983s run_tests_on_device(060ddd7af0ea8220) INSTRUMENTATION_STATUS: stream= I 790.983s run_tests_on_device(060ddd7af0ea8220) org.chromium.chrome.browser.sync.ui.PassphraseActivityTest: I 790.983s run_tests_on_device(060ddd7af0ea8220) INSTRUMENTATION_STATUS: id=AndroidJUnitRunner I 790.983s run_tests_on_device(060ddd7af0ea8220) INSTRUMENTATION_STATUS: test=testCallbackAfterBackgrounded I 790.983s run_tests_on_device(060ddd7af0ea8220) INSTRUMENTATION_STATUS: class=org.chromium.chrome.browser.sync.ui.PassphraseActivityTest I 790.983s run_tests_on_device(060ddd7af0ea8220) INSTRUMENTATION_STATUS: current=1 I 790.983s run_tests_on_device(060ddd7af0ea8220) INSTRUMENTATION_STATUS_CODE: 1 I 790.983s run_tests_on_device(060ddd7af0ea8220) INSTRUMENTATION_RESULT: shortMsg=Process crashed. I 790.983s run_tests_on_device(060ddd7af0ea8220) INSTRUMENTATION_CODE: 0 TBR=nyquist,bsazonov Bug: 799941 Change-Id: I629484e71a7c4e58521dd121804d26d3e5ea5a6b Reviewed-on: https://chromium-review.googlesource.com/854617 Reviewed-by: Mike Wittman <wittman@chromium.org> Commit-Queue: Mike Wittman <wittman@chromium.org> Cr-Commit-Position: refs/heads/master@{#527773} [modify] https://crrev.com/49ff8037f6ca8b3924769bce5fb484d798c69cc7/chrome/android/javatests/src/org/chromium/chrome/browser/sync/ui/PassphraseActivityTest.java
,
Jan 8 2018
,
Jan 9 2018
Unfortunately I think that disabling testCallbackAfterBackgrounded won't help to address flakiness around oauth2_token_service_delegate_android.cc. There were several reports of this DCHECK failing on startup: issue 541297, issue 698778. As this test doesn't do anything specific to accounts, it looks like disabling it won't reduce the flakiness. I'm going to add debugging info to that DCHECK and re-enable the test.
,
Jan 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/71dc85fdef6c994ce379c087f2ed5b6d6144f4fe commit 71dc85fdef6c994ce379c087f2ed5b6d6144f4fe Author: Boris Sazonov <bsazonov@chromium.org> Date: Thu Jan 11 15:33:46 2018 [Signin][Android] Add info to AccountTokenServiceDelegateAndroid DCHECK This CL adds debugging info to DCHECK that has been causing test failures. It also re-enables the test that was disabled because of this check. Bug: 541297, 698778, 799941 Change-Id: Id2cca4d7fedd19c2b6cf2b808fe41338088d75e8 Reviewed-on: https://chromium-review.googlesource.com/857177 Reviewed-by: Bernhard Bauer <bauerb@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Commit-Queue: Boris Sazonov <bsazonov@chromium.org> Cr-Commit-Position: refs/heads/master@{#528633} [modify] https://crrev.com/71dc85fdef6c994ce379c087f2ed5b6d6144f4fe/chrome/android/javatests/src/org/chromium/chrome/browser/sync/ui/PassphraseActivityTest.java [modify] https://crrev.com/71dc85fdef6c994ce379c087f2ed5b6d6144f4fe/chrome/browser/signin/oauth2_token_service_delegate_android.cc
,
Jan 12 2018
OK, I've dug to the bottom of it. The issue has been introduced by https://crrev.com/c/817444, because now account list cache in AccountManagerFacade is updated from background thread. This makes the possible scenario possible: 1. AccountTrackerService seeds a list of accounts. 2. Account list is updated from background thread (observers aren't notified yet, as it must happen on UI thread). 3. OAuth2TokenService.validateAccounts checks whether accounts have been seeded by calling AccountTrackerService.checkAndSeedSystemAccounts. 4. As AccountTrackerService doesn't know that account list has been changed, checkAndSeedSystemAccounts returns true. 5. OAuth2TokenService.getSystemAccountNames gets a list of accounts (it's the one from step 2). 6. Some accounts in the list haven't been seeded, so DCHECK fires.
,
Jan 12 2018
I've uploaded a CL that should fix this issue: https://crrev.com/c/865394. I've tried running PassphraseActivityTest 400 times with this patch applied and there were zero crashes. Without this patch approximately one run out of five ends in a crash.
,
Jan 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/63243a2ae1b4b20ecfcacb6d7c823a6fe1ca9a92 commit 63243a2ae1b4b20ecfcacb6d7c823a6fe1ca9a92 Author: Boris Sazonov <bsazonov@chromium.org> Date: Mon Jan 15 12:36:51 2018 [Signin][Android] Fix race condition in accounts seeding This CL changes the way the account list cache is updated in AccountManagerFacade. Now the cache is populated on startup by AccountManagerFacade.InitializeTask on a background thread (as main thread may be waiting for the same cache to be populated). After initialization is completed, all changes to the account list are handled by UpdateTask, that always updates the cache on the main thread. This approach solves a race condition that has been causing test failures and simplifies reasoning about the state of the account list in general. Bug: 799941 Change-Id: I98635c5bc44e5b4dc392a604e2755cc04cb9f42d Reviewed-on: https://chromium-review.googlesource.com/865394 Reviewed-by: Bernhard Bauer <bauerb@chromium.org> Commit-Queue: Boris Sazonov <bsazonov@chromium.org> Cr-Commit-Position: refs/heads/master@{#529256} [modify] https://crrev.com/63243a2ae1b4b20ecfcacb6d7c823a6fe1ca9a92/components/signin/core/browser/android/java/src/org/chromium/components/signin/AccountManagerFacade.java
,
Jan 22 2018
There hasn't been any flakes after Jan 15, closing the bug. |
||||
►
Sign in to add a comment |
||||
Comment 1 by wittman@chromium.org
, Jan 8 2018Status: Started (was: Untriaged)