SafeBrowsing: Track and cancel API checks.

components/safe_browsing_db/database_manager.cc

 // Copyright 2015 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 "components/safe_browsing_db/database_manager.h"
 
 #include "components/safe_browsing_db/v4_get_hash_protocol_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "url/gurl.h"
@@ skipping 22 matching lines @@
 // |shutdown| not used. Destroys the v4 protocol managers. This may be called
 // multiple times during the life of the DatabaseManager.
 // Must be called on IO thread.
 void SafeBrowsingDatabaseManager::StopOnIOThread(bool shutdown) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   // This cancels all in-flight GetHash requests.
   if (v4_get_hash_protocol_manager_) {
     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,
                                                        Client* client) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(v4_get_hash_protocol_manager_);
 
   // Make sure we can check this url.
   if (!(url.SchemeIs(url::kHttpScheme) || url.SchemeIs(url::kHttpsScheme))) {
     return true;
   }
 
   // Compute a list of hashes for this url.
   std::vector<SBFullHash> full_hashes;
   UrlToFullHashes(url, false, &full_hashes);
   if (full_hashes.empty())
     return true;
 
   // Copy to prefixes.
   std::vector<SBPrefix> prefixes;
   for (const SBFullHash& full_hash : full_hashes) {
     prefixes.push_back(full_hash.prefix);
   }
   // Multiple full hashes could share a prefix, remove duplicates.
   std::sort(prefixes.begin(), prefixes.end());
   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,
       base::Bind(&SafeBrowsingDatabaseManager::HandleGetHashesWithApisResults,
                  base::Unretained(this), check));
 
   return false;
 }
 
 void SafeBrowsingDatabaseManager::HandleGetHashesWithApisResults(
     std::shared_ptr<SafeBrowsingApiCheck> check,
     const std::vector<SBFullHashResult>& full_hash_results,
     const base::TimeDelta& negative_cache_duration) {
   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) {
     for (const SBFullHash& full_hash : check->full_hashes()) {
       if (SBFullHashEqual(full_hash, result.hash)) {
         md.api_permissions.insert(md.api_permissions.end(),
                                   result.metadata.api_permissions.begin(),
                                   result.metadata.api_permissions.end());
         break;
       }
     }
   }
 
   check->client()->OnCheckApiBlacklistUrlResult(check->url(), md);
+  api_checks_.erase(it);
 }
 
 SafeBrowsingDatabaseManager::SafeBrowsingApiCheck::SafeBrowsingApiCheck(
     const GURL& url, const std::vector<SBFullHash>& full_hashes, Client* client)
         : url_(url), full_hashes_(full_hashes), client_(client) {
 }
 
 SafeBrowsingDatabaseManager::SafeBrowsingApiCheck::~SafeBrowsingApiCheck() {
 }
 
 }  // namespace safe_browsing