Commit eb651697 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

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/+/1762475Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689122}
parent b39b132c
......@@ -9,7 +9,6 @@
#include <memory>
#include <string>
#include <utility>
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/session/test_session_controller_client.h"
......@@ -78,10 +77,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,
std::forward<TaskEnvironmentTraits>(traits)...) {}
traits...) {}
// Alternatively a subclass may pass this tag to ask this AshTestBase not to
// instantiate a TaskEnvironment. The subclass is then responsible to
......
......@@ -162,21 +162,20 @@ class TaskEnvironment {
// Constructor accepts zero or more traits which customize the testing
// environment.
template <typename... TaskEnvironmentTraits,
template <class... ArgTypes,
class CheckArgumentsAreValid = std::enable_if_t<
trait_helpers::AreValidTraits<ValidTrait,
TaskEnvironmentTraits...>::value>>
NOINLINE explicit TaskEnvironment(TaskEnvironmentTraits... traits)
trait_helpers::AreValidTraits<ValidTrait, ArgTypes...>::value>>
NOINLINE TaskEnvironment(ArgTypes... args)
: TaskEnvironment(
trait_helpers::GetEnum<TimeSource, TimeSource::DEFAULT>(traits...),
trait_helpers::GetEnum<TimeSource, TimeSource::DEFAULT>(args...),
trait_helpers::GetEnum<MainThreadType, MainThreadType::DEFAULT>(
traits...),
args...),
trait_helpers::GetEnum<ThreadPoolExecutionMode,
ThreadPoolExecutionMode::DEFAULT>(traits...),
ThreadPoolExecutionMode::DEFAULT>(args...),
trait_helpers::GetEnum<ThreadingMode, ThreadingMode::DEFAULT>(
traits...),
args...),
trait_helpers::HasTrait<SubclassCreatesDefaultTaskRunner,
TaskEnvironmentTraits...>(),
ArgTypes...>(),
trait_helpers::NotATraitTag()) {}
// Waits until no undelayed ThreadPool tasks remain. Then, unregisters the
......
......@@ -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);
......
......@@ -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_;
......
......@@ -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_;
......
......@@ -88,19 +88,17 @@ 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 <
typename... TaskEnvironmentTraits,
class... ArgTypes,
class CheckArgumentsAreValid = std::enable_if_t<
base::trait_helpers::AreValidTraits<ValidTraits,
TaskEnvironmentTraits...>::value>>
NOINLINE explicit BrowserWithTestWindowTest(TaskEnvironmentTraits... traits)
base::trait_helpers::AreValidTraits<ValidTraits, ArgTypes...>::value>>
NOINLINE BrowserWithTestWindowTest(const ArgTypes... args)
: BrowserWithTestWindowTest(
std::make_unique<content::BrowserTaskEnvironment>(
base::trait_helpers::Exclude<HostedApp, Browser::Type>::Filter(
traits)...),
args)...),
base::trait_helpers::GetEnum<Browser::Type, Browser::TYPE_NORMAL>(
traits...),
base::trait_helpers::HasTrait<HostedApp,
TaskEnvironmentTraits...>()) {}
args...),
base::trait_helpers::HasTrait<HostedApp, ArgTypes...>()) {}
~BrowserWithTestWindowTest() override;
......
......@@ -6,7 +6,6 @@
#define CHROME_TEST_BASE_CHROME_RENDER_VIEW_HOST_TEST_HARNESS_H_
#include <memory>
#include <utility>
#include <vector>
#include "base/compiler_specific.h"
......@@ -21,10 +20,9 @@ class ChromeRenderViewHostTestHarness
public:
// Construct a ChromeRenderViewHostTestHarness with zero or more arguments
// passed to content::RenderViewHostTestHarness.
template <typename... TaskEnvironmentTraits>
explicit ChromeRenderViewHostTestHarness(TaskEnvironmentTraits&&... traits)
: content::RenderViewHostTestHarness(
std::forward<TaskEnvironmentTraits>(traits)...) {}
template <typename... Args>
constexpr ChromeRenderViewHostTestHarness(Args... args)
: content::RenderViewHostTestHarness(args...) {}
~ChromeRenderViewHostTestHarness() override;
......
......@@ -26,7 +26,7 @@ class ChromeViewsTestBase : public views::ViewsTestBase {
: views::ViewsTestBase(
views::ViewsTestBase::SubclassManagesTaskEnvironment()),
task_environment_(
content::BrowserTaskEnvironment::MainThreadType::UI,
base::test::TaskEnvironment::MainThreadType::UI,
base::trait_helpers::GetEnum<
content::BrowserTaskEnvironment::TimeSource,
content::BrowserTaskEnvironment::TimeSource::MOCK_TIME>(
......
......@@ -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};
};
......
......@@ -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): task_environment.RunUntilIdle();
// or ThreadPool): browser_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 task_environment_;
// content::BrowserTaskEnvironment browser_task_environment_;
//
// // Other members go here (or further below in private section.)
// };
......@@ -101,10 +101,8 @@ class TestBrowserThread;
// // Constructs a FooBase with |traits| being forwarded to its
// // TaskEnvironment.
// template <typename... TaskEnvironmentTraits>
// explicit FooBase(TaskEnvironmentTraits&&... traits)
// : task_environment_(
// base::in_place,
// std::forward<TaskEnvironmentTraits>(traits)...) {}
// explicit FooBase(TaskEnvironmentTraits... traits)
// : task_environment_(base::in_place, traits...) {}
//
// // Alternatively a subclass may pass this tag to ask this FooBase not to
// // instantiate a TaskEnvironment. The subclass is then responsible
......@@ -120,15 +118,14 @@ class TestBrowserThread;
//
// class ChromeFooBase : public FooBase {
// public:
// explicit ChromeFooBase(TaskEnvironmentTraits&&... traits)
// explicit ChromeFooBase(TaskEnvironmentTraits... traits)
// : FooBase(FooBase::SubclassManagesTaskEnvironment()),
// task_environment_(
// std::forward<TaskEnvironmentTraits>(traits)...) {}
// browser_task_environment_(traits...) {}
//
// protected:
// // Use this protected member directly to drive tasks posted within a
// // ChromeFooBase-based test.
// content::BrowserTaskEnvironment task_environment_;
// content::BrowserTaskEnvironment browser_task_environment_;
// };
// See views::ViewsTestBase / ChromeViewsTestBase for a real-world example.
class BrowserTaskEnvironment : public base::test::TaskEnvironment {
......@@ -151,20 +148,19 @@ class BrowserTaskEnvironment : public base::test::TaskEnvironment {
// TaskEnvironment the default MainThreadType for
// BrowserTaskEnvironment is MainThreadType::UI.
template <
typename... TaskEnvironmentTraits,
class... ArgTypes,
class CheckArgumentsAreValid = std::enable_if_t<
base::trait_helpers::AreValidTraits<ValidTraits,
TaskEnvironmentTraits...>::value>>
NOINLINE explicit BrowserTaskEnvironment(TaskEnvironmentTraits... traits)
base::trait_helpers::AreValidTraits<ValidTraits, ArgTypes...>::value>>
NOINLINE BrowserTaskEnvironment(const ArgTypes... args)
: BrowserTaskEnvironment(
base::test::TaskEnvironment(
SubclassCreatesDefaultTaskRunner{},
base::trait_helpers::GetEnum<MainThreadType,
MainThreadType::UI>(traits...),
MainThreadType::UI>(args...),
base::trait_helpers::Exclude<MainThreadType, Options>::Filter(
traits)...),
args)...),
UseRealIOThread(
base::trait_helpers::GetOptionalEnum<Options>(traits...))) {}
base::trait_helpers::GetOptionalEnum<Options>(args...))) {}
// Flush the IO thread. Replacement for RunLoop::RunUntilIdle() for tests that
// have a REAL_IO_THREAD. As with TaskEnvironment::RunUntilIdle() prefer using
......
......@@ -9,7 +9,6 @@
#include <memory>
#include <string>
#include <utility>
#include <vector>
#include "base/macros.h"
......@@ -164,12 +163,12 @@ class RenderViewHostTestEnabler {
// RenderViewHostTestHarness ---------------------------------------------------
class RenderViewHostTestHarness : public testing::Test {
public:
// 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)...)) {}
// Constructs a RenderViewHostTestHarness which uses |args| to initialize its
// BrowserTaskEnvironment.
template <typename... Args>
RenderViewHostTestHarness(Args... args)
: RenderViewHostTestHarness(
std::make_unique<BrowserTaskEnvironment>(args...)) {}
~RenderViewHostTestHarness() override;
......
......@@ -6,7 +6,6 @@
#define UI_VIEWS_TEST_VIEWS_TEST_BASE_H_
#include <memory>
#include <utility>
#include "base/compiler_specific.h"
#include "base/macros.h"
......@@ -46,10 +45,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,
std::forward<TaskEnvironmentTraits>(traits)...) {
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.
......
......@@ -6,7 +6,6 @@
#define UI_VIEWS_TEST_WIDGET_TEST_H_
#include <memory>
#include <utility>
#include "base/compiler_specific.h"
#include "base/macros.h"
......@@ -51,8 +50,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(std::forward<TaskEnvironmentTraits>(traits)...) {}
NOINLINE explicit WidgetTest(TaskEnvironmentTraits... traits)
: ViewsTestBase(traits...) {}
~WidgetTest() override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment