SafeBrowsing: Track and cancel API checks.

components/safe_browsing_db/database_manager.cc

Index: components/safe_browsing_db/database_manager.cc
diff --git a/components/safe_browsing_db/database_manager.cc b/components/safe_browsing_db/database_manager.cc
index df67504207e3e34c1e977978165770c5dcc82378..fa5eac413f006f667e17609c4d45647b830bb6eb 100644
--- a/components/safe_browsing_db/database_manager.cc
+++ b/components/safe_browsing_db/database_manager.cc
@@ -40,7 +40,29 @@ void SafeBrowsingDatabaseManager::StopOnIOThread(bool shutdown) {
     delete v4_get_hash_protocol_manager_;
     v4_get_hash_protocol_manager_ = NULL;
   }
-  // TODO(kcarattini): Call back clients with pending requests.
+
+  // Delete pending checks, calling back any clients with empty metadata.
+  for (CurrentApiChecks::iterator it = api_checks_.begin();

[Nathan Parker, 2016/04/15 23:27:35]

for (auto itr : api_checks_) ?

[kcarattini, 2016/04/18 03:00:09]

Done.

+      it != api_checks_.end(); ++it) {
+    std::shared_ptr<SafeBrowsingApiCheck> check = *it;
+    if (check->client()) {
+      check->client()->
+          OnCheckApiBlacklistUrlResult(check->url(), ThreatMetadata());
+    }
+  }
+  api_checks_.clear();
+}
+
+bool SafeBrowsingDatabaseManager::CancelApiCheck(Client* client) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  for (CurrentApiChecks::iterator it = api_checks_.begin();
+      it != api_checks_.end(); ++it) {
+    if ((*it)->client() == client) {
+      api_checks_.erase(it);
+      return true;
+    }
+  }
+  return false;

[Nathan Parker, 2016/04/15 23:27:35]

Add NOTREACHED()

[kcarattini, 2016/04/18 03:00:09]

Done.

 }
 
 bool SafeBrowsingDatabaseManager::CheckApiBlacklistUrl(const GURL& url,
@@ -69,9 +91,11 @@ bool SafeBrowsingDatabaseManager::CheckApiBlacklistUrl(const GURL& url,
   prefixes.erase(std::unique(prefixes.begin(), prefixes.end()), prefixes.end());
   DCHECK(!prefixes.empty());
 
-  // TODO(kcarattini): Track checks in a map.
+  // TODO(kcarattini): Merge multiple checks for the same full_hashes? What
+  // about pages that request two permissions?
   std::shared_ptr<SafeBrowsingApiCheck> check(
       new SafeBrowsingApiCheck(url, full_hashes, client));
+  api_checks_.insert(check);

[Nathan Parker, 2016/04/15 23:27:35]

I _think_ this could be a scoped_ptr if you just assume the api_checks_ set owns
it. I think that's how the local and remote db managers do it.

[kcarattini, 2016/04/18 03:00:09]

The local and remote managers both store the raw pointer in the set/vector and
manage the memory manually. I think unique_ptr would work in the set, but then
I'd have to pass around the raw pointer to the non-owners. Which of those do you
prefer?

[Nathan Parker, 2016/04/18 17:29:58]

The raw ptr is probably fine since that continues an existing pattern.  Just
document that they're owned there.

 
   // TODO(kcarattini): Implement cache compliance.
   v4_get_hash_protocol_manager_->GetFullHashesWithApis(prefixes,
@@ -88,6 +112,12 @@ void SafeBrowsingDatabaseManager::HandleGetHashesWithApisResults(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(check);
 
+  // If the check is not in |api_checks_| then the request was cancelled by the
+  // client.
+  CurrentApiChecks::iterator it = api_checks_.find(check);
+  if (it == api_checks_.end())
+    return;
+
   ThreatMetadata md;
   // Merge the metadata from all matching results.
   for (const SBFullHashResult& result : full_hash_results) {
@@ -102,6 +132,7 @@ void SafeBrowsingDatabaseManager::HandleGetHashesWithApisResults(
   }
 
   check->client()->OnCheckApiBlacklistUrlResult(check->url(), md);
+  api_checks_.erase(it);
 }
 
 SafeBrowsingDatabaseManager::SafeBrowsingApiCheck::SafeBrowsingApiCheck(