chrome_public_test_apk failing on chromium.memory/Android CFI (org.chromium.chrome.browser.omnibox.UrlBarTest is flaky) |
||||||||||||||||||
Issue descriptionFiled by sheriff-o-matic@appspot.gserviceaccount.com on behalf of mlamouri@google.com chrome_public_test_apk failing on chromium.memory/Android CFI Builders failed on: - Android CFI: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Android%20CFI
,
Nov 26
Landing https://chromium-review.googlesource.com/c/chromium/src/+/1351428 that marks the tests above as flaky. Note that a few tests were failing specifically with "--disable-features=SpannableInlineAutocomplete" but not all so I made no distinctions. Switching the bug from Sheriff-Chromium to a flaky test bug.
,
Nov 26
What are the crash stacks for the failing tests? The two I looked at where not the omnibox test failing but this: chromium: [FATAL:oauth2_token_service_delegate_android.cc(511)] Check failed: !account_name.empty() || account_id.empty(). Can't find account name, account_id=gaia-id-foo@gmail.com +bsazonov to comment on that. I don't think we should be disabling those tests if that is the root cause.
,
Nov 26
04399b53 OAuth2TokenServiceDelegateAndroid::MapAccountIdToAccountName(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) const ../../chrome/browser/signin/oauth2_token_service_delegate_android.cc:511:3 04399a67 OAuth2TokenServiceDelegateAndroid::RefreshTokenIsAvailable(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) const ../../chrome/browser/signin/oauth2_token_service_delegate_android.cc:200:30 02ac6d5b OAuth2TokenService::StartRequestForClientWithContext(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, scoped_refptr<network::SharedURLLoaderFactory>, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, std::__ndk1::set<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, std::__ndk1::less<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >, std::__ndk1::allocator<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > > > const&, OAuth2TokenService::Consumer*) ../../google_apis/gaia/oauth2_token_service.cc:492:8 02ac6bf5 OAuth2TokenService::StartRequest(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, std::__ndk1::set<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, std::__ndk1::less<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >, std::__ndk1::allocator<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > > > const&, OAuth2TokenService::Consumer*) ../../google_apis/gaia/oauth2_token_service.cc:443:10 059ec319 AccountInfoFetcher::Start() ../../components/signin/core/browser/account_info_fetcher.cc:37:23 059ea88b AccountFetcherService::StartFetchingUserInfo(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) ../../components/signin/core/browser/account_fetcher_service.cc:214:14 059ea049 AccountFetcherService::RefreshAccountInfo(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, bool) ../../components/signin/core/browser/account_fetcher_service.cc:250:5 059ea245 AccountFetcherService::RefreshAllAccountInfo(bool) ../../components/signin/core/browser/account_fetcher_service.cc:131:5 059ea5c7 AccountFetcherService::RefreshAllAccountsAndScheduleNext() ../../components/signin/core/browser/account_fetcher_service.cc:176:3 059ea523 AccountFetcherService::ScheduleNextRefresh() ../../components/signin/core/browser/account_fetcher_service.cc:191:5 059ea201 AccountFetcherService::MaybeEnableNetworkFetches() ../../components/signin/core/browser/account_fetcher_service.cc:166:5 059ea17f AccountFetcherService::SetupInvalidationsOnProfileLoad(invalidation::InvalidationService*) ../../components/signin/core/browser/account_fetcher_service.cc:119:3 04211885 ProfileManager::DoFinalInitForServices(Profile*, bool) ../../chrome/browser/profiles/profile_manager.cc:1353:9 04211643 ProfileManager::DoFinalInit(Profile*, bool) ../../chrome/browser/profiles/profile_manager.cc:1271:3 04212c4b ProfileManager::AddProfile(Profile*) ../../chrome/browser/profiles/profile_manager.cc:1453:3 0420e6e7 ProfileManager::CreateAndInitializeProfile(base::FilePath const&) ../../chrome/browser/profiles/profile_manager.cc:1469:19 0420e49b ProfileManager::GetProfile(base::FilePath const&) ../../chrome/browser/profiles/profile_manager.cc:518:10 0420e39d ProfileManager::CreateInitialProfile() ../../chrome/browser/profiles/profile_manager.cc:502:24 0403a541 (anonymous namespace)::CreatePrimaryProfile(content::MainFunctionParams const&, base::FilePath const&, base::CommandLine const&) ../../chrome/browser/chrome_browser_main.cc:408:13 0403965b ChromeBrowserMainParts::PreMainMessageLoopRunImpl() ../../chrome/browser/chrome_browser_main.cc:1534:14 04039347 ChromeBrowserMainParts::PreMainMessageLoopRun()
,
Nov 26
The one example I found: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Android%20CFI/3800
,
Nov 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/336bb21b9ce7a9843eca44c3c156c9f2223d61eb commit 336bb21b9ce7a9843eca44c3c156c9f2223d61eb Author: Mounir Lamouri <mlamouri@chromium.org> Date: Mon Nov 26 22:11:30 2018 Sheriff: mark a few omnibox.UrlBarTest as flaky as the test suite flakes a lot. Bug: 908542 Change-Id: Ibf9b8f2ac2c5bcdd14dcf65483d576fe61a3bf8d TBR: tedchoc@chromium.org Reviewed-on: https://chromium-review.googlesource.com/c/1351428 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/heads/master@{#610958} [modify] https://crrev.com/336bb21b9ce7a9843eca44c3c156c9f2223d61eb/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/UrlBarTest.java
,
Nov 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c3414cc4e2f8796e7ef1584f8a221aa643516eea commit c3414cc4e2f8796e7ef1584f8a221aa643516eea Author: Ted Choc <tedchoc@chromium.org> Date: Tue Nov 27 18:09:19 2018 Revert "Sheriff: mark a few omnibox.UrlBarTest as flaky as the test suite flakes a lot." This reverts commit 336bb21b9ce7a9843eca44c3c156c9f2223d61eb. Reason for revert: From the comment on the bug, these failures were a red herring and the underlying issue was an auth one. If you have explicit failure logs that were these tests failing then please add them to the bug. Original change's description: > Sheriff: mark a few omnibox.UrlBarTest as flaky as the test suite flakes a lot. > > Bug: 908542 > Change-Id: Ibf9b8f2ac2c5bcdd14dcf65483d576fe61a3bf8d > TBR: tedchoc@chromium.org > Reviewed-on: https://chromium-review.googlesource.com/c/1351428 > Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> > Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> > Cr-Commit-Position: refs/heads/master@{#610958} TBR=mlamouri@chromium.org,tedchoc@chromium.org Change-Id: I9df39181033b11a2ac8e3b0e8e395bdb0a626e1d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 908542 Reviewed-on: https://chromium-review.googlesource.com/c/1352336 Reviewed-by: Ted Choc <tedchoc@chromium.org> Commit-Queue: Ted Choc <tedchoc@chromium.org> Cr-Commit-Position: refs/heads/master@{#611191} [modify] https://crrev.com/c3414cc4e2f8796e7ef1584f8a221aa643516eea/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/UrlBarTest.java
,
Nov 27
Assigning to Boris who might know what is going wrong with the tests based on that error stack.
,
Nov 28
This is strange. UrlBarTest doesn't seem to do anything with accounts. Futhermore, from "account_id=gaia-id-foo@gmail.com" in the error message it looks like this account was injected by some other test (likely by OAuth2TokenServiceIntegrationTest [1]). However, ChromeActivityTestRule should've wiped the data before starting the test. Ted, can it be that sometimes ApplicationData.clearAppData doesn't clear everything? [1] https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java
,
Nov 28
Any idea where the account information would be stored? Is it just in memory? For that particular test, should we be setting the AccountIdProvider back to null in teardown? https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java?l=94 Also, we aren't removing the app data at the end of the test and just the beginning, so I don't know what would be cleared between test runs. +jbudorick@ to answer what is cleared between test invocations and is this bot different?
,
Nov 30
We actually store account info in several places: OAuth2TokenService stores account names [1] and native AccountTrackerService stores full account info [2]. From the fact that "account_id=gaia-id-foo@gmail.com" is an artificial Gaia ID, I take it that it's AccountTrackerService data that wasn't wiped between runs. It looks like setting AccountIdProvider back to null won't help here, as the data has already been seeded into AccountTrackerService. [1] https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/signin/OAuth2TokenService.java?l=339&rcl=e58a0f3aebc67caa5439de4650da4aca4c81420c [2] https://cs.chromium.org/chromium/src/components/signin/core/browser/account_tracker_service.cc?l=479&rcl=e58a0f3aebc67caa5439de4650da4aca4c81420c
,
Dec 3
I'm seeing this fail pretty regularly, upping the priority since it frequently shows up on the sheriff dashboard.
,
Dec 4
Robert, the last failure happened on Nov 29 [1]. Has it shown up on the sheriff dashboard after that? [1] https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=chrome_public_test_apk&tests=org.chromium.chrome.browser.omnibox.UrlBarTest%23testCutHuge
,
Dec 4
Sorry, to be clear I didn't mean that particular test, this failure line shows up regularly on failed runs: chromium: [FATAL:oauth2_token_service_delegate_android.cc(511)] Check failed: !account_name.empty() || account_id.empty(). Can't find account name, account_id=gaia-id-foo@gmail.com which seems to lead to failing whichever test was running. For example, this failure: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Android%20CFI/3892 If you look at why the chrome_public_test_apk tests failed, it crashed, with the following in LOGCAT 12-03 15:10:26.878 17203 17203 W GooglePlayServicesUtil: Google Play services out of date. Requires 12451000 but found 8186448 12-03 15:10:26.922 17261 17261 I cr_ChildProcessService: Creating new ChildProcessService pid=17261 12-03 15:10:26.930 17203 17203 F chromium: [FATAL:oauth2_token_service_delegate_android.cc(510)] Check failed: !account_name.empty() || account_id.empty(). Can't find account name, account_id=gaia-id-bar@gmail.com --------- beginning of crash
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f6c633257fdd88daf45b1abe9fcc543fc739532 commit 3f6c633257fdd88daf45b1abe9fcc543fc739532 Author: Boris Sazonov <bsazonov@chromium.org> Date: Wed Dec 05 16:34:12 2018 [Signin][Android] Reset sign-in state from SigninTestUtil.tearDownAuthForTest This CL adds resetSigninState call to SigninTestUtil.tearDownAuthForTest method. This call wipes accounts stored in shared preferences so this state doesn't leak between tests. Bug: 908542 Change-Id: I1bfa2c42525102022c27f846be6946efd5881d60 Reviewed-on: https://chromium-review.googlesource.com/c/1361730 Reviewed-by: Theresa <twellington@chromium.org> Commit-Queue: Boris Sazonov <bsazonov@chromium.org> Cr-Commit-Position: refs/heads/master@{#613990} [modify] https://crrev.com/3f6c633257fdd88daf45b1abe9fcc543fc739532/chrome/android/javatests/src/org/chromium/chrome/browser/sync/SyncTestRule.java [modify] https://crrev.com/3f6c633257fdd88daf45b1abe9fcc543fc739532/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/DocumentModeAssassinTest.java [modify] https://crrev.com/3f6c633257fdd88daf45b1abe9fcc543fc739532/chrome/android/javatests/src/org/chromium/chrome/browser/test/ChromeBrowserTestRule.java [modify] https://crrev.com/3f6c633257fdd88daf45b1abe9fcc543fc739532/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/signin/SigninTestUtil.java
,
Dec 7
From https://chromium-swarm.appspot.com/task?id=41a03a164f5da710&refresh=10&show_raw=1 it looks like it is indeed OAuth2TokenServiceIntegrationTest that is causing all these failures. https://crrev.com/c/1368147 should fix it.
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9c2fafc982d5c3c2d94f53a8e47e2b131a923ee1 commit 9c2fafc982d5c3c2d94f53a8e47e2b131a923ee1 Author: Boris Sazonov <bsazonov@chromium.org> Date: Fri Dec 07 19:23:11 2018 [Signin][Android] Reset shared prefs in OAuth2TokenServiceIntegrationTest This CL adds SigninHelper.resetSharedPrefs call to OAuth2TokenServiceIntegrationTest.tearDown method. OAuth2TokenServiceIntegrationTest has been causing many test failures in different tests because of the state it has been leaving in shared preferences. Bug: 908542 Change-Id: I10df9d0ab1d268c7da3352ca87ae07659bfa55d2 Reviewed-on: https://chromium-review.googlesource.com/c/1368147 Reviewed-by: Theresa <twellington@chromium.org> Commit-Queue: Boris Sazonov <bsazonov@chromium.org> Cr-Commit-Position: refs/heads/master@{#614777} [modify] https://crrev.com/9c2fafc982d5c3c2d94f53a8e47e2b131a923ee1/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/87406132518479f505ae1d110894641f057f8d63 commit 87406132518479f505ae1d110894641f057f8d63 Author: Boris Sazonov <bsazonov@chromium.org> Date: Tue Dec 11 09:49:11 2018 [Signin][Android] Wipe account list in OAuth2TokenService test This CL addsSigninTestUtil.resetSigninState call to OAuth2TokenServiceIntegrationTest.tearDown, so all signin-related prefs are wiped and the state doesn't leak from the test. Bug: 908542 Change-Id: I43b37d76bd24e14687a4e9d0ab35d2e88d283f6f Reviewed-on: https://chromium-review.googlesource.com/c/1369940 Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Commit-Queue: Boris Sazonov <bsazonov@chromium.org> Cr-Commit-Position: refs/heads/master@{#615472} [modify] https://crrev.com/87406132518479f505ae1d110894641f057f8d63/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java
,
Dec 17
Looks like this is still happening, example here: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Android%20CFI/3994 The test is also flaky for other reasons, this one in particular comes up multiple times: java.lang.UnsatisfiedLinkError: No implementation found for long org.chromium.base.metrics.RecordHistogram.nativeRecordEnumeratedHistogram Example: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Android%20CFI/4006
,
Dec 17
Adding to the sheriff queue, we might want to disable these tests
,
Dec 17
See also bug 912316.
,
Dec 17
The failure from #19 has a different backtrace. CC'ing Mihai, as he has recently mentioned missing tokens during IdentityManager initialization on Android.
[FATAL:identity_manager.cc(271)] Check failed: HasAccountWithRefreshToken(account_id). '
r0 00000000 r1 00003d2d r2 00000006 r3 f770ab7c
r4 f770ab84 r5 f770ab34 r6 00000000 r7 0000010c
r8 00000061 r9 ff84cb8c sl ff84cb88 fp f74b7ec0
ip 00000006 sp ff84c6b0 lr f747cf85 pc f747f374
Stack Trace:
RELADDR FUNCTION FILE:LINE
00042374 <UNKNOWN> /system/lib/libc.so
0003ff81 <UNKNOWN> /system/lib/libc.so
0001c73f <UNKNOWN> /system/lib/libc.so
000198f1 <UNKNOWN> /system/lib/libc.so
000174b0 <UNKNOWN> /system/lib/libc.so
04542831 base::debug::(anonymous namespace)::DebugBreak() ../../base/debug/debugger_posix.cc:229:5
044a5297 logging::LogMessage::~LogMessage() ../../base/logging.cc:876:7
02beaa7b identity::IdentityManager::GetAccountInfoForAccountWithRefreshToken(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) const ../../services/identity/public/cpp/identity_manager.cc:271:3
02bea965 identity::IdentityManager::GetAccountsWithRefreshTokens() const ../../services/identity/public/cpp/identity_manager.cc:92:24
03ff1293 syncer::SyncSessionDurationsMetricsRecorder::HandleSyncAndAccountChange() ../../components/sync/driver/sync_session_durations_metrics_recorder.cc:174:26
03ff1031 syncer::SyncSessionDurationsMetricsRecorder::SyncSessionDurationsMetricsRecorder(syncer::SyncService*, identity::IdentityManager*, GaiaCookieManagerService*) ../../components/sync/driver/sync_session_durations_metrics_recorder.cc:46:3
v------> std::__ndk1::__unique_if<syncer::SyncSessionDurationsMetricsRecorder>::__unique_single std::__ndk1::make_unique<syncer::SyncSessionDurationsMetricsRecorder, syncer::SyncService*&, identity::IdentityManager*&, GaiaCookieManagerService*&>(syncer::SyncService*&, identity::IdentityManager*&, GaiaCookieManagerService*&) ../../third_party/android_ndk/sources/cxx-stl/llvm-libc++/include/memory:3027:32
043906bf AndroidProfileSessionDurationsService::AndroidProfileSessionDurationsService(syncer::SyncService*, identity::IdentityManager*, GaiaCookieManagerService*) ../../chrome/browser/android/metrics/android_profile_session_durations_service.cc:12:0
05e02d6b BrowserContextKeyedServiceFactory::BuildServiceInstanceFor(base::SupportsUserData*) const ../../components/keyed_service/content/browser_context_keyed_service_factory.cc:105:7
052b4d55 KeyedServiceFactory::GetServiceForContext(base::SupportsUserData*, bool) ../../components/keyed_service/core/keyed_service_factory.cc:89:15
04390893 AndroidProfileSessionDurationsServiceFactory::GetForProfile(Profile*) ../../chrome/browser/android/metrics/android_profile_session_durations_service_factory.cc:28:22
042adc4d ProfileManager::DoFinalInitForServices(Profile*, bool) ../../chrome/browser/profiles/profile_manager.cc:1369:3
042ad9e7 ProfileManager::DoFinalInit(Profile*, bool) ../../chrome/browser/profiles/profile_manager.cc:1276:3
042af03d ProfileManager::AddProfile(Profile*) ../../chrome/browser/profiles/profile_manager.cc:1466:3
042aab13 ProfileManager::CreateAndInitializeProfile(base::FilePath const&) ../../chrome/browser/profiles/profile_manager.cc:1482:19
042aa8c7 ProfileManager::GetProfile(base::FilePath const&) ../../chrome/browser/profiles/profile_manager.cc:523:10
042aa7c9 ProfileManager::CreateInitialProfile() ../../chrome/browser/profiles/profile_manager.cc:507:24
040e3701 (anonymous namespace)::CreatePrimaryProfile(content::MainFunctionParams const&, base::FilePath const&, base::CommandLine const&) ../../chrome/browser/chrome_browser_main.cc:403:13
040e28ef ChromeBrowserMainParts::PreMainMessageLoopRunImpl() ../../chrome/browser/chrome_browser_main.cc:1497:14
040e260f ChromeBrowserMainParts::PreMainMessageLoopRun() ../../chrome/browser/chrome_browser_main.cc:1175:18
02dcfbe3 content::BrowserMainLoop::PreMainMessageLoopRun() ../../content/browser/browser_main_loop.cc:983:13
02dd1fc7 int base::internal::Invoker<base::internal::BindState<int (content::BrowserMainLoop::*)(), base::internal::UnretainedWrapper<content::BrowserMainLoop> >, int ()>::RunImpl<int (content::BrowserMainLoop::* const&)(), std::__ndk1::tuple<base::internal::UnretainedWrapper<content::BrowserMainLoop> > const&, 0u>(int (content::BrowserMainLoop::* const&)(), std::__ndk1::tuple<base::internal::UnretainedWrapper<content::BrowserMainLoop> > const&, std::__ndk1::integer_sequence<unsigned int, 0u>) ../../base/bind_internal.h:689:12
032ea24f content::StartupTaskRunner::WrappedTask() ../../content/browser/startup_task_runner.cc:57:35
032ea45f void base::internal::Invoker<base::internal::BindState<void (content::StartupTaskRunner::*)(), base::internal::UnretainedWrapper<content::StartupTaskRunner> >, void ()>::RunImpl<void (content::StartupTaskRunner::* const&)(), std::__ndk1::tuple<base::internal::UnretainedWrapper<content::StartupTaskRunner> > const&, 0u>(void (content::StartupTaskRunner::* const&)(), std::__ndk1::tuple<base::internal::UnretainedWrapper<content::StartupTaskRunner> > const&, std::__ndk1::integer_sequence<unsigned int, 0u>) ../../base/bind_internal.h:689:12
02258c67 base::OnceCallback<void ()>::Run() && ../../base/callback.h:99:12
0449981f base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) ../../base/debug/task_annotator.cc:99:33
044af55f base::MessageLoopImpl::RunTask(base::PendingTask*) ../../base/message_loop/message_loop_impl.cc:374:46
044af767 base::MessageLoopImpl::DeferOrRunPendingTask(base::PendingTask) ../../base/message_loop/message_loop_impl.cc:385:5
044afd6f base::MessageLoopImpl::DoWork() ../../base/message_loop/message_loop_impl.cc:473:16
044b25ad base::MessagePumpForUI::OnNonDelayedLooperCallback() ../../base/message_loop/message_pump_android.cc:165:37
044b21b5 base::(anonymous namespace)::NonDelayedLooperCallback(int, int, void*)
,
Dec 19
,
Dec 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/008ffa428479c33aebbf33c603ba18a2c96e4939 commit 008ffa428479c33aebbf33c603ba18a2c96e4939 Author: Boris Sazonov <bsazonov@chromium.org> Date: Thu Dec 20 19:19:28 2018 [Android] Commit SharedPreferences before finishing the test This CL adds CommitSharedPreferencesTestRule that is used to commit SharedPreferences after tests. This is necessary to make sure that all writes to SharedPreferences have been completed, so tests that wipe SharedPreferences in tearDown don't leak their state. Bug: 916717, 908542 Change-Id: If6213a5a86287feedf5b877cae15f9f4369c92be Reviewed-on: https://chromium-review.googlesource.com/c/1384261 Reviewed-by: Yaron Friedman <yfriedman@chromium.org> Commit-Queue: Boris Sazonov <bsazonov@chromium.org> Cr-Commit-Position: refs/heads/master@{#618284} [modify] https://crrev.com/008ffa428479c33aebbf33c603ba18a2c96e4939/base/BUILD.gn [modify] https://crrev.com/008ffa428479c33aebbf33c603ba18a2c96e4939/base/test/android/javatests/src/org/chromium/base/test/BaseJUnit4ClassRunner.java [add] https://crrev.com/008ffa428479c33aebbf33c603ba18a2c96e4939/base/test/android/javatests/src/org/chromium/base/test/CommitSharedPreferencesTestRule.java
,
Jan 4
Looking here https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=chrome_public_test_apk&showExpectations=true&tests=org.chromium.chrome.browser.omnibox.UrlBarTest it seems that these tests are still somewhat flaky. Is there a reason not to re-instate the flake annotations until they are stable?
,
Jan 4
,
Jan 4
CC-ing bludell@ as this is happening in the Identity s13n code.
,
Jan 4
@fergal, the tests listed seem to have @RetryOnFailure annotations anyway, so that is hint that they are known to be somewhat flaky, but still provide value to run anyway. If they're failing consistently with that annotation, then they should be disabled. The flakiness dashboard to me looked like they were failing only intermittently and never multiple times within a single test run, so they "shouldn't" be producing any sheriff-o-matic alerts. Granted, it would be good to remove those retry annotations and make these consistently pass within a single run, but that isn't the problem with this bug.
,
Jan 4
To me, it looks like a lot of the tests failure are because getInputConnection() is null. I wonder if there is a race for that to be created and we should be waiting for it to be non-null. +changwan@ for potential awareness of the timeline of the input connection.
,
Jan 7
The stack trace in c#22 indeed looks like it's something to fix within IdentityManager. I'll dig in further here tomorrow.
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4ac37e71d84ddd207dbff4fe7a6e22c94515126e commit 4ac37e71d84ddd207dbff4fe7a6e22c94515126e Author: Colin Blundell <blundell@chromium.org> Date: Tue Jan 08 14:50:38 2019 IdentityManager: Remove DCHECK on Android IdentityManager has a sanity-check that verifies that ProfileOAuth2TokenService::RefreshTokenIsAvailable() returns true at various times that match the documented semantics of OAuth2TokenService: - when an account is present in O2TS::GetAccounts(). - on a callback from O2TS::Observer::OnRefreshTokenAvailable(). However, these semantics are currently not fully enforced on Android (see full description in crbug.com/919793). Hence, we need to eliminate this DCHECK on that platform, as it was causing tests to fail (https://bugs.chromium.org/p/chromium/issues/detail?id=908542#c22). Fixing behavior on Android is tracked by crbug.com/919793. In the meantime, IdentityManager gives clients the same view that they previously had via interacting directly with ProfileOAuth2TokenService. Bug: 908542, 919793 Change-Id: I402fff2b06bb1bacb96fd769f115d91a95af97a1 Reviewed-on: https://chromium-review.googlesource.com/c/1400267 Reviewed-by: David Roger <droger@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#620714} [modify] https://crrev.com/4ac37e71d84ddd207dbff4fe7a6e22c94515126e/services/identity/public/cpp/identity_manager.cc
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bed306a54986426bd84757f123b0c27db8940b96 commit bed306a54986426bd84757f123b0c27db8940b96 Author: Ted Choc <tedchoc@chromium.org> Date: Tue Jan 08 17:39:59 2019 Attempt to deflake UrlBarTest In the error logs, there are a few tests that fail occasionally with a null input connection on focus. This adds a wait for the connection to become non-null. BUG=908542 Change-Id: I35d6988ca5615aaef47329618f19c7f66cd13bdb Reviewed-on: https://chromium-review.googlesource.com/c/1399981 Reviewed-by: Changwan Ryu <changwan@chromium.org> Commit-Queue: Ted Choc <tedchoc@chromium.org> Cr-Commit-Position: refs/heads/master@{#620781} [modify] https://crrev.com/bed306a54986426bd84757f123b0c27db8940b96/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/UrlBarTest.java [modify] https://crrev.com/bed306a54986426bd84757f123b0c27db8940b96/chrome/test/android/javatests/src/org/chromium/chrome/test/util/OmniboxTestUtils.java
,
Jan 9
Moving back to Available as https://chromium-review.googlesource.com/c/1400267 should eliminate the problem that Boris identified in c#22 above.
,
Jan 9
Thanks a lot for your help in de-flaking this! However, it looks like there is still some flakiness related to input connection not being initialized: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/KitKat%20Phone%20Tester%20%28dbg%29/11344 IUUC, this build includes CL from #32. Stacktrace: java.lang.AssertionError: Input connection never initialized for URL bar. at org.junit.Assert.fail(Assert.java:88) at org.junit.Assert.assertTrue(Assert.java:41) at org.chromium.content_public.browser.test.util.CriteriaHelper.pollInstrumentationThread(CriteriaHelper.java:92) at org.chromium.content_public.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:171) at org.chromium.content_public.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:210) at org.chromium.content_public.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:222) at org.chromium.chrome.browser.omnibox.UrlBarTest.toggleFocusAndIgnoreImeOperations(UrlBarTest.java:98) at org.chromium.chrome.browser.omnibox.UrlBarTest.testAutocompleteCorrectlyPerservedOnBatchMode(UrlBarTest.java:696) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:515) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:52) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) at org.chromium.base.test.ScreenshotOnFailureStatement.evaluate(ScreenshotOnFailureStatement.java:41) at org.chromium.chrome.test.ChromeActivityTestRule$1.evaluate(ChromeActivityTestRule.java:129) at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55) Ted, who would be the right person to investigate this further? I won't have bandwidth for this for at least a couple of weeks.
,
Jan 9
changwan@ probably has the most knowledge of how the input connection is created. The flake above likely just moved a previous NPE I was seeing to that failure, but it seems like simply polling for the input connection doesn't resolve the issue. There is something in the SpannableAutocompleteEditTextModel that is preventing the connection from being created. Either onCreateInputConnection isn't being called or it is being called with null. This will likely take more digging into the textview code to figure out. |
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by mlamouri@chromium.org
, Nov 26Summary: chrome_public_test_apk failing on chromium.memory/Android CFI (org.chromium.chrome.browser.omnibox.UrlBarTest is flaky) (was: chrome_public_test_apk failing on chromium.memory/Android CFI)