Skip to content

Commit

Permalink
[cronet] Apply AtExitManager workaround to Component builds.
Browse files Browse the repository at this point in the history
We work-around shared global state in component builds of the Cronet
native library by avoiding creating the AtExitManager in those builds.
The condition for that is fixed here, from being based on Debug build
to depending directly on whether this is a component build.

The tests are also enabled on Fuchsia/Debug, where they now pass.

Bug: 833401, 816705
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I1401c0dcaa8e893c6cd4bcbf75f8d4d30ccd0116
Reviewed-on: https://chromium-review.googlesource.com/1014642
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Reid Kleckner <rnk@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551788}
  • Loading branch information
Wez authored and Commit Bot committed Apr 18, 2018
1 parent ed92923 commit 5fe1469
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 14 deletions.
14 changes: 6 additions & 8 deletions components/cronet/cronet_global_state_stubs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,14 @@ namespace cronet {
namespace {

scoped_refptr<base::SingleThreadTaskRunner> InitializeAndCreateTaskRunner() {
// TODO(https://crbug.com/816705): Chromium Debug bots use component builds,
// which result in //base and other process-global state being shared between
// the library and the test suite. Since the Debug build is only used to run
// the Debug cronet_tests, assume the suite will define things, for now.
#if defined(NDEBUG)
// TODO(https://crbug.com/816705): Component builds result in //base and other
// process-global state being shared between the library and the test suite.
// Since we only expect Cronet native library component build to be used to run
// cronet_tests, we can assume that suite will define some things, for now.
#if !defined(COMPONENT_BUILD)
// TODO(wez): Remove this once AtExitManager dependencies are gone.
// This fails cronet_test in component builds, which are not supported.
// See https://crbug.com/816705.
ignore_result(new base::AtExitManager);
#endif // defined(NDEBUG)
#endif

base::FeatureList::InitializeInstance(std::string(), std::string());

Expand Down
11 changes: 11 additions & 0 deletions testing/buildbot/chromium.fyi.json
Original file line number Diff line number Diff line change
Expand Up @@ -3054,6 +3054,17 @@
},
"test": "base_unittests"
},
{
"swarming": {
"can_use_on_swarming_builders": true,
"dimension_sets": [
{
"kvm": "1"
}
]
},
"test": "cronet_tests"
},
{
"swarming": {
"can_use_on_swarming_builders": true,
Expand Down
6 changes: 0 additions & 6 deletions testing/buildbot/test_suite_exceptions.pyl
Original file line number Diff line number Diff line change
Expand Up @@ -1281,12 +1281,6 @@
},
},
},
'cronet_tests': {
'remove_from': [
# chromium.fyi
'Fuchsia (dbg)',
],
},
'crypto_unittests': {
'remove_from': [
# TODO(kbr): these tests aren't run on Android except on one bot
Expand Down

0 comments on commit 5fe1469

Please sign in to comment.