Revert "Re-enable EngineRequestsNoBlockingTest on Win8 by initializing...

Revert "Re-enable EngineRequestsNoBlockingTest on Win8 by initializing TaskEnvironment with COM disabled"

This reverts commit 2dd4658f.

Reason for revert: Causing the TaskEnvironmentTest.DefaultCOMEnvironment base_unittests to fail on Win7 starting at this build: https://ci.chromium.org/p/chromium/builders/ci/Win7%20Tests%20%281%29/98745

Original change's description:
> Re-enable EngineRequestsNoBlockingTest on Win8 by initializing TaskEnvironment with COM disabled
> 
> R=​fdoray
> 
> Bug: 947576
> Change-Id: I1f901367e6fcc5243f34f50e5704b68727a40731
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1990135
> Commit-Queue: Joe Mason <joenotcharles@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#729802}

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

Change-Id: Ie19f651e25680bc523ca8c8e37e39d11937fedc1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 947576
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1993841Reviewed-by: sebsg <sebsg@chromium.org>
Commit-Queue: sebsg <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729900}

base/test/task_environment.cc

@@ -354,13 +354,11 @@ TaskEnvironment::TaskEnvironment(
     MainThreadType main_thread_type,
     ThreadPoolExecutionMode thread_pool_execution_mode,
     ThreadingMode threading_mode,
-    ThreadPoolCOMEnvironment thread_pool_com_environment,
     bool subclass_creates_default_taskrunner,
     trait_helpers::NotATraitTag)
     : main_thread_type_(main_thread_type),
       thread_pool_execution_mode_(thread_pool_execution_mode),
       threading_mode_(threading_mode),
-      thread_pool_com_environment_(thread_pool_com_environment),
       subclass_creates_default_taskrunner_(subclass_creates_default_taskrunner),
       sequence_manager_(
           CreateSequenceManagerForMainThreadType(main_thread_type)),
@@ -436,10 +434,18 @@ void TaskEnvironment::InitializeThreadPool() {
   ThreadPoolInstance::InitParams init_params(kMaxThreads);
   init_params.suggested_reclaim_time = TimeDelta::Max();
 #if defined(OS_WIN)
-  if (thread_pool_com_environment_ == ThreadPoolCOMEnvironment::COM_MTA) {
-    init_params.common_thread_pool_environment =
-        ThreadPoolInstance::InitParams::CommonThreadPoolEnvironment::COM_MTA;
-  }
+  // Enable the MTA in unit tests to match the browser process's
+  // ThreadPoolInstance configuration.
+  //
+  // This has the adverse side-effect of enabling the MTA in non-browser unit
+  // tests as well but the downside there is not as bad as not having it in
+  // browser unit tests. It just means some COM asserts may pass in unit tests
+  // where they wouldn't in integration tests or prod. That's okay because unit
+  // tests are already generally very loose on allowing I/O, waits, etc. Such
+  // misuse will still be caught in later phases (and COM usage should already
+  // be pretty much inexistent in sandboxed processes).
+  init_params.common_thread_pool_environment =
+      ThreadPoolInstance::InitParams::CommonThreadPoolEnvironment::COM_MTA;
 #endif
 
   auto task_tracker = std::make_unique<TestTaskTracker>();

base/test/task_environment.h

@@ -156,28 +156,6 @@ class TaskEnvironment {
     DEFAULT = MULTIPLE_THREADS
   };
 
-  // On Windows, sets the COM environment for the ThreadPoolInstance. Ignored
-  // on other platforms.
-  enum class ThreadPoolCOMEnvironment {
-    // Do not initialize COM for the pool's workers.
-    NONE,
-
-    // Place the pool's workers in a COM MTA.
-    COM_MTA,
-
-    // Enable the MTA by default in unit tests to match the browser process's
-    // ThreadPoolInstance configuration.
-    //
-    // This has the adverse side-effect of enabling the MTA in non-browser unit
-    // tests as well but the downside there is not as bad as not having it in
-    // browser unit tests. It just means some COM asserts may pass in unit
-    // tests where they wouldn't in integration tests or prod. That's okay
-    // because unit tests are already generally very loose on allowing I/O,
-    // waits, etc. Such misuse will still be caught in later phases (and COM
-    // usage should already be pretty much inexistent in sandboxed processes).
-    DEFAULT = COM_MTA,
-  };
-
   // List of traits that are valid inputs for the constructor below.
   struct ValidTraits {
     ValidTraits(TimeSource);
@@ -185,7 +163,6 @@ class TaskEnvironment {
     ValidTraits(ThreadPoolExecutionMode);
     ValidTraits(SubclassCreatesDefaultTaskRunner);
     ValidTraits(ThreadingMode);
-    ValidTraits(ThreadPoolCOMEnvironment);
   };
 
   // Constructor accepts zero or more traits which customize the testing
@@ -203,9 +180,6 @@ class TaskEnvironment {
                                    ThreadPoolExecutionMode::DEFAULT>(traits...),
             trait_helpers::GetEnum<ThreadingMode, ThreadingMode::DEFAULT>(
                 traits...),
-            trait_helpers::GetEnum<ThreadPoolCOMEnvironment,
-                                   ThreadPoolCOMEnvironment::DEFAULT>(
-                traits...),
             trait_helpers::HasTrait<SubclassCreatesDefaultTaskRunner,
                                     TaskEnvironmentTraits...>(),
             trait_helpers::NotATraitTag()) {}
@@ -355,14 +329,12 @@ class TaskEnvironment {
                   MainThreadType main_thread_type,
                   ThreadPoolExecutionMode thread_pool_execution_mode,
                   ThreadingMode threading_mode,
-                  ThreadPoolCOMEnvironment thread_pool_com_environment,
                   bool subclass_creates_default_taskrunner,
                   trait_helpers::NotATraitTag tag);
 
   const MainThreadType main_thread_type_;
   const ThreadPoolExecutionMode thread_pool_execution_mode_;
   const ThreadingMode threading_mode_;
-  const ThreadPoolCOMEnvironment thread_pool_com_environment_;
   const bool subclass_creates_default_taskrunner_;
 
   std::unique_ptr<sequence_manager::SequenceManager> sequence_manager_;

base/test/task_environment_unittest.cc

@@ -44,10 +44,6 @@
 #include "base/files/file_descriptor_watcher_posix.h"
 #endif  // defined(OS_POSIX)
 
-#if defined(OS_WIN)
-#include "base/win/scoped_com_initializer.h"
-#endif
-
 namespace base {
 namespace test {
 
@@ -1203,58 +1199,5 @@ TEST_F(TaskEnvironmentTest, CurrentThread) {
   run_loop.Run();
 }
 
-#if defined(OS_WIN)
-namespace {
-
-enum class ApartmentType {
-  kSTA,
-  kMTA,
-};
-
-void InitializeSTAApartment() {
-  base::win::ScopedCOMInitializer initializer;
-  EXPECT_TRUE(initializer.Succeeded());
-}
-
-void InitializeMTAApartment() {
-  base::win::ScopedCOMInitializer initializer(
-      base::win::ScopedCOMInitializer::kMTA);
-  EXPECT_TRUE(initializer.Succeeded());
-}
-
-void InitializeCOMOnWorker(
-    TaskEnvironment::ThreadPoolCOMEnvironment com_environment,
-    ApartmentType apartment_type) {
-  TaskEnvironment task_environment(com_environment);
-  PostTask(FROM_HERE, BindOnce(apartment_type == ApartmentType::kSTA
-                                   ? &InitializeSTAApartment
-                                   : &InitializeMTAApartment));
-  task_environment.RunUntilIdle();
-}
-
-}  // namespace
-
-TEST_F(TaskEnvironmentTest, DefaultCOMEnvironment) {
-  // Attempt to initialize an MTA COM apartment. Expect this to succeed since
-  // the thread is already in an MTA apartment.
-  InitializeCOMOnWorker(TaskEnvironment::ThreadPoolCOMEnvironment::DEFAULT,
-                        ApartmentType::kMTA);
-
-  // Attempt to initialize an STA COM apartment. Expect this to fail since the
-  // thread is already in an MTA apartment.
-  EXPECT_CHECK_DEATH(InitializeCOMOnWorker(
-      TaskEnvironment::ThreadPoolCOMEnvironment::DEFAULT, ApartmentType::kSTA));
-}
-
-TEST_F(TaskEnvironmentTest, NoCOMEnvironment) {
-  // Attempt to initialize both MTA and STA COM apartments. Both should succeed
-  // when the thread is not already in an apartment.
-  InitializeCOMOnWorker(TaskEnvironment::ThreadPoolCOMEnvironment::NONE,
-                        ApartmentType::kMTA);
-  InitializeCOMOnWorker(TaskEnvironment::ThreadPoolCOMEnvironment::NONE,
-                        ApartmentType::kSTA);
-}
-#endif
-
 }  // namespace test
 }  // namespace base

chrome/chrome_cleaner/engines/broker/engine_requests_no_blocking_unittest.cc

@@ -437,9 +437,7 @@ class TestEngineRequestInvoker {
 };
 
 MULTIPROCESS_TEST_MAIN(EngineRequestsNoBlocking) {
-  // COM can't be initialized inside the sandbox.
-  base::test::TaskEnvironment task_environment(
-      base::test::TaskEnvironment::ThreadPoolCOMEnvironment::NONE);
+  base::test::TaskEnvironment task_environment;
 
   auto child_process = SetupSandboxedChildProcess();
   if (!child_process)
@@ -522,6 +520,11 @@ class EngineRequestsNoBlockingTest
     : public ::testing::TestWithParam<const char*> {};
 
 TEST_P(EngineRequestsNoBlockingTest, TestRequest) {
+  // All of these tests fail when run on win8 bots so return right away.
+  // TODO(crbug.com/947576): Find out why and re-enable them.
+  if (base::win::GetVersion() == base::win::Version::WIN8)
+    return;
+
   base::test::TaskEnvironment task_environment;
 
   // This event will be shared between the parent and child processes. The