Handle timeout for update requests.

components/safe_browsing_db/v4_update_protocol_manager_unittest.cc

Index: components/safe_browsing_db/v4_update_protocol_manager_unittest.cc
diff --git a/components/safe_browsing_db/v4_update_protocol_manager_unittest.cc b/components/safe_browsing_db/v4_update_protocol_manager_unittest.cc
index faf6dad0db2b0f84168f2c7510f2b802c415c2dc..5eeeab9d3c3e8e2c46a14fc615ffa02a85db5494 100644
--- a/components/safe_browsing_db/v4_update_protocol_manager_unittest.cc
+++ b/components/safe_browsing_db/v4_update_protocol_manager_unittest.cc
@@ -164,8 +164,6 @@ TEST_F(V4UpdateProtocolManagerTest, TestGetUpdatesErrorHandlingNetwork) {
 
   EXPECT_FALSE(pm->IsUpdateScheduled());
 
-  runner->RunPendingTasks();
-
   net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
   DCHECK(fetcher);
   // Failed request status should result in error.
@@ -198,8 +196,6 @@ TEST_F(V4UpdateProtocolManagerTest, TestGetUpdatesErrorHandlingResponseCode) {
 
   EXPECT_FALSE(pm->IsUpdateScheduled());
 
-  runner->RunPendingTasks();
-
   net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
   DCHECK(fetcher);
   fetcher->set_status(net::URLRequestStatus());
@@ -234,8 +230,6 @@ TEST_F(V4UpdateProtocolManagerTest, TestGetUpdatesNoError) {
 
   EXPECT_FALSE(pm->IsUpdateScheduled());
 

[Nathan Parker, 2016/11/08 23:39:29]

I'm not clear on why you need to remove these RunPendingTasks..

[vakh (use Gerrit instead), 2016/11/09 00:08:51]

IssueUpdateRequest queues a network request right away so there's no need to
wait before getting |fetcher|.

-  runner->RunPendingTasks();
-
   net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
   DCHECK(fetcher);
   fetcher->set_status(net::URLRequestStatus());
@@ -269,8 +263,6 @@ TEST_F(V4UpdateProtocolManagerTest, TestGetUpdatesWithOneBackoff) {
 
   EXPECT_FALSE(pm->IsUpdateScheduled());
 
-  runner->RunPendingTasks();
-
   net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
   DCHECK(fetcher);
   fetcher->set_status(net::URLRequestStatus());
@@ -286,6 +278,7 @@ TEST_F(V4UpdateProtocolManagerTest, TestGetUpdatesWithOneBackoff) {
 
   // Retry, now no backoff.
   expect_callback_to_be_called_ = true;
+  // Call RunPendingTasks to ensure that the request is sent after backoff.
   runner->RunPendingTasks();
 
   fetcher = factory.GetFetcherByID(1);
@@ -340,4 +333,42 @@ TEST_F(V4UpdateProtocolManagerTest, TestDisableAutoUpdates) {
   DCHECK(!fetcher);
 }
 
+TEST_F(V4UpdateProtocolManagerTest, TestGetUpdatesHasTimeout) {
+  scoped_refptr<base::TestSimpleTaskRunner> runner(
+      new base::TestSimpleTaskRunner());
+  base::ThreadTaskRunnerHandle runner_handler(runner);
+  net::TestURLFetcherFactory factory;
+  std::vector<ListUpdateResponse> expected_lurs;
+  SetupExpectedListUpdateResponse(&expected_lurs);
+  std::unique_ptr<V4UpdateProtocolManager> pm(
+      CreateProtocolManager(expected_lurs));
+  runner->ClearPendingTasks();
+
+  // Initial state. No errors.
+  EXPECT_EQ(0ul, pm->update_error_count_);
+  EXPECT_EQ(1ul, pm->update_back_off_mult_);
+  expect_callback_to_be_called_ = true;
+  pm->store_state_map_ = std::move(store_state_map_);
+  pm->IssueUpdateRequest();
+
+  net::TestURLFetcher* timeout_fetcher = factory.GetFetcherByID(0);
+  DCHECK(timeout_fetcher);
+  // Don't set anything on the fetcher. Let it time out.
+  runner->RunPendingTasks();
+  // Now wait for the next request to be scheduled.

[Nathan Parker, 2016/11/08 23:39:29]

Is there some state you can test between these to verify it changes?  Like,
maybe there shouldn't be a Fetcher(1) yet?

[vakh (use Gerrit instead), 2016/11/09 00:08:51]

Done. Not much else to test.

+  runner->RunPendingTasks();
+
+  // There should be another fetcher now.
+  net::TestURLFetcher* fetcher = factory.GetFetcherByID(1);
+  DCHECK(fetcher);
+  fetcher->set_status(net::URLRequestStatus());
+  fetcher->set_response_code(net::HTTP_OK);
+  fetcher->SetResponseString(GetExpectedV4UpdateResponse(expected_lurs));
+  fetcher->delegate()->OnURLFetchComplete(fetcher);
+
+  // No error, back off multiplier is unchanged.
+  EXPECT_EQ(0ul, pm->update_error_count_);
+  EXPECT_EQ(1ul, pm->update_back_off_mult_);
+}
+
 }  // namespace safe_browsing