Skip to content

Commit

Permalink
Call TearDown() from ~AshTestHelper().
Browse files Browse the repository at this point in the history
This means callers only need to call TearDown() if they need to do
something between teardown and destruction, which most don't; and they
cannot fail to call TearDown() (and thus leave objects alive).  So it's
both simpler and safer.

Bug: none
Change-Id: Iba7ad30e64dca62d060b8c572253117d7952db3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2124979
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754709}
  • Loading branch information
pkasting authored and Commit Bot committed Mar 30, 2020
1 parent bac8e1e commit 3b81c72
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 16 deletions.
1 change: 0 additions & 1 deletion ash/shell/content/client/shell_browser_main_parts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ void ShellBrowserMainParts::PostMainMessageLoopRun() {
example_app_list_client_.reset();
example_session_controller_client_.reset();

ash_test_helper_->TearDown();
ash_test_helper_.reset();

views_delegate_.reset();
Expand Down
3 changes: 3 additions & 0 deletions ash/test/ash_test_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ AshTestHelper::AshTestHelper(ConfigType config_type,
}

AshTestHelper::~AshTestHelper() {
if (app_list_test_helper_)
TearDown();

// Ensure the next test starts with a null display::Screen. This must be done
// here instead of in TearDown() since some tests test access to the Screen
// after the shell shuts down (which they use TearDown() to trigger).
Expand Down
3 changes: 2 additions & 1 deletion ash/test/ash_test_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ class AshTestHelper {
void SetUp();

// Tears down everything but the Screen instance, which some tests access
// after this point.
// after this point. This will be called automatically on destruction if it
// is not called manually earlier.
void TearDown();

aura::Window* GetContext();
Expand Down
12 changes: 3 additions & 9 deletions ash/test/ash_test_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,16 @@ class AshTestHelperTest : public testing::Test {

void SetUp() override {
testing::Test::SetUp();
ash_test_helper_ = std::make_unique<AshTestHelper>();
ash_test_helper_->SetUp();
ash_test_helper_.SetUp();
}

void TearDown() override {
ash_test_helper_->TearDown();
testing::Test::TearDown();
}

AshTestHelper* ash_test_helper() { return ash_test_helper_.get(); }
AshTestHelper* ash_test_helper() { return &ash_test_helper_; }

private:
base::test::TaskEnvironment task_environment_{
base::test::TaskEnvironment::MainThreadType::UI};

std::unique_ptr<AshTestHelper> ash_test_helper_;
AshTestHelper ash_test_helper_;

DISALLOW_COPY_AND_ASSIGN(AshTestHelperTest);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -744,9 +744,7 @@ class RelaunchNotificationControllerPlatformImplTest : public ::testing::Test {
RelaunchNotificationControllerPlatformImplTest()
: task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}

~RelaunchNotificationControllerPlatformImplTest() override {
ash_test_helper_.TearDown();
}
~RelaunchNotificationControllerPlatformImplTest() override = default;

void SetUp() override {
ash::AshTestHelper::InitParams init_params;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,7 @@ void WaylandClientTestHelper::TearDownOnUIThread(base::WaitableEvent* event) {
wm_helper_.reset();

ash::Shell::Get()->session_controller()->NotifyChromeTerminating();
ash_test_helper_->TearDown();
ash_test_helper_ = nullptr;
ash_test_helper_.reset();
xdg_temp_dir_ = nullptr;
event->Signal();
}
Expand Down

0 comments on commit 3b81c72

Please sign in to comment.