Introduce ThreadTaskRunnerHandle::OverrideForTesting and TestMockTimeTaskRunner::ScopedContext.

chrome/browser/chromeos/policy/server_backed_state_keys_broker_unittest.cc

Index: chrome/browser/chromeos/policy/server_backed_state_keys_broker_unittest.cc
diff --git a/chrome/browser/chromeos/policy/server_backed_state_keys_broker_unittest.cc b/chrome/browser/chromeos/policy/server_backed_state_keys_broker_unittest.cc
index 30c4c8f6f312d848d9dc13e89e9cff80d0f96236..12dd2cb4b35d47661482dc16184aecf47b1feb64 100644
--- a/chrome/browser/chromeos/policy/server_backed_state_keys_broker_unittest.cc
+++ b/chrome/browser/chromeos/policy/server_backed_state_keys_broker_unittest.cc
@@ -9,7 +9,7 @@
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
-#include "base/test/test_simple_task_runner.h"
+#include "base/test/scoped_mock_time_message_loop_task_runner.h"
 #include "chromeos/dbus/fake_session_manager_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -18,8 +18,7 @@ namespace policy {
 class ServerBackedStateKeysBrokerTest : public testing::Test {
  public:
   ServerBackedStateKeysBrokerTest()
-      : task_runner_(new base::TestSimpleTaskRunner()),
-        broker_(&fake_session_manager_client_, task_runner_),
+      : broker_(&fake_session_manager_client_),
         updated_(false),
         callback_invoked_(false) {
     state_keys_.push_back("1");
@@ -47,7 +46,7 @@ class ServerBackedStateKeysBrokerTest : public testing::Test {
 
  protected:
   base::MessageLoop loop_;
-  scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
+  base::ScopedMockTimeMessageLoopTaskRunner mocked_main_runner_;
   chromeos::FakeSessionManagerClient fake_session_manager_client_;
   ServerBackedStateKeysBroker broker_;
   std::vector<std::string> state_keys_;
@@ -69,7 +68,7 @@ TEST_F(ServerBackedStateKeysBrokerTest, Load) {
       broker_.RegisterUpdateCallback(
           base::Bind(&ServerBackedStateKeysBrokerTest::StateKeysUpdated,
                      base::Unretained(this)));
-  base::RunLoop().RunUntilIdle();
+  mocked_main_runner_->RunUntilIdle();

[emaxx, 2017/03/01 14:43:53]

nit: #include "base/test/test_mock_time_task_runner.h"?

[gab, 2017/03/02 00:33:31]

We had a similar discussion @
https://bugs.chromium.org/p/chromium/issues/detail?id=618793#c6.

Conclusion was that headers that always require another one to be at all useful
should include it and not require its users to include multiple headers every
time.

i.e. jam@: "Stepping back, the purpose of IWYU is to avoid slowing down the
build by including unnecessary headers. In this case, the headers are always
necessary and will always be included by the source files anyways. So I don't
see a negative."

[emaxx, 2017/03/02 02:39:39]

The definition of the ScopedMockTimeMessageLoopTaskRunner class uses only
pointers to TestMockTimeTaskRunner and scoped_refptr's to it. So technically,
unless I'm missing something, it's not required for
scoped_mock_time_message_loop_task_runner.h to include
test_mock_time_task_runner.h.
Therefore IWYU, in my opinion, still applies here: the header A is not required
to include header B, and not every consumer of A is required to include header B
- only if they want to dereference pointers to corresponding types.

Having said that, I don't have any intention to stand in the way of these
changes. Just trying to understand the reasoning here and the scope of the IWYU
rule.

[gab, 2017/03/02 03:24:44]

In pure IWYU sense you're right, but as was concluded in discussion on
aforementioned bug. It makes sense for
scoped_mock_time_message_loop_task_runner.h to provide
test_mock_timer_task_runner.h given it's merely a proxy to it (i.e. it doesn't
make sense to use this mock if you're not going to at least use the underlying
TestMockTimeTaskRunner* once).

   EXPECT_TRUE(updated_);
   ExpectGood();
 }
@@ -82,7 +81,7 @@ TEST_F(ServerBackedStateKeysBrokerTest, Retry) {
       broker_.RegisterUpdateCallback(
           base::Bind(&ServerBackedStateKeysBrokerTest::StateKeysUpdated,
                      base::Unretained(this)));
-  base::RunLoop().RunUntilIdle();
+  mocked_main_runner_->RunUntilIdle();
   EXPECT_TRUE(updated_);
 
   EXPECT_FALSE(broker_.pending());
@@ -94,7 +93,7 @@ TEST_F(ServerBackedStateKeysBrokerTest, Retry) {
   updated_ = false;
   ServerBackedStateKeysBroker::Subscription subscription2 =
       broker_.RegisterUpdateCallback(base::Bind(&base::DoNothing));
-  base::RunLoop().RunUntilIdle();
+  mocked_main_runner_->RunUntilIdle();
   EXPECT_TRUE(updated_);
   ExpectGood();
 }
@@ -104,7 +103,7 @@ TEST_F(ServerBackedStateKeysBrokerTest, Refresh) {
       broker_.RegisterUpdateCallback(
           base::Bind(&ServerBackedStateKeysBrokerTest::StateKeysUpdated,
                      base::Unretained(this)));
-  base::RunLoop().RunUntilIdle();
+  mocked_main_runner_->RunUntilIdle();
   EXPECT_TRUE(updated_);
   ExpectGood();
 
@@ -113,15 +112,15 @@ TEST_F(ServerBackedStateKeysBrokerTest, Refresh) {
   state_keys_.push_back("4");
   fake_session_manager_client_.set_server_backed_state_keys(state_keys_);
   updated_ = false;
-  task_runner_->RunPendingTasks();
-  base::RunLoop().RunUntilIdle();
+  mocked_main_runner_->FastForwardBy(
+      ServerBackedStateKeysBroker::GetPollIntervalForTesting());
   EXPECT_TRUE(updated_);
   ExpectGood();
 
   // No update callback if the keys are unchanged.
   updated_ = false;
-  task_runner_->RunPendingTasks();
-  base::RunLoop().RunUntilIdle();
+  mocked_main_runner_->FastForwardBy(
+      ServerBackedStateKeysBroker::GetPollIntervalForTesting());
   EXPECT_FALSE(updated_);
   ExpectGood();
 }
@@ -130,7 +129,7 @@ TEST_F(ServerBackedStateKeysBrokerTest, Request) {
   broker_.RequestStateKeys(
       base::Bind(&ServerBackedStateKeysBrokerTest::HandleStateKeysCallback,
                  base::Unretained(this)));
-  base::RunLoop().RunUntilIdle();
+  mocked_main_runner_->RunUntilIdle();
   ExpectGood();
   EXPECT_TRUE(callback_invoked_);
   EXPECT_EQ(state_keys_, callback_state_keys_);
@@ -143,7 +142,7 @@ TEST_F(ServerBackedStateKeysBrokerTest, RequestFailure) {
   broker_.RequestStateKeys(
       base::Bind(&ServerBackedStateKeysBrokerTest::HandleStateKeysCallback,
                  base::Unretained(this)));
-  base::RunLoop().RunUntilIdle();
+  mocked_main_runner_->RunUntilIdle();
   EXPECT_TRUE(callback_invoked_);
   EXPECT_TRUE(callback_state_keys_.empty());
 }