Reland "[TaskEnvironment] Standardize paradigm for classes forwarding traits"

This reverts commit eb651697.

Reason for revert: the culprit was another one of my CLs!

Original change's description:
> Revert "[TaskEnvironment] Standardize paradigm for classes forwarding traits"
> 
> This reverts commit 7472b906.
> 
> Reason for revert: sheriff suspects this caused a build break on win{,32}-archive-rel:
> 
> https://ci.chromium.org/p/chromium/builders/ci/win-archive-rel/4309
> 
> In file included from ../../chrome/chrome_cleaner/ipc/proto_chrome_prompt_ipc_unittest.cc:14:
> In file included from ../..\base/test/scoped_task_environment.h:15:
> In file included from ../..\base/task/lazy_task_runner.h:17:
> ../..\base/task/task_traits.h(263,5): error: static_assert failed due to requirement 'has_thread_pool ^ has_extension' "Traits must explicitly specify a destination (e.g. ThreadPool or a named thread like BrowserThread)"
>     static_assert(
>     ^
> ../..\chrome/chrome_cleaner/ipc/proto_chrome_prompt_ipc.h(108,39): note: in instantiation of function template specialization 'base::TaskTraits::TaskTraits<base::MayBlock, void>' requested here
>       base::CreateSequencedTaskRunner({base::MayBlock()});
>                                       ^
> 1 error generated.
> 
> Original change's description:
> > [TaskEnvironment] Standardize paradigm for classes forwarding traits
> > 
> > As discussed @
> > https://chromium-review.googlesource.com/c/chromium/src/+/1749538/6/ash/test/ash_test_base.h#84
> > 
> > Usage of explicit everywhere also avoids implicit copy construction
> > and is the reason for changes to a few unit tests that previously
> > used operator= to initialize their task environments.
> > 
> > TBR=dcheng@chromium.org
> > (for widespread mechanical change)
> > 
> > Bug: 992483
> > Change-Id: I321576674f3b387a6f5069e8e71eebf41c98e7ed
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1758338
> > Reviewed-by: Gabriel Charette <gab@chromium.org>
> > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > Commit-Queue: Gabriel Charette <gab@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#689098}
> 
> TBR=dcheng@chromium.org,gab@chromium.org
> 
> Change-Id: I69b71b220ab583bc3a18e8fb07213fa1b6a75b50
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 992483
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1762475
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#689122}

TBR=ellyjones@chromium.org,dcheng@chromium.org,gab@chromium.org

Change-Id: Ibc0ab9c4df6dcbc4a65fd26e0356c78e8b0f90b3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 992483
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1764390Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689191}

ash/test/ash_test_base.h

@@ -9,6 +9,7 @@
 
 #include <memory>
 #include <string>
+#include <utility>
 
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/session/test_session_controller_client.h"
@@ -77,10 +78,10 @@ class AshTestBase : public testing::Test {
   // TaskEnvironment. MainThreadType always defaults to UI and must not be
   // specified.
   template <typename... TaskEnvironmentTraits>
-  NOINLINE explicit AshTestBase(TaskEnvironmentTraits... traits)
+  NOINLINE explicit AshTestBase(TaskEnvironmentTraits&&... traits)
       : task_environment_(base::in_place,
                           base::test::TaskEnvironment::MainThreadType::UI,
-                          traits...) {}
+                          std::forward<TaskEnvironmentTraits>(traits)...) {}
 
   // Alternatively a subclass may pass this tag to ask this AshTestBase not to
   // instantiate a TaskEnvironment. The subclass is then responsible to

base/test/scoped_task_environment.h

@@ -162,20 +162,21 @@ class TaskEnvironment {
 
   // Constructor accepts zero or more traits which customize the testing
   // environment.
-  template <class... ArgTypes,
+  template <typename... TaskEnvironmentTraits,
             class CheckArgumentsAreValid = std::enable_if_t<
-                trait_helpers::AreValidTraits<ValidTrait, ArgTypes...>::value>>
-  NOINLINE TaskEnvironment(ArgTypes... args)
+                trait_helpers::AreValidTraits<ValidTrait,
+                                              TaskEnvironmentTraits...>::value>>
+  NOINLINE explicit TaskEnvironment(TaskEnvironmentTraits... traits)
       : TaskEnvironment(
-            trait_helpers::GetEnum<TimeSource, TimeSource::DEFAULT>(args...),
+            trait_helpers::GetEnum<TimeSource, TimeSource::DEFAULT>(traits...),
             trait_helpers::GetEnum<MainThreadType, MainThreadType::DEFAULT>(
-                args...),
+                traits...),
             trait_helpers::GetEnum<ThreadPoolExecutionMode,
-                                   ThreadPoolExecutionMode::DEFAULT>(args...),
+                                   ThreadPoolExecutionMode::DEFAULT>(traits...),
             trait_helpers::GetEnum<ThreadingMode, ThreadingMode::DEFAULT>(
-                args...),
+                traits...),
             trait_helpers::HasTrait<SubclassCreatesDefaultTaskRunner,
-                                    ArgTypes...>(),
+                                    TaskEnvironmentTraits...>(),
             trait_helpers::NotATraitTag()) {}
 
   // Waits until no undelayed ThreadPool tasks remain. Then, unregisters the

chrome/browser/chromeos/policy/cached_policy_key_loader_chromeos_unittest.cc

@@ -69,7 +69,7 @@ class CachedPolicyKeyLoaderTest : public testing::Test {
         &CachedPolicyKeyLoaderTest::OnPolicyKeyLoaded, base::Unretained(this)));
   }
 
-  base::test::TaskEnvironment task_environment_ = {
+  base::test::TaskEnvironment task_environment_{
       base::test::TaskEnvironment::MainThreadType::UI};
   chromeos::FakeCryptohomeClient cryptohome_client_;
   const AccountId account_id_ = AccountId::FromUserEmail(kTestUserName);

chrome/browser/chromeos/policy/pre_signin_policy_fetcher_unittest.cc

@@ -163,7 +163,7 @@ class PreSigninPolicyFetcherTestBase : public testing::Test {
     task_environment_.RunUntilIdle();
   }
 
-  base::test::TaskEnvironment task_environment_ = {
+  base::test::TaskEnvironment task_environment_{
       base::test::TaskEnvironment::MainThreadType::UI};
   std::unique_ptr<chromeos::FakeCryptohomeClient> cryptohome_client_;
   chromeos::FakeSessionManagerClient session_manager_client_;

chrome/browser/optimization_guide/optimization_guide_hints_manager_unittest.cc

@@ -320,7 +320,7 @@ class OptimizationGuideHintsManagerTest
                               serialized_config.size()));
   }
 
-  content::BrowserTaskEnvironment task_environment_ = {
+  content::BrowserTaskEnvironment task_environment_{
       base::test::TaskEnvironment::MainThreadType::UI,
       base::test::TaskEnvironment::TimeSource::MOCK_TIME};
   TestingProfile testing_profile_;

chrome/test/base/browser_with_test_window_test.h

@@ -88,17 +88,19 @@ class BrowserWithTestWindowTest : public testing::Test {
   // the initial window will be a tabbed browser created on the native desktop,
   // which is not a hosted app.
   template <
-      class... ArgTypes,
+      typename... TaskEnvironmentTraits,
       class CheckArgumentsAreValid = std::enable_if_t<
-          base::trait_helpers::AreValidTraits<ValidTraits, ArgTypes...>::value>>
-  NOINLINE BrowserWithTestWindowTest(const ArgTypes... args)
+          base::trait_helpers::AreValidTraits<ValidTraits,
+                                              TaskEnvironmentTraits...>::value>>
+  NOINLINE explicit BrowserWithTestWindowTest(TaskEnvironmentTraits... traits)
       : BrowserWithTestWindowTest(
             std::make_unique<content::BrowserTaskEnvironment>(
                 base::trait_helpers::Exclude<HostedApp, Browser::Type>::Filter(
-                    args)...),
+                    traits)...),
             base::trait_helpers::GetEnum<Browser::Type, Browser::TYPE_NORMAL>(
-                args...),
-            base::trait_helpers::HasTrait<HostedApp, ArgTypes...>()) {}
+                traits...),
+            base::trait_helpers::HasTrait<HostedApp,
+                                          TaskEnvironmentTraits...>()) {}
 
   ~BrowserWithTestWindowTest() override;
 

chrome/test/base/chrome_render_view_host_test_harness.h

@@ -6,6 +6,7 @@
 #define CHROME_TEST_BASE_CHROME_RENDER_VIEW_HOST_TEST_HARNESS_H_
 
 #include <memory>
+#include <utility>
 #include <vector>
 
 #include "base/compiler_specific.h"
@@ -20,9 +21,10 @@ class ChromeRenderViewHostTestHarness
  public:
   // Construct a ChromeRenderViewHostTestHarness with zero or more arguments
   // passed to content::RenderViewHostTestHarness.
-  template <typename... Args>
-  constexpr ChromeRenderViewHostTestHarness(Args... args)
-      : content::RenderViewHostTestHarness(args...) {}
+  template <typename... TaskEnvironmentTraits>
+  explicit ChromeRenderViewHostTestHarness(TaskEnvironmentTraits&&... traits)
+      : content::RenderViewHostTestHarness(
+            std::forward<TaskEnvironmentTraits>(traits)...) {}
 
   ~ChromeRenderViewHostTestHarness() override;
 

chrome/test/views/chrome_views_test_base.h

@@ -26,7 +26,7 @@ class ChromeViewsTestBase : public views::ViewsTestBase {
       : views::ViewsTestBase(
             views::ViewsTestBase::SubclassManagesTaskEnvironment()),
         task_environment_(
-            base::test::TaskEnvironment::MainThreadType::UI,
+            content::BrowserTaskEnvironment::MainThreadType::UI,
             base::trait_helpers::GetEnum<
                 content::BrowserTaskEnvironment::TimeSource,
                 content::BrowserTaskEnvironment::TimeSource::MOCK_TIME>(

components/exo/data_source_unittest.cc

@@ -20,7 +20,7 @@ constexpr char kTestData[] = "Test Data";
 
 class DataSourceTest : public testing::Test {
  protected:
-  base::test::TaskEnvironment task_environment_ = {
+  base::test::TaskEnvironment task_environment_{
       base::test::TaskEnvironment::MainThreadType::DEFAULT,
       base::test::TaskEnvironment::ThreadPoolExecutionMode::ASYNC};
 };

content/public/test/test_browser_thread_bundle.h

@@ -48,7 +48,7 @@ class TestBrowserThread;
 //
 // ... until there are no undelayed tasks in the shared message loop, in
 // ThreadPool (excluding tasks not posted from the shared message loop's thread
-// or ThreadPool): browser_task_environment.RunUntilIdle();
+// or ThreadPool): task_environment.RunUntilIdle();
 //
 // ... until a condition is met:
 //    base::RunLoop run_loop;
@@ -89,7 +89,7 @@ class TestBrowserThread;
 //     // instead of private visibility will allow controlling the task
 //     // environment (e.g. clock --see base::test::TaskEnvironment for
 //     // details).
-//     content::BrowserTaskEnvironment browser_task_environment_;
+//     content::BrowserTaskEnvironment task_environment_;
 //
 //     // Other members go here (or further below in private section.)
 //   };
@@ -101,8 +101,10 @@ class TestBrowserThread;
 //     // Constructs a FooBase with |traits| being forwarded to its
 //     // TaskEnvironment.
 //     template <typename... TaskEnvironmentTraits>
-//     explicit FooBase(TaskEnvironmentTraits... traits)
-//         : task_environment_(base::in_place, traits...) {}
+//     explicit FooBase(TaskEnvironmentTraits&&... traits)
+//         : task_environment_(
+//               base::in_place,
+//               std::forward<TaskEnvironmentTraits>(traits)...) {}
 //
 //     // Alternatively a subclass may pass this tag to ask this FooBase not to
 //     // instantiate a TaskEnvironment. The subclass is then responsible
@@ -118,14 +120,15 @@ class TestBrowserThread;
 //
 //   class ChromeFooBase : public FooBase {
 //    public:
-//     explicit ChromeFooBase(TaskEnvironmentTraits... traits)
+//     explicit ChromeFooBase(TaskEnvironmentTraits&&... traits)
 //         : FooBase(FooBase::SubclassManagesTaskEnvironment()),
-//           browser_task_environment_(traits...) {}
+//           task_environment_(
+//               std::forward<TaskEnvironmentTraits>(traits)...) {}
 //
 //    protected:
 //     // Use this protected member directly to drive tasks posted within a
 //     // ChromeFooBase-based test.
-//     content::BrowserTaskEnvironment browser_task_environment_;
+//     content::BrowserTaskEnvironment task_environment_;
 //   };
 // See views::ViewsTestBase / ChromeViewsTestBase for a real-world example.
 class BrowserTaskEnvironment : public base::test::TaskEnvironment {
@@ -148,19 +151,20 @@ class BrowserTaskEnvironment : public base::test::TaskEnvironment {
   // TaskEnvironment the default MainThreadType for
   // BrowserTaskEnvironment is MainThreadType::UI.
   template <
-      class... ArgTypes,
+      typename... TaskEnvironmentTraits,
       class CheckArgumentsAreValid = std::enable_if_t<
-          base::trait_helpers::AreValidTraits<ValidTraits, ArgTypes...>::value>>
-  NOINLINE BrowserTaskEnvironment(const ArgTypes... args)
+          base::trait_helpers::AreValidTraits<ValidTraits,
+                                              TaskEnvironmentTraits...>::value>>
+  NOINLINE explicit BrowserTaskEnvironment(TaskEnvironmentTraits... traits)
       : BrowserTaskEnvironment(
             base::test::TaskEnvironment(
                 SubclassCreatesDefaultTaskRunner{},
                 base::trait_helpers::GetEnum<MainThreadType,
-                                             MainThreadType::UI>(args...),
+                                             MainThreadType::UI>(traits...),
                 base::trait_helpers::Exclude<MainThreadType, Options>::Filter(
-                    args)...),
+                    traits)...),
             UseRealIOThread(
-                base::trait_helpers::GetOptionalEnum<Options>(args...))) {}
+                base::trait_helpers::GetOptionalEnum<Options>(traits...))) {}
 
   // Flush the IO thread. Replacement for RunLoop::RunUntilIdle() for tests that
   // have a REAL_IO_THREAD. As with TaskEnvironment::RunUntilIdle() prefer using

content/public/test/test_renderer_host.h

@@ -9,6 +9,7 @@
 
 #include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/macros.h"
@@ -163,12 +164,12 @@ class RenderViewHostTestEnabler {
 // RenderViewHostTestHarness ---------------------------------------------------
 class RenderViewHostTestHarness : public testing::Test {
  public:
-  // Constructs a RenderViewHostTestHarness which uses |args| to initialize its
-  // BrowserTaskEnvironment.
-  template <typename... Args>
-  RenderViewHostTestHarness(Args... args)
-      : RenderViewHostTestHarness(
-            std::make_unique<BrowserTaskEnvironment>(args...)) {}
+  // Constructs a RenderViewHostTestHarness which uses |traits| to initialize
+  // its BrowserTaskEnvironment.
+  template <typename... TaskEnvironmentTraits>
+  explicit RenderViewHostTestHarness(TaskEnvironmentTraits&&... traits)
+      : RenderViewHostTestHarness(std::make_unique<BrowserTaskEnvironment>(
+            std::forward<TaskEnvironmentTraits>(traits)...)) {}
 
   ~RenderViewHostTestHarness() override;
 

ui/views/test/views_test_base.h

@@ -6,6 +6,7 @@
 #define UI_VIEWS_TEST_VIEWS_TEST_BASE_H_
 
 #include <memory>
+#include <utility>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
@@ -45,10 +46,10 @@ class ViewsTestBase : public PlatformTest {
   // TaskEnvironment. MainThreadType always defaults to UI and must not be
   // specified.
   template <typename... TaskEnvironmentTraits>
-  NOINLINE explicit ViewsTestBase(TaskEnvironmentTraits... traits)
+  NOINLINE explicit ViewsTestBase(TaskEnvironmentTraits&&... traits)
       : task_environment_(base::in_place,
                           base::test::TaskEnvironment::MainThreadType::UI,
-                          traits...) {
+                          std::forward<TaskEnvironmentTraits>(traits)...) {
     // MaterialDesignController is initialized here instead of in SetUp because
     // a subclass might construct a MaterialDesignControllerTestAPI as a member
     // to override the value, and this must happen first.

ui/views/test/widget_test.h

@@ -6,6 +6,7 @@
 #define UI_VIEWS_TEST_WIDGET_TEST_H_
 
 #include <memory>
+#include <utility>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
@@ -50,8 +51,8 @@ class WidgetTest : public ViewsTestBase {
   // can also be passed as a sole trait to indicate that this WidgetTest's
   // subclass will manage the task environment.
   template <typename... TaskEnvironmentTraits>
-  NOINLINE explicit WidgetTest(TaskEnvironmentTraits... traits)
-      : ViewsTestBase(traits...) {}
+  NOINLINE explicit WidgetTest(TaskEnvironmentTraits&&... traits)
+      : ViewsTestBase(std::forward<TaskEnvironmentTraits>(traits)...) {}
 
   ~WidgetTest() override;