Commit 0beb1151 authored by Wojciech Dzierżanowski's avatar Wojciech Dzierżanowski Committed by Commit Bot

Remove TestingProfile teardown workaround from CRVHTH

This reverts
https://chromium-review.googlesource.com/c/chromium/src/+/1685372, which
has been obsoleted by
https://chromium-review.googlesource.com/c/chromium/src/+/2332703.  The
profile directory is now guaranteed to outlive both TestingProfile and
TaskEnvironment.

The revert introduced a failure in
TabManagerTest.IsTabRestoredInForeground: PathDeleterOnTestEnd was not
able to delete the profile directory on Windows.  This is fixed here
by getting rid of TestMockTimeTaskRunner in favor of using the MOCK_TIME
trait for TaskEnvironment.  This is the preferred way of mocking time
and it makes the test simpler.

Bug: 978442
Change-Id: I3810d2ae57911f5b2d3337f954be5f5f360d7d4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2416354Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Commit-Queue: Wojciech Dzierżanowski <wdzierzanowski@opera.com>
Cr-Commit-Position: refs/heads/master@{#809729}
parent fa8d19fe
...@@ -17,13 +17,8 @@ ...@@ -17,13 +17,8 @@
#include "base/metrics/field_trial.h" #include "base/metrics/field_trial.h"
#include "base/notreached.h" #include "base/notreached.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/task/current_thread.h"
#include "base/test/mock_entropy_provider.h" #include "base/test/mock_entropy_provider.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/simple_test_tick_clock.h"
#include "base/test/test_mock_time_task_runner.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/tick_clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
...@@ -113,15 +108,10 @@ class FakeOfflineNetworkChangeNotifier : public net::NetworkChangeNotifier { ...@@ -113,15 +108,10 @@ class FakeOfflineNetworkChangeNotifier : public net::NetworkChangeNotifier {
class TabManagerTest : public ChromeRenderViewHostTestHarness { class TabManagerTest : public ChromeRenderViewHostTestHarness {
public: public:
TabManagerTest() TabManagerTest()
: scoped_context_( : ChromeRenderViewHostTestHarness(
std::make_unique<base::TestMockTimeTaskRunner::ScopedContext>( base::test::TaskEnvironment::TimeSource::MOCK_TIME) {
task_runner_)),
scoped_set_tick_clock_for_testing_(task_runner_->GetMockTickClock()),
previous_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
base::CurrentThread::Get()->SetTaskRunner(task_runner_);
// Start with a non-zero time. // Start with a non-zero time.
task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(42)); task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(42));
} }
std::unique_ptr<WebContents> CreateWebContents() { std::unique_ptr<WebContents> CreateWebContents() {
...@@ -131,9 +121,6 @@ class TabManagerTest : public ChromeRenderViewHostTestHarness { ...@@ -131,9 +121,6 @@ class TabManagerTest : public ChromeRenderViewHostTestHarness {
content::WebContentsTester::For(web_contents.get()) content::WebContentsTester::For(web_contents.get())
->NavigateAndCommit(GURL("https://www.example.com")); ->NavigateAndCommit(GURL("https://www.example.com"));
base::RepeatingClosure run_loop_cb = base::BindRepeating(
&base::TestMockTimeTaskRunner::RunUntilIdle, task_runner_);
return web_contents; return web_contents;
} }
...@@ -171,8 +158,6 @@ class TabManagerTest : public ChromeRenderViewHostTestHarness { ...@@ -171,8 +158,6 @@ class TabManagerTest : public ChromeRenderViewHostTestHarness {
void TearDown() override { void TearDown() override {
ResetState(); ResetState();
base::CurrentThread::Get()->SetTaskRunner(std::move(previous_task_runner_));
scoped_context_.reset();
ChromeRenderViewHostTestHarness::TearDown(); ChromeRenderViewHostTestHarness::TearDown();
} }
...@@ -263,11 +248,6 @@ class TabManagerTest : public ChromeRenderViewHostTestHarness { ...@@ -263,11 +248,6 @@ class TabManagerTest : public ChromeRenderViewHostTestHarness {
} }
TabManager* tab_manager_ = nullptr; TabManager* tab_manager_ = nullptr;
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_ =
base::MakeRefCounted<base::TestMockTimeTaskRunner>();
std::unique_ptr<base::TestMockTimeTaskRunner::ScopedContext> scoped_context_;
ScopedSetTickClockForTesting scoped_set_tick_clock_for_testing_;
scoped_refptr<base::SingleThreadTaskRunner> previous_task_runner_;
std::unique_ptr<BackgroundTabNavigationThrottle> throttle1_; std::unique_ptr<BackgroundTabNavigationThrottle> throttle1_;
std::unique_ptr<BackgroundTabNavigationThrottle> throttle2_; std::unique_ptr<BackgroundTabNavigationThrottle> throttle2_;
std::unique_ptr<BackgroundTabNavigationThrottle> throttle3_; std::unique_ptr<BackgroundTabNavigationThrottle> throttle3_;
...@@ -363,7 +343,7 @@ TEST_F(TabManagerTest, MAYBE_DiscardTabWithNonVisibleTabs) { ...@@ -363,7 +343,7 @@ TEST_F(TabManagerTest, MAYBE_DiscardTabWithNonVisibleTabs) {
tab_strip2->GetWebContentsAt(1)->WasHidden(); tab_strip2->GetWebContentsAt(1)->WasHidden();
// Advance time enough that the tabs are urgent discardable. // Advance time enough that the tabs are urgent discardable.
task_runner_->AdvanceMockTickClock(kBackgroundUrgentProtectionTime); task_environment()->AdvanceClock(kBackgroundUrgentProtectionTime);
for (int i = 0; i < 4; ++i) for (int i = 0; i < 4; ++i)
tab_manager_->DiscardTab(LifecycleUnitDiscardReason::URGENT); tab_manager_->DiscardTab(LifecycleUnitDiscardReason::URGENT);
...@@ -517,7 +497,7 @@ TEST_F(TabManagerTest, TimeoutWhenLoadingBackgroundTabs) { ...@@ -517,7 +497,7 @@ TEST_F(TabManagerTest, TimeoutWhenLoadingBackgroundTabs) {
// Simulate timeout when loading the 1st tab. TabManager should start loading // Simulate timeout when loading the 1st tab. TabManager should start loading
// the 2nd tab. // the 2nd tab.
task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(10)); task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(10));
EXPECT_TRUE(tab_manager_->IsTabLoadingForTest(contents1_.get())); EXPECT_TRUE(tab_manager_->IsTabLoadingForTest(contents1_.get()));
EXPECT_TRUE(tab_manager_->IsTabLoadingForTest(contents2_.get())); EXPECT_TRUE(tab_manager_->IsTabLoadingForTest(contents2_.get()));
...@@ -527,7 +507,7 @@ TEST_F(TabManagerTest, TimeoutWhenLoadingBackgroundTabs) { ...@@ -527,7 +507,7 @@ TEST_F(TabManagerTest, TimeoutWhenLoadingBackgroundTabs) {
EXPECT_TRUE(tab_manager_->IsNavigationDelayedForTest(nav_handle3_.get())); EXPECT_TRUE(tab_manager_->IsNavigationDelayedForTest(nav_handle3_.get()));
// Simulate timeout again. TabManager should start loading the 3rd tab. // Simulate timeout again. TabManager should start loading the 3rd tab.
task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(10)); task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(10));
EXPECT_TRUE(tab_manager_->IsTabLoadingForTest(contents1_.get())); EXPECT_TRUE(tab_manager_->IsTabLoadingForTest(contents1_.get()));
EXPECT_TRUE(tab_manager_->IsTabLoadingForTest(contents2_.get())); EXPECT_TRUE(tab_manager_->IsTabLoadingForTest(contents2_.get()));
...@@ -560,7 +540,7 @@ TEST_F(TabManagerTest, BackgroundTabLoadingMode) { ...@@ -560,7 +540,7 @@ TEST_F(TabManagerTest, BackgroundTabLoadingMode) {
tab_manager_->background_tab_loading_mode_); tab_manager_->background_tab_loading_mode_);
// Simulate timeout when loading the 1st tab. // Simulate timeout when loading the 1st tab.
task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(10)); task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(10));
// Tab 2 and 3 are still pending because of the paused loading mode. // Tab 2 and 3 are still pending because of the paused loading mode.
EXPECT_FALSE(tab_manager_->IsTabLoadingForTest(contents2_.get())); EXPECT_FALSE(tab_manager_->IsTabLoadingForTest(contents2_.get()));
...@@ -924,7 +904,7 @@ TEST_F(TabManagerTest, GetSortedLifecycleUnits) { ...@@ -924,7 +904,7 @@ TEST_F(TabManagerTest, GetSortedLifecycleUnits) {
const int num_of_tabs_to_test = 20; const int num_of_tabs_to_test = 20;
for (int i = 0; i < num_of_tabs_to_test; ++i) { for (int i = 0; i < num_of_tabs_to_test; ++i) {
task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(10)); task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(10));
tab_strip->AppendWebContents(CreateWebContents(), /*foreground=*/true); tab_strip->AppendWebContents(CreateWebContents(), /*foreground=*/true);
} }
......
...@@ -38,24 +38,11 @@ ChromeRenderViewHostTestHarness::GetTestingFactories() const { ...@@ -38,24 +38,11 @@ ChromeRenderViewHostTestHarness::GetTestingFactories() const {
std::unique_ptr<TestingProfile> std::unique_ptr<TestingProfile>
ChromeRenderViewHostTestHarness::CreateTestingProfile() { ChromeRenderViewHostTestHarness::CreateTestingProfile() {
// Maintain the profile directory ourselves so that it isn't deleted along
// with TestingProfile. RenderViewHostTestHarness::TearDown() will destroy
// the profile and also destroy the thread bundle to ensure that any tasks
// posted throughout the test run to completion. By postponing the deletion
// of the profile directory until ~ChromeRenderViewHostTestHarness() we
// guarantee that no tasks will try to access the profile directory when it's
// (being) deleted.
auto temp_dir = std::make_unique<base::ScopedTempDir>();
CHECK(temp_dir->CreateUniqueTempDir());
TestingProfile::Builder builder; TestingProfile::Builder builder;
builder.SetPath(temp_dir->GetPath());
for (auto& pair : GetTestingFactories()) for (auto& pair : GetTestingFactories())
builder.AddTestingFactory(pair.first, pair.second); builder.AddTestingFactory(pair.first, pair.second);
temp_dirs_.push_back(std::move(temp_dir));
return builder.Build(); return builder.Build();
} }
......
...@@ -7,10 +7,8 @@ ...@@ -7,10 +7,8 @@
#include <memory> #include <memory>
#include <utility> #include <utility>
#include <vector>
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/files/scoped_temp_dir.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "content/public/test/test_renderer_host.h" #include "content/public/test/test_renderer_host.h"
...@@ -43,9 +41,6 @@ class ChromeRenderViewHostTestHarness ...@@ -43,9 +41,6 @@ class ChromeRenderViewHostTestHarness
// content::RenderViewHostTestHarness. // content::RenderViewHostTestHarness.
std::unique_ptr<content::BrowserContext> CreateBrowserContext() final; std::unique_ptr<content::BrowserContext> CreateBrowserContext() final;
private:
std::vector<std::unique_ptr<base::ScopedTempDir>> temp_dirs_;
}; };
#endif // CHROME_TEST_BASE_CHROME_RENDER_VIEW_HOST_TEST_HARNESS_H_ #endif // CHROME_TEST_BASE_CHROME_RENDER_VIEW_HOST_TEST_HARNESS_H_
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