I started a CL to go back to testing in one-library mode in Chromium but still support testing in two-library mode, the idea being Chromium developers won't be responsible for maintaining the funky two-library mode:
https://chromium-review.googlesource.com/c/585298

> If you want test coverage for the cronet api, write a few dedicated tests against it.

The Cronet API is on the order of 300 methods/members/classes so it's a significant task to create tests for it.  The Cronet tests are essentially just tests of our API.  They contain very little passing of C++ objects across .so boundaries, e.g. the WeakPtr change only broke 1 of 322 tests.

Optional tests sounds like we're just waiting for someone to depend on it and ask for a rollback again. Supposing this optional two-library mode breaks, what will the result be? If the result is closing the tree or CLs against //base or //net, I don't think this bug could be considered solved.

The two-library mode won't be run in Chromium, so it won't close the tree; that's what I meant by "Chromium developers won't be responsible for maintaining the funky two-library mode".  If it breaks, it's up to me to fix it after the fact :)

This issue caused https://chromium-review.googlesource.com/1182612 to be reverted, due to cronet_tests failing in a component-build waterfall-bot config that does not exist on CQ.

Since we don't currently use Cronet on platforms other than Android & iOS, can we just disable these tests on the other platforms, for now?

Hmm I was unaware/forgot that cronet_tests used this libcronet.so/libcronet_test.so configuration.  I'll let Misha respond as I think he investigated this yesterday and knows the value of running the Cronet tests on other platforms.  I'm not sure I'm super excited about reverting changes for non-CQ bot breakages.

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

commit 3189010800a3d171ada31068ef82fbda172e365b
Author: Wez <wez@chromium.org>
Date: Fri Aug 24 23:57:42 2018

Check that TaskScheduler is not leaked between tests, or test-cases.

This is a reland of a2a4e75d2752687d91e1272450bdcc6e48b33739 with a
work-around for cronet_tests' sharing libbase.so with libcronet.so in
component builds.

- Move some APIs off of base::TestSuite, that we unnecessarily public.
- Rename the various internal TestEventListeners to express purpose.
- Add CheckForLeakedGlobals check, and DisableCheckForLeakedGlobals API.
  The latter is called by the content::ContentTestSuiteBase, since the
  various browser- and ui-tests running under that suite base run more
  like the browser itself, and so are expected to let global singletons
  leak.

If tests fail or flake due to this patch, then this indicates that the
test is actually leaking global state that may break other tests if they
happen to run in the same process, leading to hard-to-diagnose flakes.

There are two main failure cases:
If the test directly or indirectly uses TaskScheduler, but does not
create a ScopedTaskEnvironment, then please add one to the test, or to
the test base-class, as appropriate, to fix it.
If the test suite cannot be fixed in this way then please add a call to
DisableCheckForLeakedGlobals(), with a comment linking to a bug for the
issue.

TBR: gab, sky, mef
Bug: 875486,  877355 ,  744567 
Change-Id: Iaea38d24ede271c248a3abb0b3f7ee931c2538f5
Reviewed-on: https://chromium-review.googlesource.com/1187783
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586065}
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/base/test/test_suite.cc
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/base/test/test_suite.h
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/chrome/test/base/chrome_test_launcher.cc
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/chrome/test/base/interactive_ui_tests_main.cc
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/chromecast/app/cast_test_launcher.cc
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/components/cronet/BUILD.gn
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/components/cronet/run_all_unittests.cc
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/content/test/content_test_launcher.cc
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/extensions/shell/test/shell_test_launcher_delegate.cc
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/headless/test/headless_test_launcher.cc
[modify] https://crrev.com/3189010800a3d171ada31068ef82fbda172e365b/webrunner/browser/webrunner_test_launcher.cc

TBH I think supporting Cronet in component builds is out of scope and we can just disable it as it has known issues.

I think there is great value of running cronet_tests on desktop platforms, particularly on asan / msan / tsan bots that only support linux.

The issue I was debugging was NOT in component build, but rather in normal build on Windows, and was caused by having 2 copies of //base and //net in one process (cronet.dll and cronet_tests.exe). 

While there is value in running it in other platforms, the fact that Cronet tests are set up in such an unsound way means that doing so imposes costs on development for all libraries that Cronet duplicates in this way.

Cronet really needs to get to the point that its testing strategy is sound. You either need to go back to one copy or have the two copies completely separate with no transfer of objects between them. If I recall, the issue is that you all pass TaskRunners around?

David - excellent point! 

I think for desktop platforms having cronet_tests statically link to cronet and its dependencies is reasonable compromise between test fidelity and development costs. I'll draft a CL for that.

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

commit 41fba7257f1e7d6c7a74c858817fe877776363ac
Author: Misha Efimov <mef@google.com>
Date: Wed Sep 05 17:02:58 2018

[Cronet] Change cronet_tests on desktop to use cronet implementation.

Bug:  744567 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;master.tryserver.chromium.android:android_cronet_tester
Change-Id: I34b5f7c5446ab88a6a66acad8c77c99a8dcb984b
Reviewed-on: https://chromium-review.googlesource.com/1191464
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588893}
[modify] https://crrev.com/41fba7257f1e7d6c7a74c858817fe877776363ac/components/cronet/BUILD.gn
[modify] https://crrev.com/41fba7257f1e7d6c7a74c858817fe877776363ac/components/cronet/cronet_global_state_stubs.cc