Introduce ThreadTaskRunnerHandle::OverrideForTesting and TestMockTimeTaskRunner::ScopedContext.

net/http/http_server_properties_manager_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_server_properties_manager.h"
 
 #include <memory>
 #include <utility>
 
+#include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
-#include "base/message_loop/message_loop.h"
-#include "base/run_loop.h"
+#include "base/memory/ref_counted.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
+#include "base/test/scoped_mock_time_message_loop_task_runner.h"
 #include "base/test/test_mock_time_task_runner.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "net/base/ip_address.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 namespace net {
@@ skipping 45 matching lines @@
   base::DictionaryValue prefs_;
   base::Closure prefs_changed_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(MockPrefDelegate);
 };
 
 class TestingHttpServerPropertiesManager : public HttpServerPropertiesManager {
  public:
   TestingHttpServerPropertiesManager(
       HttpServerPropertiesManager::PrefDelegate* pref_delegate,
-      scoped_refptr<base::SingleThreadTaskRunner> pref_task_runner,
-      scoped_refptr<base::SingleThreadTaskRunner> net_task_runner)
+      scoped_refptr<TestMockTimeTaskRunner> pref_task_runner,
+      scoped_refptr<TestMockTimeTaskRunner> net_task_runner)
       : HttpServerPropertiesManager(pref_delegate,
                                     pref_task_runner,
-                                    net_task_runner) {
-    InitializeOnNetworkThread();
+                                    net_task_runner),
+        pref_task_runner_(std::move(pref_task_runner)),
+        net_task_runner_(std::move(net_task_runner)) {
+    // This call must run in the context of |net_task_runner_|.

[xunjieli, 2017/02/16 02:49:18]

Just so I understand the api, if we remove the |scoped_context|, will
HttpServerPropertiesManager::InitializeOnNetworkThread() complain about the
thread doesn't match?

[gab, 2017/02/16 16:50:15]

It doesn't yet but it should (see TODO(gab) I added in this CL on
TestMockTimeTaskRunner::RunsTasksOnCurrentThread() -- will then require more
tweaks to this test and others, e.g. in this CL I only fixed calls that "do"
things and for which it "matters to get the logic right" but many getters
DCHECK(RunsTasksOnCurrentThread()) and aren't scoped by this CL but will
eventually have to be).

This CL now only tasks care of making sure base::ThreadTaskRunnerHandle::Get()
in the scope of the ScopedContext now properly returns |net_task_runner_|
instead of the main message loop (i.e. that's why you needed RunLoop()
previously in a few places).

This will matter even more in https://codereview.chromium.org/2491613004/ as the
base::Timer will run its delayed task on SequencedTaskRunnerHandle::Get() (which
defers to ThreadTaskRunnerHandle::Get() when running on a
SingleThreadTaskRunner) and having that be the wrong underlying task runner was
breaking this test (and a few others).

Long term I want TestMockTimeTaskRunner to provide isolated
ThreadTaskRunnerHandle and RunsTasksOnCurrentThread for its tasks (even though
it shares the main thread) as it's really mimicking multiple threads on the main
thread for test simplicity but they shouldn't behave as though running on "main
thread".

[xunjieli, 2017/02/16 17:08:29]

Acknowledged. Thanks for the context.

+    TestMockTimeTaskRunner::ScopedContext scoped_context(net_task_runner_);
+    HttpServerPropertiesManager::InitializeOnNetworkThread();
   }
 
   ~TestingHttpServerPropertiesManager() override {}
 
   // Make these methods public for testing.
   using HttpServerPropertiesManager::ScheduleUpdateCacheOnPrefThread;
 
   void UpdateCacheFromPrefsOnUIConcrete() {
+    TestMockTimeTaskRunner::ScopedContext scoped_context(pref_task_runner_);
     HttpServerPropertiesManager::UpdateCacheFromPrefsOnPrefThread();
   }
 
   void UpdatePrefsFromCacheOnNetworkThreadConcrete(
       const base::Closure& callback) {
+    TestMockTimeTaskRunner::ScopedContext scoped_context(net_task_runner_);
     HttpServerPropertiesManager::UpdatePrefsFromCacheOnNetworkThread(callback);
   }
 
   void ScheduleUpdatePrefsOnNetworkThreadConcrete(Location location) {
+    TestMockTimeTaskRunner::ScopedContext scoped_context(net_task_runner_);
     HttpServerPropertiesManager::ScheduleUpdatePrefsOnNetworkThread(location);
   }
 
-  void ScheduleUpdatePrefsOnNetworkThread() {
+  void ScheduleUpdatePrefsOnNetworkThreadDefault() {
+    TestMockTimeTaskRunner::ScopedContext scoped_context(net_task_runner_);
     // Picked a random Location as caller.
     HttpServerPropertiesManager::ScheduleUpdatePrefsOnNetworkThread(
         DETECTED_CORRUPTED_PREFS);
   }
 
   MOCK_METHOD0(UpdateCacheFromPrefsOnPrefThread, void());
   MOCK_METHOD1(UpdatePrefsFromCacheOnNetworkThread, void(const base::Closure&));
   MOCK_METHOD1(ScheduleUpdatePrefsOnNetworkThread, void(Location location));
   MOCK_METHOD6(UpdateCacheFromPrefsOnNetworkThread,
                void(std::vector<std::string>* spdy_servers,
                     AlternativeServiceMap* alternative_service_map,
                     IPAddress* last_quic_address,
                     ServerNetworkStatsMap* server_network_stats_map,
                     QuicServerInfoMap* quic_server_info_map,
                     bool detected_corrupted_prefs));
   MOCK_METHOD6(UpdatePrefsOnPrefThread,
                void(base::ListValue* spdy_server_list,
                     AlternativeServiceMap* alternative_service_map,
                     IPAddress* last_quic_address,
                     ServerNetworkStatsMap* server_network_stats_map,
                     QuicServerInfoMap* quic_server_info_map,
                     const base::Closure& completion));
 
  private:
+  // References to the underlying task runners, used to simulate running in
+  // their contexts where required.
+  scoped_refptr<TestMockTimeTaskRunner> pref_task_runner_;
+  scoped_refptr<TestMockTimeTaskRunner> net_task_runner_;
+
   DISALLOW_COPY_AND_ASSIGN(TestingHttpServerPropertiesManager);
 };
 
 }  // namespace
 
 // TODO(rtenneti): After we stop supporting version 3 and everyone has migrated
 // to version 4, delete the following code.
 static const int kHttpServerPropertiesVersions[] = {3, 4, 5};
 
 class HttpServerPropertiesManagerTest : public testing::TestWithParam<int> {
  protected:
-  HttpServerPropertiesManagerTest()
-      : pref_test_task_runner_(new TestMockTimeTaskRunner()),
-        net_test_task_runner_(new TestMockTimeTaskRunner()) {}
+  HttpServerPropertiesManagerTest() = default;
 
   void SetUp() override {
     one_day_from_now_ = base::Time::Now() + base::TimeDelta::FromDays(1);
     pref_delegate_ = new MockPrefDelegate;
     http_server_props_manager_.reset(
         new StrictMock<TestingHttpServerPropertiesManager>(
-            pref_delegate_, pref_test_task_runner_, net_test_task_runner_));
+            pref_delegate_, pref_test_task_runner_.task_runner(),
+            net_test_task_runner_));
 
     EXPECT_FALSE(http_server_props_manager_->IsInitialized());
     ExpectCacheUpdate();
     EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
-    pref_test_task_runner_->FastForwardUntilNoTasksRemain();
-    // InitializeOnNetworkThread() is called from the test thread, so
-    // PostTaskAndReply() will post back to the test thread.
-    // TODO(xunjieli): Fix this in a follow-up so we can use
-    // |net_test_task_runner_|->RunUntilIdle().
-    base::RunLoop().RunUntilIdle();
+    pref_test_task_runner_->RunUntilIdle();
+    net_test_task_runner_->RunUntilIdle();
     EXPECT_TRUE(http_server_props_manager_->IsInitialized());
     EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
     EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
   }
 
-  void SetUpWithNonTaskRunner() {
-    pref_delegate_ = new MockPrefDelegate;
-    http_server_props_manager_.reset(
-        new StrictMock<TestingHttpServerPropertiesManager>(
-            pref_delegate_, base::ThreadTaskRunnerHandle::Get(),
-            net_test_task_runner_));
-
-    EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
-    ExpectCacheUpdate();
-    base::RunLoop().RunUntilIdle();
-    EXPECT_TRUE(http_server_props_manager_->IsInitialized());
-  }
-
   void TearDown() override {
     if (http_server_props_manager_.get())
       http_server_props_manager_->ShutdownOnPrefThread();
-    base::RunLoop().RunUntilIdle();
-    pref_test_task_runner_->FastForwardUntilNoTasksRemain();
-    net_test_task_runner_->FastForwardUntilNoTasksRemain();
+    // Run pending non-delayed tasks but don't FastForwardUntilNoTasksRemain()
+    // as some delayed tasks may forever repost (e.g. because impl doesn't use a
+    // mock clock and doesn't see timings as having expired, ref.
+    // HttpServerPropertiesImpl::ScheduleBrokenAlternateProtocolMappingsExpiration()).
+    pref_test_task_runner_->RunUntilIdle();
+    net_test_task_runner_->RunUntilIdle();
     http_server_props_manager_.reset();
   }
 
   void ExpectCacheUpdate() {
     EXPECT_CALL(*http_server_props_manager_, UpdateCacheFromPrefsOnPrefThread())
         .WillOnce(Invoke(http_server_props_manager_.get(),
                          &TestingHttpServerPropertiesManager::
                              UpdateCacheFromPrefsOnUIConcrete));
   }
 
@@ skipping 27 matching lines @@
   bool HasAlternativeService(const url::SchemeHostPort& server) {
     const AlternativeServiceVector alternative_service_vector =
         http_server_props_manager_->GetAlternativeServices(server);
     return !alternative_service_vector.empty();
   }
 
   MockPrefDelegate* pref_delegate_;  // Owned by HttpServerPropertiesManager.
   std::unique_ptr<TestingHttpServerPropertiesManager>
       http_server_props_manager_;
   base::Time one_day_from_now_;
-  scoped_refptr<TestMockTimeTaskRunner> pref_test_task_runner_;
-  scoped_refptr<TestMockTimeTaskRunner> net_test_task_runner_;
+
+  // Overrides the main thread's message loop with a mock tick clock. Making the
+  // main thread the |pref_test_task_runner_| matches expectations better than
+  // having an independent TestMockTimeTaskRunner and makes tests easier to
+  // write.
+  base::ScopedMockTimeMessageLoopTaskRunner pref_test_task_runner_;
+
+  // Mock the net task runner as well.
+  scoped_refptr<TestMockTimeTaskRunner> net_test_task_runner_ =
+      new TestMockTimeTaskRunner;

[xunjieli, 2017/02/16 02:49:18]

Can we initialize this one in the constructor?

[gab, 2017/02/16 16:50:15]

I prefer C++11 member initialization when at all possible, it's more readable
IMO than having to go lookup at the constructor.

It's logically equivalent but this way you have all the relevant code in one
place.

[xunjieli, 2017/02/16 17:08:29]

Acknowledged.

 
  private:
   DISALLOW_COPY_AND_ASSIGN(HttpServerPropertiesManagerTest);
 };
 
 INSTANTIATE_TEST_CASE_P(/* no prefix */,
                         HttpServerPropertiesManagerTest,
                         ::testing::ValuesIn(kHttpServerPropertiesVersions));
 
 TEST_P(HttpServerPropertiesManagerTest,
@@ skipping 399 matching lines @@
   EXPECT_TRUE(http_server_props_manager_->SupportsRequestPriority(spdy_server));
   Mock::VerifyAndClearExpectations(http_server_props_manager_.get());
 }
 
 // Regression test for crbug.com/670519. Test that there is only one pref update
 // scheduled if multiple updates happen in a given time period. Subsequent pref
 // update could also be scheduled once the previous scheduled update is
 // completed.
 TEST_P(HttpServerPropertiesManagerTest,
        SinglePrefUpdateForTwoSpdyServerCacheChanges) {
-  // Keep this in sync with http_server_properties_manager.cc
-  const int64_t kUpdatePrefsDelayMs = 60000;
   ExpectPrefsUpdate(2);
   ExpectScheduleUpdatePrefsOnNetworkThreadRepeatedly(3);
 
   // Post an update task to the network thread. SetSupportsSpdy calls
   // ScheduleUpdatePrefsOnNetworkThread with a delay of 60ms.
   url::SchemeHostPort spdy_server("https", "mail.google.com", 443);
   EXPECT_FALSE(
       http_server_props_manager_->SupportsRequestPriority(spdy_server));
   http_server_props_manager_->SetSupportsSpdy(spdy_server, true);
   // The pref update task should be scheduled to network thread.
   EXPECT_EQ(1u, net_test_task_runner_->GetPendingTaskCount());
 
-  // Move forward the task runner with kUpdatePrefsDelayMs/2.
+  // Move forward the task runner short by 20ms.
   net_test_task_runner_->FastForwardBy(
-      base::TimeDelta::FromMilliseconds(kUpdatePrefsDelayMs / 2));
+      HttpServerPropertiesManager::GetUpdatePrefsDelayForTesting() -
+      base::TimeDelta::FromMilliseconds(20));
 
   // Set another spdy server to trigger another call to
   // ScheduleUpdatePrefsOnNetworkThread. There should be no new update posted to
   // the network thread.
   url::SchemeHostPort spdy_server2("https", "drive.google.com", 443);
   http_server_props_manager_->SetSupportsSpdy(spdy_server2, true);
   EXPECT_EQ(1u, net_test_task_runner_->GetPendingTaskCount());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
 
-  // Forward another kUpdatePrefsDelayMs/2. The pref update should be executed.
-  net_test_task_runner_->FastForwardBy(
-      base::TimeDelta::FromMilliseconds(kUpdatePrefsDelayMs / 2));
+  // Move forward the extra 20ms. The pref update should be executed.
+  net_test_task_runner_->FastForwardBy(base::TimeDelta::FromMilliseconds(20));
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   EXPECT_TRUE(pref_test_task_runner_->HasPendingTask());
   pref_test_task_runner_->FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
 
   EXPECT_TRUE(http_server_props_manager_->SupportsRequestPriority(spdy_server));
   EXPECT_TRUE(
       http_server_props_manager_->SupportsRequestPriority(spdy_server2));
   // Set the third spdy server to trigger one more call to
@@ skipping 98 matching lines @@
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   Mock::VerifyAndClearExpectations(http_server_props_manager_.get());
 
   EXPECT_FALSE(HasAlternativeService(spdy_server_mail));
 }
 
 TEST_P(HttpServerPropertiesManagerTest, ConfirmAlternativeService) {
   ExpectPrefsUpdate(1);
 
-  url::SchemeHostPort spdy_server_mail("http", "mail.google.com", 80);
-  EXPECT_FALSE(HasAlternativeService(spdy_server_mail));
-  AlternativeService alternative_service(kProtoHTTP2, "mail.google.com", 443);
+  url::SchemeHostPort spdy_server_mail;
+  AlternativeService alternative_service;
 
-  ExpectScheduleUpdatePrefsOnNetworkThread();
-  http_server_props_manager_->SetAlternativeService(
-      spdy_server_mail, alternative_service, one_day_from_now_);
+  {
+    TestMockTimeTaskRunner::ScopedContext scoped_context(net_test_task_runner_);
 
-  EXPECT_FALSE(http_server_props_manager_->IsAlternativeServiceBroken(
-      alternative_service));
-  EXPECT_FALSE(http_server_props_manager_->WasAlternativeServiceRecentlyBroken(
-      alternative_service));
+    spdy_server_mail = url::SchemeHostPort("http", "mail.google.com", 80);
+    EXPECT_FALSE(HasAlternativeService(spdy_server_mail));
+    alternative_service =
+        AlternativeService(kProtoHTTP2, "mail.google.com", 443);
 
-  ExpectScheduleUpdatePrefsOnNetworkThread();
-  http_server_props_manager_->MarkAlternativeServiceBroken(alternative_service);
-  EXPECT_TRUE(http_server_props_manager_->IsAlternativeServiceBroken(
-      alternative_service));
-  EXPECT_TRUE(http_server_props_manager_->WasAlternativeServiceRecentlyBroken(
-      alternative_service));
+    ExpectScheduleUpdatePrefsOnNetworkThread();
+    http_server_props_manager_->SetAlternativeService(
+        spdy_server_mail, alternative_service, one_day_from_now_);
 
-  ExpectScheduleUpdatePrefsOnNetworkThread();
-  http_server_props_manager_->ConfirmAlternativeService(alternative_service);
-  EXPECT_FALSE(http_server_props_manager_->IsAlternativeServiceBroken(
-      alternative_service));
-  EXPECT_FALSE(http_server_props_manager_->WasAlternativeServiceRecentlyBroken(
-      alternative_service));
-  // ExpectScheduleUpdatePrefsOnNetworkThread() should be called only once.
-  http_server_props_manager_->ConfirmAlternativeService(alternative_service);
-  EXPECT_FALSE(http_server_props_manager_->IsAlternativeServiceBroken(
-      alternative_service));
-  EXPECT_FALSE(http_server_props_manager_->WasAlternativeServiceRecentlyBroken(
-      alternative_service));
+    EXPECT_FALSE(http_server_props_manager_->IsAlternativeServiceBroken(
+        alternative_service));
+    EXPECT_FALSE(
+        http_server_props_manager_->WasAlternativeServiceRecentlyBroken(
+            alternative_service));
+
+    ExpectScheduleUpdatePrefsOnNetworkThread();
+    http_server_props_manager_->MarkAlternativeServiceBroken(
+        alternative_service);
+    EXPECT_TRUE(http_server_props_manager_->IsAlternativeServiceBroken(
+        alternative_service));
+    EXPECT_TRUE(http_server_props_manager_->WasAlternativeServiceRecentlyBroken(
+        alternative_service));
+
+    ExpectScheduleUpdatePrefsOnNetworkThread();
+    http_server_props_manager_->ConfirmAlternativeService(alternative_service);
+    EXPECT_FALSE(http_server_props_manager_->IsAlternativeServiceBroken(
+        alternative_service));
+    EXPECT_FALSE(
+        http_server_props_manager_->WasAlternativeServiceRecentlyBroken(
+            alternative_service));
+    // ExpectScheduleUpdatePrefsOnNetworkThread() should be called only once.
+    http_server_props_manager_->ConfirmAlternativeService(alternative_service);
+    EXPECT_FALSE(http_server_props_manager_->IsAlternativeServiceBroken(
+        alternative_service));
+    EXPECT_FALSE(
+        http_server_props_manager_->WasAlternativeServiceRecentlyBroken(
+            alternative_service));
+  }
 
   // Run the task.
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
   EXPECT_TRUE(net_test_task_runner_->HasPendingTask());
   net_test_task_runner_->FastForwardUntilNoTasksRemain();
   EXPECT_TRUE(pref_test_task_runner_->HasPendingTask());
   pref_test_task_runner_->FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
 
   Mock::VerifyAndClearExpectations(http_server_props_manager_.get());
 
-  EXPECT_FALSE(http_server_props_manager_->IsAlternativeServiceBroken(
-      alternative_service));
-  EXPECT_FALSE(http_server_props_manager_->WasAlternativeServiceRecentlyBroken(
-      alternative_service));
+  {
+    TestMockTimeTaskRunner::ScopedContext scoped_context(net_test_task_runner_);
+    EXPECT_FALSE(http_server_props_manager_->IsAlternativeServiceBroken(
+        alternative_service));
+    EXPECT_FALSE(
+        http_server_props_manager_->WasAlternativeServiceRecentlyBroken(
+            alternative_service));
+  }
 }
 
 TEST_P(HttpServerPropertiesManagerTest, SupportsQuic) {
   ExpectPrefsUpdate(1);
   ExpectScheduleUpdatePrefsOnNetworkThread();
 
   IPAddress address;
   EXPECT_FALSE(http_server_props_manager_->GetSupportsQuic(&address));
 
   IPAddress actual_address(127, 0, 0, 1);
@@ skipping 69 matching lines @@
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
 
   Mock::VerifyAndClearExpectations(http_server_props_manager_.get());
 
   EXPECT_EQ(quic_server_info1, *http_server_props_manager_->GetQuicServerInfo(
                                    mail_quic_server_id));
 }
 
 TEST_P(HttpServerPropertiesManagerTest, Clear) {
-  // This task expect to run the QuitWhenIdleClosure in the current thread,
-  // thus can not mock the pref task runner.
-  SetUpWithNonTaskRunner();
-
   ExpectPrefsUpdate(1);
   ExpectScheduleUpdatePrefsOnNetworkThreadRepeatedly(5);
 
-  url::SchemeHostPort spdy_server("https", "mail.google.com", 443);
-  http_server_props_manager_->SetSupportsSpdy(spdy_server, true);
-  AlternativeService alternative_service(kProtoHTTP2, "mail.google.com", 1234);
-  http_server_props_manager_->SetAlternativeService(
-      spdy_server, alternative_service, one_day_from_now_);
-  IPAddress actual_address(127, 0, 0, 1);
-  http_server_props_manager_->SetSupportsQuic(true, actual_address);
-  ServerNetworkStats stats;
-  stats.srtt = base::TimeDelta::FromMicroseconds(10);
-  http_server_props_manager_->SetServerNetworkStats(spdy_server, stats);
+  const url::SchemeHostPort spdy_server("https", "mail.google.com", 443);
+  const IPAddress actual_address(127, 0, 0, 1);
+  const QuicServerId mail_quic_server_id("mail.google.com", 80);
+  const std::string quic_server_info1("quic_server_info1");
 
-  QuicServerId mail_quic_server_id("mail.google.com", 80);
-  std::string quic_server_info1("quic_server_info1");
-  http_server_props_manager_->SetQuicServerInfo(mail_quic_server_id,
-                                                quic_server_info1);
+  {
+    TestMockTimeTaskRunner::ScopedContext scoped_context(net_test_task_runner_);
+
+    http_server_props_manager_->SetSupportsSpdy(spdy_server, true);
+    AlternativeService alternative_service(kProtoHTTP2, "mail.google.com",
+                                           1234);
+    http_server_props_manager_->SetAlternativeService(
+        spdy_server, alternative_service, one_day_from_now_);
+    http_server_props_manager_->SetSupportsQuic(true, actual_address);
+    ServerNetworkStats stats;
+    stats.srtt = base::TimeDelta::FromMicroseconds(10);
+    http_server_props_manager_->SetServerNetworkStats(spdy_server, stats);
+
+    http_server_props_manager_->SetQuicServerInfo(mail_quic_server_id,
+                                                  quic_server_info1);
+  }
 
   // Run the task.
-  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
   EXPECT_TRUE(net_test_task_runner_->HasPendingTask());
   net_test_task_runner_->FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
 
   EXPECT_TRUE(http_server_props_manager_->SupportsRequestPriority(spdy_server));
   EXPECT_TRUE(HasAlternativeService(spdy_server));
   IPAddress address;
   EXPECT_TRUE(http_server_props_manager_->GetSupportsQuic(&address));
   EXPECT_EQ(actual_address, address);
   const ServerNetworkStats* stats1 =
       http_server_props_manager_->GetServerNetworkStats(spdy_server);
   EXPECT_EQ(10, stats1->srtt.ToInternalValue());
   EXPECT_EQ(quic_server_info1, *http_server_props_manager_->GetQuicServerInfo(
                                    mail_quic_server_id));
 
   Mock::VerifyAndClearExpectations(http_server_props_manager_.get());
 
   ExpectPrefsUpdate(1);
 
-  // Clear http server data, time out if we do not get a completion callback.
-  http_server_props_manager_->Clear(base::MessageLoop::QuitWhenIdleClosure());
-
-  base::RunLoop().Run();
+  // Clear http server data and run the ensuing non-delayed prefs update.
+  {
+    TestMockTimeTaskRunner::ScopedContext scoped_context(net_test_task_runner_);
+    http_server_props_manager_->Clear();
+  }
+  EXPECT_TRUE(pref_test_task_runner_->HasPendingTask());
+  pref_test_task_runner_->RunUntilIdle();
+  EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
 
   EXPECT_FALSE(
       http_server_props_manager_->SupportsRequestPriority(spdy_server));
   EXPECT_FALSE(HasAlternativeService(spdy_server));
   EXPECT_FALSE(http_server_props_manager_->GetSupportsQuic(&address));
   const ServerNetworkStats* stats2 =
       http_server_props_manager_->GetServerNetworkStats(spdy_server);
   EXPECT_EQ(nullptr, stats2);
   EXPECT_EQ(nullptr,
@@ skipping 179 matching lines @@
   const base::Value* http_server_properties =
       &pref_delegate_->GetServerProperties();
   std::string preferences_json;
   EXPECT_TRUE(
       base::JSONWriter::Write(*http_server_properties, &preferences_json));
   EXPECT_EQ(expected_json, preferences_json);
 }
 
 TEST_P(HttpServerPropertiesManagerTest,
        SingleCacheUpdateForMultipleUpdatesScheduled) {
-  // Keep this in sync with http_server_properties_manager.cc
-  const int64_t kUpdateCacheDelayMs = 1000;
   // Update cache.
   ExpectCacheUpdate();
 
   EXPECT_EQ(0u, pref_test_task_runner_->GetPendingTaskCount());
   // Update cache.
   http_server_props_manager_->ScheduleUpdateCacheOnPrefThread();
   EXPECT_EQ(1u, pref_test_task_runner_->GetPendingTaskCount());
 
-  // Move forward the task runner kUpdateCacheDelayMs/2.
+  // Move forward the task runner short by 20ms.
   pref_test_task_runner_->FastForwardBy(
-      base::TimeDelta::FromMilliseconds(kUpdateCacheDelayMs / 2));
+      HttpServerPropertiesManager::GetUpdateCacheDelayForTesting() -
+      base::TimeDelta::FromMilliseconds(20));
   // Schedule a new cache update within the time window should be a no-op.
   http_server_props_manager_->ScheduleUpdateCacheOnPrefThread();
   EXPECT_EQ(1u, pref_test_task_runner_->GetPendingTaskCount());
 
-  // Move forward the task runner kUpdateCaceDelayMs/2, now the cache update
-  // should be exectured.
-  pref_test_task_runner_->FastForwardBy(
-      base::TimeDelta::FromMilliseconds(kUpdateCacheDelayMs / 2));
+  // Move forward the task runner the extra 20ms, now the cache update should be
+  // exectuted.

[xunjieli, 2017/02/16 02:49:18]

nit: executed. (i know it was misspelled in the original code) 

[gab, 2017/02/16 16:50:15]

Done.

+  pref_test_task_runner_->FastForwardBy(base::TimeDelta::FromMilliseconds(20));
 
   // Since this test has no pref corruption, there shouldn't be any pref update.
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
 
   // Schedule one more cache update. The task should be successfully scheduled
   // on pref task runner.
   ExpectCacheUpdate();
   http_server_props_manager_->ScheduleUpdateCacheOnPrefThread();
   EXPECT_EQ(1u, pref_test_task_runner_->GetPendingTaskCount());
@@ skipping 68 matching lines @@
 
   AlternativeServiceMap::iterator it = alternative_service_map.Get(server);
   EXPECT_EQ(alternative_service_map.end(), it);
 }
 
 // Do not persist expired or broken alternative service entries to disk.
 TEST_P(HttpServerPropertiesManagerTest,
        DoNotPersistExpiredOrBrokenAlternativeService) {
   ExpectScheduleUpdatePrefsOnNetworkThreadRepeatedly(2);
 
-  AlternativeServiceInfoVector alternative_service_info_vector;
+  {
+    TestMockTimeTaskRunner::ScopedContext scoped_context(net_test_task_runner_);
 
-  const AlternativeService broken_alternative_service(
-      kProtoHTTP2, "broken.example.com", 443);
-  const base::Time time_one_day_later =
-      base::Time::Now() + base::TimeDelta::FromDays(1);
-  alternative_service_info_vector.push_back(
-      AlternativeServiceInfo(broken_alternative_service, time_one_day_later));
-  // #1: MarkAlternativeServiceBroken().
-  http_server_props_manager_->MarkAlternativeServiceBroken(
-      broken_alternative_service);
+    AlternativeServiceInfoVector alternative_service_info_vector;
 
-  const AlternativeService expired_alternative_service(
-      kProtoHTTP2, "expired.example.com", 443);
-  const base::Time time_one_day_ago =
-      base::Time::Now() - base::TimeDelta::FromDays(1);
-  alternative_service_info_vector.push_back(
-      AlternativeServiceInfo(expired_alternative_service, time_one_day_ago));
+    const AlternativeService broken_alternative_service(
+        kProtoHTTP2, "broken.example.com", 443);
+    const base::Time time_one_day_later =
+        base::Time::Now() + base::TimeDelta::FromDays(1);
+    alternative_service_info_vector.push_back(
+        AlternativeServiceInfo(broken_alternative_service, time_one_day_later));
+    // #1: MarkAlternativeServiceBroken().
+    http_server_props_manager_->MarkAlternativeServiceBroken(
+        broken_alternative_service);
 
-  const AlternativeService valid_alternative_service(kProtoHTTP2,
-                                                     "valid.example.com", 443);
-  alternative_service_info_vector.push_back(
-      AlternativeServiceInfo(valid_alternative_service, time_one_day_later));
+    const AlternativeService expired_alternative_service(
+        kProtoHTTP2, "expired.example.com", 443);
+    const base::Time time_one_day_ago =
+        base::Time::Now() - base::TimeDelta::FromDays(1);
+    alternative_service_info_vector.push_back(
+        AlternativeServiceInfo(expired_alternative_service, time_one_day_ago));
 
-  const url::SchemeHostPort server("https", "www.example.com", 443);
-  // #2: SetAlternativeService().
-  ASSERT_TRUE(http_server_props_manager_->SetAlternativeServices(
-      server, alternative_service_info_vector));
+    const AlternativeService valid_alternative_service(
+        kProtoHTTP2, "valid.example.com", 443);
+    alternative_service_info_vector.push_back(
+        AlternativeServiceInfo(valid_alternative_service, time_one_day_later));
+
+    const url::SchemeHostPort server("https", "www.example.com", 443);
+    // #2: SetAlternativeService().
+    ASSERT_TRUE(http_server_props_manager_->SetAlternativeServices(
+        server, alternative_service_info_vector));
+  }
 
   // Update cache.
   ExpectPrefsUpdate(1);
 
   EXPECT_TRUE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
-  net_test_task_runner_->FastForwardUntilNoTasksRemain();
+
+  // |net_test_task_runner_| has a remaining pending task to expire
+  // |broken_alternative_service| |time_one_day_later| (and the impl uses
+  // TimeTicks::Now() without a mock clock so FastForwardUntilNoTasksRemain()
+  // would result in an infinite loop).
+  net_test_task_runner_->FastForwardBy(
+      HttpServerPropertiesManager::GetUpdatePrefsDelayForTesting());
+  EXPECT_EQ(1U, net_test_task_runner_->GetPendingTaskCount());
+
   EXPECT_TRUE(pref_test_task_runner_->HasPendingTask());
   pref_test_task_runner_->FastForwardUntilNoTasksRemain();
-  EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
+  EXPECT_EQ(1U, net_test_task_runner_->GetPendingTaskCount());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
 
   const base::DictionaryValue& pref_dict =
       pref_delegate_->GetServerProperties();
 
   const base::ListValue* servers_list = nullptr;
   ASSERT_TRUE(pref_dict.GetListWithoutPathExpansion("servers", &servers_list));
   base::ListValue::const_iterator it = servers_list->begin();
   const base::DictionaryValue* server_pref_dict;
   ASSERT_TRUE((*it)->GetAsDictionary(&server_pref_dict));
@@ skipping 104 matching lines @@
   http_server_props_manager_.reset();
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
 }
 
 //
 // Tests for shutdown when updating prefs.
 //
 TEST_P(HttpServerPropertiesManagerTest, ShutdownWithPendingUpdatePrefs0) {
   // Post an update task to the IO thread.
-  http_server_props_manager_->ScheduleUpdatePrefsOnNetworkThread();
+  http_server_props_manager_->ScheduleUpdatePrefsOnNetworkThreadDefault();
   // Shutdown comes before the task is executed.
   http_server_props_manager_->ShutdownOnPrefThread();
   http_server_props_manager_.reset();
   // Run the task after shutdown and deletion.
   EXPECT_TRUE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
   net_test_task_runner_->FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
 }
 
 TEST_P(HttpServerPropertiesManagerTest, ShutdownWithPendingUpdatePrefs1) {
   ExpectPrefsUpdate(1);
   // Post an update task.
-  http_server_props_manager_->ScheduleUpdatePrefsOnNetworkThread();
+  http_server_props_manager_->ScheduleUpdatePrefsOnNetworkThreadDefault();
   // Shutdown comes before the task is executed.
   http_server_props_manager_->ShutdownOnPrefThread();
   // Run the task after shutdown, but before deletion.
   EXPECT_TRUE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
   net_test_task_runner_->FastForwardUntilNoTasksRemain();
   EXPECT_TRUE(pref_test_task_runner_->HasPendingTask());
   pref_test_task_runner_->FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
@@ skipping 16 matching lines @@
   pref_test_task_runner_->FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
   Mock::VerifyAndClearExpectations(http_server_props_manager_.get());
   http_server_props_manager_.reset();
   EXPECT_FALSE(net_test_task_runner_->HasPendingTask());
   EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
 }
 
 }  // namespace net