Call CancelCheck if the call to the DB times out

Bug: 
Change-Id: Iffc70523b255f49ac905dc0db51b0f9ef758669f
Reviewed-on: https://chromium-review.googlesource.com/572221
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486952}

components/safe_browsing_db/whitelist_checker_client.cc

@@ -22,7 +22,8 @@ void WhitelistCheckerClient::StartCheckCsdWhitelist(
   // Make a client for each request. The caller could have several in
   // flight at once.
   std::unique_ptr<WhitelistCheckerClient> client =
-      base::MakeUnique<WhitelistCheckerClient>(callback_for_result);
+      base::MakeUnique<WhitelistCheckerClient>(callback_for_result,
+                                               database_manager);
   AsyncMatch match = database_manager->CheckCsdWhitelistUrl(url, client.get());
 
   switch (match) {
@@ -41,13 +42,16 @@ void WhitelistCheckerClient::StartCheckCsdWhitelist(
 }
 
 WhitelistCheckerClient::WhitelistCheckerClient(
-    base::Callback<void(bool)> callback_for_result)
-    : callback_for_result_(callback_for_result), weak_factory_(this) {
+    base::Callback<void(bool)> callback_for_result,
+    scoped_refptr<SafeBrowsingDatabaseManager> database_manager)
+    : callback_for_result_(callback_for_result),
+      database_manager_(database_manager),
+      weak_factory_(this) {
   // Set a timer to fail open, i.e. call it "whitelisted", if the full
   // check takes too long.
   auto timeout_callback =
-      base::Bind(&WhitelistCheckerClient::OnCheckWhitelistUrlResult,
-                 weak_factory_.GetWeakPtr(), true /* is_whitelisted */);
+      base::Bind(&WhitelistCheckerClient::OnCheckWhitelistUrlTimeout,
+                 weak_factory_.GetWeakPtr());
   timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(kTimeoutMsec),
                timeout_callback);
 }
@@ -56,9 +60,15 @@ WhitelistCheckerClient::~WhitelistCheckerClient() {}
 
 // SafeBrowsingDatabaseMananger::Client impl
 void WhitelistCheckerClient::OnCheckWhitelistUrlResult(bool is_whitelisted) {
-  // This method is invoked only if we're already self-owned.
+  timer_.Stop();
   callback_for_result_.Run(is_whitelisted);
+  // This method is invoked only if we're already self-owned.
   delete this;
 }
 
+void WhitelistCheckerClient::OnCheckWhitelistUrlTimeout() {
+  database_manager_->CancelCheck(this);
+  this->OnCheckWhitelistUrlResult(true /* is_whitelisted */);
+}
+
 }  // namespace safe_browsing

components/safe_browsing_db/whitelist_checker_client.h

@@ -28,7 +28,9 @@ class WhitelistCheckerClient : public SafeBrowsingDatabaseManager::Client {
       const GURL& url,
       BoolCallback callback_for_result);
 
-  WhitelistCheckerClient(BoolCallback callback_for_result);
+  WhitelistCheckerClient(
+      BoolCallback callback_for_result,
+      scoped_refptr<SafeBrowsingDatabaseManager> database_manager);
   ~WhitelistCheckerClient() override;
 
   // SafeBrowsingDatabaseMananger::Client impl
@@ -38,10 +40,14 @@ class WhitelistCheckerClient : public SafeBrowsingDatabaseManager::Client {
   static const int kTimeoutMsec = 5000;
   base::OneShotTimer timer_;
   BoolCallback callback_for_result_;
+  scoped_refptr<SafeBrowsingDatabaseManager> database_manager_;
   base::WeakPtrFactory<WhitelistCheckerClient> weak_factory_;
 
  private:
   WhitelistCheckerClient();
+
+  // Called when the call to CheckCsdWhitelistUrl times out.
+  void OnCheckWhitelistUrlTimeout();
 };
 
 }  // namespace safe_browsing

components/safe_browsing_db/whitelist_checker_client_unittest.cc

@@ -25,9 +25,12 @@ using testing::SaveArg;
 using BoolCallback = base::Callback<void(bool /* is_whitelisted */)>;
 using MockBoolCallback = testing::StrictMock<base::MockCallback<BoolCallback>>;
 
+namespace {
 class MockSafeBrowsingDatabaseManager : public TestSafeBrowsingDatabaseManager {
  public:
-  MockSafeBrowsingDatabaseManager(){};
+  MockSafeBrowsingDatabaseManager() {}
+
+  MOCK_METHOD1(CancelCheck, void(SafeBrowsingDatabaseManager::Client*));
 
   MOCK_METHOD2(CheckCsdWhitelistUrl,
                AsyncMatch(const GURL&, SafeBrowsingDatabaseManager::Client*));
@@ -38,6 +41,7 @@ class MockSafeBrowsingDatabaseManager : public TestSafeBrowsingDatabaseManager {
  private:
   DISALLOW_COPY_AND_ASSIGN(MockSafeBrowsingDatabaseManager);
 };
+}  // namespace
 
 class WhitelistCheckerClientTest : public testing::Test {
  public:
@@ -110,8 +114,10 @@ TEST_F(WhitelistCheckerClientTest, TestAsyncNoMatch) {
 }
 
 TEST_F(WhitelistCheckerClientTest, TestAsyncTimeout) {
+  SafeBrowsingDatabaseManager::Client* client;
   EXPECT_CALL(*database_manager_.get(), CheckCsdWhitelistUrl(target_url_, _))
-      .WillOnce(Return(AsyncMatch::ASYNC));
+      .WillOnce(DoAll(SaveArg<1>(&client), Return(AsyncMatch::ASYNC)));
+  EXPECT_CALL(*database_manager_.get(), CancelCheck(_)).Times(1);
 
   MockBoolCallback callback;
   WhitelistCheckerClient::StartCheckCsdWhitelist(database_manager_, target_url_,