Always persist data to disk after 60s from the receiving update request.

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/json/json_reader.h"
@@ skipping 66 matching lines @@
 };
 
 class TestingHttpServerPropertiesManager : public HttpServerPropertiesManager {
  public:
   TestingHttpServerPropertiesManager(
       HttpServerPropertiesManager::PrefDelegate* pref_delegate,
       scoped_refptr<base::SingleThreadTaskRunner> pref_task_runner,
       scoped_refptr<base::SingleThreadTaskRunner> net_task_runner)
       : HttpServerPropertiesManager(pref_delegate,
                                     pref_task_runner,
-                                    net_task_runner) {
+                                    net_task_runner),
+        pref_update_delay_(base::TimeDelta()) {
     InitializeOnNetworkThread();
   }
 
   ~TestingHttpServerPropertiesManager() override {}
 
   // Make these methods public for testing.
   using HttpServerPropertiesManager::ScheduleUpdateCacheOnPrefThread;
 
   // Post tasks without a delay during tests.
   void StartPrefsUpdateTimerOnNetworkThread(base::TimeDelta delay) override {
     HttpServerPropertiesManager::StartPrefsUpdateTimerOnNetworkThread(
-        base::TimeDelta());
+        pref_update_delay_);
   }
 
   void UpdateCacheFromPrefsOnUIConcrete() {
     HttpServerPropertiesManager::UpdateCacheFromPrefsOnPrefThread();
   }
 
   // Post tasks without a delay during tests.
   void StartCacheUpdateTimerOnPrefThread(base::TimeDelta delay) override {
     HttpServerPropertiesManager::StartCacheUpdateTimerOnPrefThread(
         base::TimeDelta());
   }
 
   void UpdatePrefsFromCacheOnNetworkThreadConcrete(
       const base::Closure& callback) {
     HttpServerPropertiesManager::UpdatePrefsFromCacheOnNetworkThread(callback);
   }
 
   void ScheduleUpdatePrefsOnNetworkThreadConcrete(Location location) {
     HttpServerPropertiesManager::ScheduleUpdatePrefsOnNetworkThread(location);
   }
 
   void ScheduleUpdatePrefsOnNetworkThread() {
     // Picked a random Location as caller.
     HttpServerPropertiesManager::ScheduleUpdatePrefsOnNetworkThread(
         DETECTED_CORRUPTED_PREFS);
   }
 
+  void set_pref_update_delay(base::TimeDelta delay) {
+    pref_update_delay_ = delay;
+  }
   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:
+  // Time delays used in test for posting tasks. Default to zero.
+  base::TimeDelta pref_update_delay_;
   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> {
@@ skipping 496 matching lines @@
   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());
 
   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,
+       SinglePrefUpdateForTwoSpdyServerCacheChangese) {
+  http_server_props_manager_->set_pref_update_delay(
+      base::TimeDelta::FromMilliseconds(60));
+  ExpectPrefsUpdateRepeatedly();

[xunjieli, 2016/12/16 15:48:08]

I know in the test you asserted pref_task_runner_ does not have task posted. To
make the test expectation tighter, can we assert that this called exactly 2
times (rather than using WillRepeately())?

[Zhongyi Shi, 2016/12/17 00:59:43]

Thanks a lot. Since the change your proposed will apply to all the unittests. I
will do that in a separate cl. 

+  ExpectScheduleUpdatePrefsOnNetworkThreadRepeatedly();

[xunjieli, 2016/12/16 15:48:08]

Same here. Can we assert that this called exactly 3 times?

[Zhongyi Shi, 2016/12/17 00:59:43]

This is might be hard. Some of the tests are calling the same method with
different args different times. If we want to enforce checking on how many times
this method is called, we will also need to specify the argument, which is
defined in the .h file. 

+
+  // 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 20ms.
+  net_test_task_runner_->FastForwardBy(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());
+
+  // Move forward another 40ms. The pref update should be executed.
+  net_test_task_runner_->FastForwardBy(base::TimeDelta::FromMilliseconds(40));
+  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
+  // ScheduleUpdatePrefsOnNetworkThread. A new update task should be posted to
+  // network thread now since the previous one is completed.
+  url::SchemeHostPort spdy_server3("https", "maps.google.com", 443);
+  http_server_props_manager_->SetSupportsSpdy(spdy_server3, true);
+  EXPECT_FALSE(pref_test_task_runner_->HasPendingTask());
+  EXPECT_EQ(1u, net_test_task_runner_->GetPendingTaskCount());
+
+  // Run the task.
+  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());
+
+  Mock::VerifyAndClearExpectations(http_server_props_manager_.get());
+}
+
 TEST_P(HttpServerPropertiesManagerTest, GetAlternativeServices) {
   ExpectPrefsUpdate();
   ExpectScheduleUpdatePrefsOnNetworkThread();
 
   url::SchemeHostPort spdy_server_mail("http", "mail.google.com", 80);
   EXPECT_FALSE(HasAlternativeService(spdy_server_mail));
   const AlternativeService alternative_service(kProtoHTTP2, "mail.google.com",
                                                443);
   http_server_props_manager_->SetAlternativeService(
       spdy_server_mail, alternative_service, one_day_from_now_);
@@ skipping 738 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