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

Issue 771205 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

CommandLine flakiness

Project Member Reported by bauerb@chromium.org, Oct 3 2017

Issue description

Example stack trace (from https://luci-milo.appspot.com/buildbot/tryserver.chromium.android/linux_android_rel_ng/397999 / https://luci-logdog.appspot.com/v/?s=chromium%2Fandroid%2Fswarming%2Flogcats%2F38fb673e6e601c11%2F%2B%2Ftombstones_20171003T141929-UTC_06971cd5003b7a77):

base::debug::(anonymous namespace)::DebugBreak()
base::debug::BreakDebugger()+20
logging::LogMessage::~LogMessage()+806
base::CommandLine::ForCurrentProcess()+46
google_util::CommandLineGoogleBaseURL()+36
UIThreadSearchTermsData::GoogleBaseURLValue() const+72
TemplateURLRef::ParseHostAndSearchTermKey(SearchTermsData const&) const+32
TemplateURLRef::ParseIfNecessary(SearchTermsData const&) const+136
TemplateURLRef::HasGoogleBaseURLs(SearchTermsData const&) const+4
TemplateURL::IsGoogleSearchURLWithReplaceableKeyword(SearchTermsData const&) const+16
TemplateURL::ResetKeywordIfNecessary(SearchTermsData const&, bool)+10
TemplateURLService::AddNoNotify(std::__ndk1::unique_ptr<TemplateURL, std::__ndk1::default_delete<TemplateURL> >, bool)+170
TemplateURLService::SetTemplateURLs(std::__ndk1::unique_ptr<std::__ndk1::vector<std::__ndk1::unique_ptr<TemplateURL, std::__ndk1::default_delete<TemplateURL> >, std::__ndk1::allocator<std::__ndk1::unique_ptr<TemplateURL, std::__ndk1::default_delete<TemplateURL> > > >, std::__ndk1::default_delete<std::__ndk1::vector<std::__ndk1::unique_ptr<TemplateURL, std::__ndk1::default_delete<TemplateURL> >, std::__ndk1::allocator<std::__ndk1::unique_ptr<TemplateURL, std::__ndk1::default_delete<TemplateURL> > > > > >)+174
TemplateURLService::OnWebDataServiceRequestDone(int, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >)+100
WebDataRequestManager::RequestCompletedOnThread(std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >)+46
void base::internal::FunctorTraits<void (WebDataRequestManager::*)(std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >), void>::Invoke<scoped_refptr<WebDataRequestManager>, std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> > >(void (WebDataRequestManager::*)(std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >), scoped_refptr<WebDataRequestManager>&&, std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >&&, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >&&)+44
void base::internal::InvokeHelper<false, void>::MakeItSo<void (WebDataRequestManager::*)(std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >), scoped_refptr<WebDataRequestManager>, std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> > >(void (WebDataRequestManager::*&&)(std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >), scoped_refptr<WebDataRequestManager>&&, std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >&&, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >&&)+18
void base::internal::Invoker<base::internal::BindState<void (WebDataRequestManager::*)(std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >), scoped_refptr<WebDataRequestManager>, base::internal::PassedWrapper<std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> > >, base::internal::PassedWrapper<std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> > > >, void ()>::RunImpl<void (WebDataRequestManager::*)(std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >), std::__ndk1::tuple<scoped_refptr<WebDataRequestManager>, base::internal::PassedWrapper<std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> > >, base::internal::PassedWrapper<std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> > > >, 0u, 1u, 2u>(void (WebDataRequestManager::*&&)(std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> >, std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> >), std::__ndk1::tuple<scoped_refptr<WebDataRequestManager>, base::internal::PassedWrapper<std::__ndk1::unique_ptr<WebDataRequest, std::__ndk1::default_delete<WebDataRequest> > >, base::internal::PassedWrapper<std::__ndk1::unique_ptr<WDTypedResult, std::__ndk1::default_delete<WDTypedResult> > > >&&, std::__ndk1::integer_sequence<unsigned int, 0u, 1u, 2u>)+36
base::OnceCallback<void ()>::Run() &&
base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)+98
base::internal::IncomingTaskQueue::RunTask(base::PendingTask*)+62
base::MessageLoop::RunTask(base::PendingTask*)+266
base::MessageLoop::DeferOrRunPendingTask(base::PendingTask
base::MessageLoop::DoWork()+406
DoRunLoopOnce(_JNIEnv*, base::android::JavaParamRef<_jobject*> const&, long long, long long, long long
Java_org_chromium_base_SystemMessageHandler_nativeDoRunLoopOnce+30
dvmPlatformInvoke+112
dvmCallJNIMethod(unsigned int const*, JValue*, Method const*, Thread*)+398
<unknown>
dvmMterpStd(Thread*)+76
dvmInterpret(Thread*, Method const*, JValue*)+184


Given that this seems to happen while the main thread is pumping messages, I don't think it's very likely that the command line never has been initialized at all (it would have failed earlier in that case), but maybe the command line got reset while the browser is running?

 
Cc: gayane@chromium.org
 Issue 700500  has been merged into this issue.
Cc: tedc...@chromium.org thildebr@chromium.org yolandyan@chromium.org
Oh hey, turns out the failing tests all have calls to CommandLine.init(null) in them :-D

org.chromium.chrome.browser.omnibox.OmniboxUrlEmphasizerTest#testEmptyUrl
org.chromium.chrome.browser.omnibox.OmniboxUrlEmphasizerTest#testLongInsecureHTTPSUrl
org.chromium.chrome.browser.omnibox.OmniboxUrlEmphasizerTest#testOtherUrlsOriginEndIndex
org.chromium.chrome.browser.omnibox.OmniboxUrlEmphasizerTest#testVeryShortHTTPWarningUrl
org.chromium.chrome.browser.preferences.privacy.PrivacyPreferencesManagerNativeTest#testSyncUsageAndCrashReporting
org.chromium.chrome.browser.preferences.website.WebsiteAddressTest#testEqualsHashCodeCompareTo
org.chromium.chrome.browser.widget.RoundedIconGeneratorTest#testGetIconTextForUrl

We should probably make sure we don't do that after native startup.
IUUC, Yoland submitted a CL https://chromium-review.googlesource.com/c/chromium/src/+/685811 to fix that. However, one of my CQ dry runs failed after the fix had landed, so apparently the princess is in another castle.
hmm, OmniboxUrlEmphasizerTest, PrivacyPreferencesManagerNativeTest, WebsiteAddressTest, RoundedIconGeneratorTest should all be using ChromeJUnit4ClassRunner for the right commandline init.

Fix CL: https://chromium-review.googlesource.com/#/c/chromium/src/+/698351
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 4 2017

Status: Fixed (was: Untriaged)
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 9 2017

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

commit 137034a1880ce42f27ade6e6566c5e62ac3051ee
Author: Bernhard Bauer <bauerb@chromium.org>
Date: Mon Oct 09 23:26:30 2017

Disallow resetting the CommandLine after native startup.

Once the native message loop is running, native code might access the
command line. Resetting the CommandLine back to a Java version (thereby
nulling out the native command line) can cause DCHECKs at a later point,
which results in test flakiness. Tests that legitimately need to reset
the command line can do that before native startup.

Because the NativeCommandLine is only instantiated during native
startup, remove the whole JNI call. Also disentangle LibraryLoader and
CommandLine, and delete unused code to do with command line resets.

Bug:  771205 
TBR: michaelbai@chromium.org
Change-Id: Ie0b0dbef4e5a7b4a6e6f1987ef03cf8599a07e74
Reviewed-on: https://chromium-review.googlesource.com/698304
Commit-Queue: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507534}
[modify] https://crrev.com/137034a1880ce42f27ade6e6566c5e62ac3051ee/base/BUILD.gn
[modify] https://crrev.com/137034a1880ce42f27ade6e6566c5e62ac3051ee/base/android/command_line_android.cc
[delete] https://crrev.com/270569080f27a7b0b17d3db7af7cfd47fdf7fc70/base/android/command_line_android.h
[modify] https://crrev.com/137034a1880ce42f27ade6e6566c5e62ac3051ee/base/android/java/src/org/chromium/base/CommandLine.java
[modify] https://crrev.com/137034a1880ce42f27ade6e6566c5e62ac3051ee/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
[modify] https://crrev.com/137034a1880ce42f27ade6e6566c5e62ac3051ee/base/android/javatests/src/org/chromium/base/CommandLineTest.java
[modify] https://crrev.com/137034a1880ce42f27ade6e6566c5e62ac3051ee/base/android/library_loader/library_loader_hooks.cc
[modify] https://crrev.com/137034a1880ce42f27ade6e6566c5e62ac3051ee/chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java
[modify] https://crrev.com/137034a1880ce42f27ade6e6566c5e62ac3051ee/content/app/android/library_loader_hooks.cc

Sign in to add a comment