Make ViewsTestBase::task_environment_ private.

This code jumped through a lot of complex hoops to allow subclasses to
provide a TestEnvironment of their own.  But no subclasses actually
want a test environment with a different lifetime than the parent's.
So just allow people to pass in a TestEnvironment instance instead.

This, coupled with providing an accessor for calling methods, allows
making the TaskEnvironment private instead of protected.

Bug: none
Change-Id: I923f5b232eb8fc01cc445e4d51cf45707d167ae7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2126093
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754560}

chrome/browser/ui/views/media_router/cast_dialog_no_sinks_view_unittest.cc

@@ -44,7 +44,7 @@ TEST_F(CastDialogNoSinksViewTest, SwitchViews) {
   EXPECT_TRUE(looking_for_sinks_view()->GetVisible());
   EXPECT_FALSE(help_icon_view());
 
-  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(3));
+  task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(3));
   // After three seconds, only the help icon view should be shown.
   EXPECT_FALSE(looking_for_sinks_view());
   EXPECT_TRUE(help_icon_view()->GetVisible());

chrome/browser/ui/views/tabs/tab_strip_unittest.cc

@@ -1035,13 +1035,13 @@ TEST_P(TabStripTest, AnimationOnTabAdd) {
   const int initial_width = tab_strip_->tab_at(1)->width();
   EXPECT_LT(initial_width, tab_strip_->tab_at(0)->width());
 
-  task_environment_.FastForwardBy(TabAnimation::kAnimationDuration / 2);
+  task_environment()->FastForwardBy(TabAnimation::kAnimationDuration / 2);
 
   EXPECT_GT(tab_strip_->tab_at(1)->width(), initial_width);
   EXPECT_LT(tab_strip_->tab_at(1)->width(), tab_strip_->tab_at(0)->width());
 
   // Fast-forward by more than enough to ensure the animation finishes.
-  task_environment_.FastForwardBy(TabAnimation::kAnimationDuration);
+  task_environment()->FastForwardBy(TabAnimation::kAnimationDuration);
 
   EXPECT_EQ(tab_strip_->tab_at(0)->width(), tab_strip_->tab_at(1)->width());
 }

chrome/test/views/chrome_views_test_base.cc

@@ -5,6 +5,13 @@
 #include "chrome/test/views/chrome_views_test_base.h"
 
 #include "chrome/test/views/chrome_test_views_delegate.h"
+#include "content/public/test/browser_task_environment.h"
+
+ChromeViewsTestBase::ChromeViewsTestBase()
+    : views::ViewsTestBase(std::unique_ptr<base::test::TaskEnvironment>(
+          std::make_unique<content::BrowserTaskEnvironment>(
+              content::BrowserTaskEnvironment::MainThreadType::UI,
+              content::BrowserTaskEnvironment::TimeSource::MOCK_TIME))) {}
 
 ChromeViewsTestBase::~ChromeViewsTestBase() = default;
 

chrome/test/views/chrome_views_test_base.h

@@ -5,46 +5,19 @@
 #ifndef CHROME_TEST_VIEWS_CHROME_VIEWS_TEST_BASE_H_
 #define CHROME_TEST_VIEWS_CHROME_VIEWS_TEST_BASE_H_
 
-#include <memory>
-
-#include "base/compiler_specific.h"
-#include "base/macros.h"
-#include "base/traits_bag.h"
-#include "content/public/test/browser_task_environment.h"
 #include "ui/views/test/views_test_base.h"
 
 // A base class for Chrome views unit tests. Changes the dependencies when they
 // need to be different than non-Chrome views.
 class ChromeViewsTestBase : public views::ViewsTestBase {
  public:
-  // Constructs a ChromeViewsTestBase with |traits| being forwarded to its
-  // BrowserTaskEnvironment. MainThreadType always defaults to UI and must not
-  // be specified. TimeSource defaults to MOCK_TIME but can be specified to
-  // override.
-  template <typename... TaskEnvironmentTraits>
-  NOINLINE explicit ChromeViewsTestBase(TaskEnvironmentTraits... traits)
-      : views::ViewsTestBase(
-            views::ViewsTestBase::SubclassManagesTaskEnvironment()),
-        task_environment_(
-            content::BrowserTaskEnvironment::MainThreadType::UI,
-            base::trait_helpers::GetEnum<
-                content::BrowserTaskEnvironment::TimeSource,
-                content::BrowserTaskEnvironment::TimeSource::MOCK_TIME>(
-                traits...),
-            base::trait_helpers::
-                Exclude<content::BrowserTaskEnvironment::TimeSource>::Filter(
-                    traits)...) {}
+  ChromeViewsTestBase();
+  ChromeViewsTestBase(const ChromeViewsTestBase&) = delete;
+  ChromeViewsTestBase& operator=(const ChromeViewsTestBase&) = delete;
   ~ChromeViewsTestBase() override;
 
   // views::ViewsTestBase:
   void SetUp() override;
-
- protected:
-  // Use this protected member directly to drive tasks posted within a
-  // ChromeViewsTestBase-based test.
-  content::BrowserTaskEnvironment task_environment_;
-
-  DISALLOW_COPY_AND_ASSIGN(ChromeViewsTestBase);
 };
 
 #endif  // CHROME_TEST_VIEWS_CHROME_VIEWS_TEST_BASE_H_

components/media_message_center/media_controls_progress_view_unittest.cc

@@ -120,8 +120,8 @@ TEST_F(MAYBE_MediaControlsProgressViewTest, UpdateProgress) {
             base::ASCIIToUTF16("05:00"));
   EXPECT_EQ(progress_view_->progress_bar_for_testing()->GetValue(), .5);
 
-  task_environment_->FastForwardBy(base::TimeDelta::FromSeconds(30));
-  task_environment_->RunUntilIdle();
+  task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(30));
+  task_environment()->RunUntilIdle();
 
   EXPECT_EQ(progress_view_->duration_for_testing(),
             base::ASCIIToUTF16("10:00"));
@@ -143,8 +143,8 @@ TEST_F(MAYBE_MediaControlsProgressViewTest, UpdateProgressFastPlayback) {
             base::ASCIIToUTF16("05:00"));
   EXPECT_EQ(progress_view_->progress_bar_for_testing()->GetValue(), .5);
 
-  task_environment_->FastForwardBy(base::TimeDelta::FromSeconds(15));
-  task_environment_->RunUntilIdle();
+  task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(15));
+  task_environment()->RunUntilIdle();
 
   EXPECT_EQ(progress_view_->duration_for_testing(),
             base::ASCIIToUTF16("10:00"));
@@ -166,8 +166,8 @@ TEST_F(MAYBE_MediaControlsProgressViewTest, UpdateProgressSlowPlayback) {
             base::ASCIIToUTF16("05:00"));
   EXPECT_EQ(progress_view_->progress_bar_for_testing()->GetValue(), .5);
 
-  task_environment_->FastForwardBy(base::TimeDelta::FromSeconds(60));
-  task_environment_->RunUntilIdle();
+  task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(60));
+  task_environment()->RunUntilIdle();
 
   EXPECT_EQ(progress_view_->duration_for_testing(),
             base::ASCIIToUTF16("10:00"));
@@ -189,8 +189,8 @@ TEST_F(MAYBE_MediaControlsProgressViewTest, UpdateProgressNegativePlayback) {
             base::ASCIIToUTF16("05:00"));
   EXPECT_EQ(progress_view_->progress_bar_for_testing()->GetValue(), .5);
 
-  task_environment_->FastForwardBy(base::TimeDelta::FromSeconds(30));
-  task_environment_->RunUntilIdle();
+  task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(30));
+  task_environment()->RunUntilIdle();
 
   EXPECT_EQ(progress_view_->duration_for_testing(),
             base::ASCIIToUTF16("10:00"));
@@ -213,8 +213,8 @@ TEST_F(MAYBE_MediaControlsProgressViewTest, UpdateProgressPastDuration) {
   EXPECT_EQ(progress_view_->progress_bar_for_testing()->GetValue(), .5);
 
   // Move forward in time past the duration.
-  task_environment_->FastForwardBy(base::TimeDelta::FromMinutes(6));
-  task_environment_->RunUntilIdle();
+  task_environment()->FastForwardBy(base::TimeDelta::FromMinutes(6));
+  task_environment()->RunUntilIdle();
 
   // Verify the progress does not go past the duration.
   EXPECT_EQ(progress_view_->duration_for_testing(),
@@ -238,8 +238,8 @@ TEST_F(MAYBE_MediaControlsProgressViewTest, UpdateProgressBeforeStart) {
   EXPECT_EQ(progress_view_->progress_bar_for_testing()->GetValue(), .5);
 
   // Move forward in time before the start using negative playback rate.
-  task_environment_->FastForwardBy(base::TimeDelta::FromMinutes(6));
-  task_environment_->RunUntilIdle();
+  task_environment()->FastForwardBy(base::TimeDelta::FromMinutes(6));
+  task_environment()->RunUntilIdle();
 
   // Verify the progress does not go below 0.
   EXPECT_EQ(progress_view_->duration_for_testing(),
@@ -262,8 +262,8 @@ TEST_F(MAYBE_MediaControlsProgressViewTest, UpdateProgressPaused) {
             base::ASCIIToUTF16("05:00"));
   EXPECT_EQ(progress_view_->progress_bar_for_testing()->GetValue(), .5);
 
-  task_environment_->FastForwardBy(base::TimeDelta::FromMinutes(6));
-  task_environment_->RunUntilIdle();
+  task_environment()->FastForwardBy(base::TimeDelta::FromMinutes(6));
+  task_environment()->RunUntilIdle();
 
   // Verify the progress does not change while media is paused.
   EXPECT_EQ(progress_view_->duration_for_testing(),

components/media_message_center/media_notification_view_impl_unittest.cc

@@ -306,8 +306,7 @@ class MediaNotificationViewImplTest : public views::ViewsTestBase {
   }
 
   void AdvanceClockMilliseconds(int milliseconds) {
-    ASSERT_TRUE(task_environment_.has_value());
-    task_environment_->FastForwardBy(
+    task_environment()->FastForwardBy(
         base::TimeDelta::FromMilliseconds(milliseconds));
   }
 

ui/message_center/views/notification_header_view_unittest.cc

@@ -66,22 +66,22 @@ TEST_F(NotificationHeaderViewTest, UpdatesTimestampOverTime) {
                 IDS_MESSAGE_NOTIFICATION_DURATION_HOURS_SHORTEST_FUTURE, 3),
             timestamp_view->GetText());
 
-  task_environment_->FastForwardBy(base::TimeDelta::FromHours(3));
-  task_environment_->RunUntilIdle();
+  task_environment()->FastForwardBy(base::TimeDelta::FromHours(3));
+  task_environment()->RunUntilIdle();
 
   EXPECT_EQ(l10n_util::GetPluralStringFUTF16(
                 IDS_MESSAGE_NOTIFICATION_DURATION_MINUTES_SHORTEST_FUTURE, 30),
             timestamp_view->GetText());
 
-  task_environment_->FastForwardBy(base::TimeDelta::FromMinutes(30));
-  task_environment_->RunUntilIdle();
+  task_environment()->FastForwardBy(base::TimeDelta::FromMinutes(30));
+  task_environment()->RunUntilIdle();
 
   EXPECT_EQ(
       l10n_util::GetStringUTF16(IDS_MESSAGE_NOTIFICATION_NOW_STRING_SHORTEST),
       timestamp_view->GetText());
 
-  task_environment_->FastForwardBy(base::TimeDelta::FromDays(2));
-  task_environment_->RunUntilIdle();
+  task_environment()->FastForwardBy(base::TimeDelta::FromDays(2));
+  task_environment()->RunUntilIdle();
 
   EXPECT_EQ(l10n_util::GetPluralStringFUTF16(
                 IDS_MESSAGE_NOTIFICATION_DURATION_DAYS_SHORTEST, 2),

ui/views/controls/webview/web_dialog_view_unittest.cc

@@ -50,11 +50,9 @@ class TestWebDialogViewWebDialogDelegate
 // Provides functionality to test a WebDialogView.
 class WebDialogViewUnitTest : public views::test::WidgetTest {
  public:
-  template <typename... TaskEnvironmentTraits>
-  NOINLINE explicit WebDialogViewUnitTest(TaskEnvironmentTraits&&... traits)
-      : views::test::WidgetTest(
-            views::test::WidgetTest::SubclassManagesTaskEnvironment()),
-        task_environment_(std::forward<TaskEnvironmentTraits>(traits)...) {}
+  WebDialogViewUnitTest()
+      : views::test::WidgetTest(std::unique_ptr<base::test::TaskEnvironment>(
+            std::make_unique<content::BrowserTaskEnvironment>())) {}
   ~WebDialogViewUnitTest() override = default;
 
   // testing::Test
@@ -127,9 +125,6 @@ class WebDialogViewUnitTest : public views::test::WidgetTest {
       widget_->OnKeyEvent(&event_copy);
   }
 
-  // TaskEnvironment must be created first
-  content::BrowserTaskEnvironment task_environment_;
-
  private:
   content::TestContentBrowserClient test_browser_client_;
   std::unique_ptr<content::TestBrowserContext> browser_context_;

ui/views/controls/webview/webview_unittest.cc

@@ -127,8 +127,9 @@ class WebViewTestWebContentsDelegate : public content::WebContentsDelegate {
 class WebViewUnitTest : public views::test::WidgetTest {
  public:
   WebViewUnitTest()
-      : views::test::WidgetTest(
-            views::ViewsTestBase::SubclassManagesTaskEnvironment()) {}
+      : views::test::WidgetTest(std::unique_ptr<base::test::TaskEnvironment>(
+            std::make_unique<content::BrowserTaskEnvironment>())) {}
+
   ~WebViewUnitTest() override = default;
 
   std::unique_ptr<content::WebContents> CreateWebContentsForWebView(
@@ -193,8 +194,6 @@ class WebViewUnitTest : public views::test::WidgetTest {
     web_view->fullscreen_native_view_for_testing_ = native_view;
   }
 
-  content::BrowserTaskEnvironment task_environment_;
-
  private:
   std::unique_ptr<content::RenderViewHostTestEnabler> rvh_enabler_;
   std::unique_ptr<content::TestBrowserContext> browser_context_;

ui/views/test/views_test_base.cc

@@ -64,7 +64,8 @@ bool InitializeVisuals() {
 }  // namespace
 
 ViewsTestBase::ViewsTestBase(
-    ViewsTestBase::SubclassManagesTaskEnvironment tag) {}
+    std::unique_ptr<base::test::TaskEnvironment> task_environment)
+    : task_environment_(std::move(task_environment)) {}
 
 ViewsTestBase::~ViewsTestBase() {
   CHECK(setup_called_)

ui/views/test/views_test_base.h

@@ -47,16 +47,13 @@ class ViewsTestBase : public PlatformTest {
   // specified.
   template <typename... TaskEnvironmentTraits>
   NOINLINE explicit ViewsTestBase(TaskEnvironmentTraits&&... traits)
-      : task_environment_(base::in_place,
+      : ViewsTestBase(std::make_unique<base::test::TaskEnvironment>(
             base::test::TaskEnvironment::MainThreadType::UI,
-                          std::forward<TaskEnvironmentTraits>(traits)...) {
-  }
+            std::forward<TaskEnvironmentTraits>(traits)...)) {}
 
-  // Alternatively a subclass may pass this tag to ask this ViewsTestBase not to
-  // instantiate a TaskEnvironment. The subclass is then responsible to
-  // instantiate one before ViewsTestBase::SetUp().
-  struct SubclassManagesTaskEnvironment {};
-  explicit ViewsTestBase(SubclassManagesTaskEnvironment tag);
+  // Alternatively a subclass may pass a TaskEnvironment directly.
+  explicit ViewsTestBase(
+      std::unique_ptr<base::test::TaskEnvironment> task_environment);
 
   ~ViewsTestBase() override;
 
@@ -85,6 +82,9 @@ class ViewsTestBase : public PlatformTest {
   void SimulateNativeDestroy(Widget* widget);
 
  protected:
+  base::test::TaskEnvironment* task_environment() {
+    return task_environment_.get();
+  }
   TestViewsDelegate* test_views_delegate() const {
     return test_helper_->test_views_delegate();
   }
@@ -121,16 +121,13 @@ class ViewsTestBase : public PlatformTest {
       const Widget::InitParams& init_params,
       internal::NativeWidgetDelegate* delegate);
 
- protected:
   Widget::InitParams CreateParamsForTestWidget(
       Widget::InitParams::Type type =
           Widget::InitParams::TYPE_WINDOW_FRAMELESS);
 
-  // Initialized first, destroyed last. Use this protected member directly from
-  // the test body to drive tasks posted within a ViewsTestBase-based test.
-  base::Optional<base::test::TaskEnvironment> task_environment_;
-
  private:
+  std::unique_ptr<base::test::TaskEnvironment> task_environment_;
+
   // Controls what type of widget will be created by default for a test (i.e.
   // when creating a Widget and leaving InitParams::native_widget unspecified).
   // kDefault will allow the system default to be used (typically

ui/views/test/widget_test.cc

@@ -16,6 +16,12 @@ void WidgetTest::WidgetCloser::operator()(Widget* widget) const {
   widget->CloseNow();
 }
 
+WidgetTest::WidgetTest() = default;
+
+WidgetTest::WidgetTest(
+    std::unique_ptr<base::test::TaskEnvironment> task_environment)
+    : ViewsTestBase(std::move(task_environment)) {}
+
 WidgetTest::~WidgetTest() = default;
 
 Widget* WidgetTest::CreateTopLevelPlatformWidget() {

ui/views/test/widget_test.h

@@ -46,14 +46,9 @@ class WidgetTest : public ViewsTestBase {
 
   using WidgetAutoclosePtr = std::unique_ptr<Widget, WidgetCloser>;
 
-  // Constructs an AshTestBase with |traits| being forwarded to its
-  // TaskEnvironment. |ViewsTestBase::SubclassManagesTaskEnvironment()|
-  // 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)...) {}
-
+  WidgetTest();
+  explicit WidgetTest(
+      std::unique_ptr<base::test::TaskEnvironment> task_environment);
   ~WidgetTest() override;
 
   // Create Widgets with |native_widget| in InitParams set to an instance of