GitLab

This reverts commit 3f40244c.

Reason for revert: Caused a data race running components_unittests with ScopedFeatureList.

Sample failure: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20TSan%20Tests/30398

WARNING: ThreadSanitizer: data race (pid=9805)
  Write of size 8 at 0x55b63bfba2b0 by main thread:
    #0 base::FeatureList::ClearInstanceForTesting() base/feature_list.cc:284:27 (components_unittests+0x85bec63)
    #1 base::test::ScopedFeatureList::~ScopedFeatureList() base/test/scoped_feature_list.cc:98:3 (components_unittests+0x9a65d52)
    #2 content::UnitTestTestSuite::~UnitTestTestSuite() content/public/test/unittest_test_suite.cc:61:1 (components_unittests+0xb03cf21)
    #3 operator() buildtools/third_party/libc++/trunk/include/memory:2325:5 (components_unittests+0x8256779)
    #4 reset buildtools/third_party/libc++/trunk/include/memory:2638 (components_unittests+0x8256779)
    #5 ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2592 (components_unittests+0x8256779)
    #6 ~__tuple_leaf buildtools/third_party/libc++/trunk/include/tuple:171 (components_unittests+0x8256779)
    #7 ~tuple buildtools/third_party/libc++/trunk/include/tuple:470 (components_unittests+0x8256779)
    #8 ~BindState base/bind_internal.h:871 (components_unittests+0x8256779)
    #9 base::internal::BindState<int (content::UnitTestTestSuite::*)(), std::__1::unique_ptr<content::UnitTestTestSuite, std::__1::default_delete<content::UnitTestTestSuite> > >::Destroy(base::internal::BindStateBase const*) base/bind_internal.h:874 (components_unittests+0x8256779)
    #10 Destruct base/callback_internal.cc:29:3 (components_unittests+0x85b80c7)
    #11 Release base/memory/ref_counted.h:403 (components_unittests+0x85b80c7)
    #12 Release base/memory/scoped_refptr.h:284 (components_unittests+0x85b80c7)
    #13 ~scoped_refptr base/memory/scoped_refptr.h:208 (components_unittests+0x85b80c7)
    #14 base::internal::CallbackBase::~CallbackBase() base/callback_internal.cc:84 (components_unittests+0x85b80c7)
    #15 base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:575:3 (components_unittests+0x9a71272)
    #16 main components/test/run_all_unittests.cc:8:10 (components_unittests+0x4e49ad5)

  Previous read of size 8 at 0x55b63bfba2b0 by thread T12 (mutexes: write M19294):
    #0 base::FeatureList::IsEnabled(base::Feature const&) base/feature_list.cc:200:8 (components_unittests+0x85be82d)
    #1 CanCleanupLockRequired base/task/task_scheduler/scheduler_worker_pool_impl.cc:672:12 (components_unittests+0x866a5e8)
    #2 base::internal::SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::GetWork(base::internal::SchedulerWorker*) base/task/task_scheduler/scheduler_worker_pool_impl.cc:538 (components_unittests+0x866a5e8)
    #3 base::internal::SchedulerWorker::RunWorker() base/task/task_scheduler/scheduler_worker.cc:324:51 (components_unittests+0x866f81d)
    #4 base::internal::SchedulerWorker::RunPooledWorker() base/task/task_scheduler/scheduler_worker.cc:229:3 (components_unittests+0x866f481)
    #5 base::internal::SchedulerWorker::ThreadMain() base/task/task_scheduler/scheduler_worker.cc:208:7 (components_unittests+0x866f2f1)
    #6 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:81:13 (components_unittests+0x86daf04)

  Location is global 'base::(anonymous namespace)::g_feature_list_instance' of size 8 at 0x55b63bfba2b0 (components_unittests+0x00000ebe52b0)

Original change's description:
> [TaskScheduler]: Create no detach below initial capacity feature
> 
> Under this experiment, scheduler workers are only detached if the pool is
> above its initial capacity (threads that are created to replace blocked threads).
> 
> 2 options were considered:
> Option A: Detach only when over initial capacity.
> 
> Option B: Detach only when over current capacity (includes currently blocked threads in capacity).
> This might better handle the following case: At any given time, there is at least 1 blocked thread.
> On top of that, some periodic work uses all worker every 30s or so. The current capacity will
> encompass for the blocked thread and avoid detaching it periodically.
> 
> Option A was picked because it is more conservative. Initial capacity is smaller or
> equal to current capacity, so detaching is closer to current behavior. We want to avoid having
> too many threads that aren't used.
> 
> Bug: 847501
> Change-Id: I0b116db54095767768b158d92f5f146249720b45
> Reviewed-on: https://chromium-review.googlesource.com/c/1348863
> Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612698}

TBR=fdoray@chromium.org,etiennep@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 847501
Change-Id: I951f5c5701e2d296b2c4edef37648105c4911cf9
Reviewed-on: https://chromium-review.googlesource.com/c/1359127Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613229}