Fetch extension blacklist states from SafeBrowsing server

chrome/browser/extensions/blacklist.cc

 // Copyright 2012 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 "chrome/browser/extensions/blacklist.h"
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/bind.h"
 #include "base/lazy_instance.h"
 #include "base/memory/ref_counted.h"
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/extensions/blacklist_state_fetcher.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/safe_browsing_util.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 
 using content::BrowserThread;
 
 namespace extensions {
@@ skipping 94 matching lines @@
   callback.Run(state_map.empty() ? Blacklist::NOT_BLACKLISTED
                                  : state_map.begin()->second);
 }
 
 void GetMalwareFromBlacklistStateMap(
     const Blacklist::GetMalwareIDsCallback& callback,
     const Blacklist::BlacklistStateMap& state_map) {
   std::set<std::string> malware;
   for (Blacklist::BlacklistStateMap::const_iterator it = state_map.begin();
        it != state_map.end(); ++it) {
-    if (it->second == Blacklist::BLACKLISTED_MALWARE)
+    // TODO(oleg): UNKNOWN is treated as MALWARE for backwards compatibility.
+    // In future GetMalwareIDs will be removed and the caller will have to
+    // deal with BLACKLISTED_UNKNOWN state returned from GetBlacklistedIDs.
+    if (it->second == Blacklist::BLACKLISTED_MALWARE ||
+        it->second == Blacklist::BLACKLISTED_UNKNOWN)
       malware.insert(it->first);
   }
   callback.Run(malware);
 }
 
 }  // namespace
 
 Blacklist::Observer::Observer(Blacklist* blacklist) : blacklist_(blacklist) {
   blacklist_->AddObserver(this);
 }
@@ skipping 74 matching lines @@
     const GetBlacklistedIDsCallback& callback,
     const std::set<std::string>& blacklisted_ids) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   std::set<std::string> ids_unknown_state;
   BlacklistStateMap extensions_state;
   for (std::set<std::string>::const_iterator it = blacklisted_ids.begin();
        it != blacklisted_ids.end(); ++it) {
     BlacklistStateMap::const_iterator cache_it =
         blacklist_state_cache_.find(*it);
-    if (cache_it == blacklist_state_cache_.end())
+    if (cache_it == blacklist_state_cache_.end() ||
+        cache_it->second == BLACKLISTED_UNKNOWN)  // Do not return UNKNOWN
+                                                  // from cache, retry request.
       ids_unknown_state.insert(*it);
     else
       extensions_state[*it] = cache_it->second;
   }
 
   if (ids_unknown_state.empty()) {
     callback.Run(extensions_state);
   } else {
     // After the extension blacklist states have been downloaded, call this
     // functions again, but prevent infinite cycle in case server is offline
@@ skipping 17 matching lines @@
     if (cache_it != blacklist_state_cache_.end())
       extensions_state[*it] = cache_it->second;
     // If for some reason we still haven't cached the state of this extension,
     // we silently skip it.
   }
 
   callback.Run(extensions_state);
 }
 
 void Blacklist::RequestExtensionsBlacklistState(
-    const std::set<std::string> ids, base::Callback<void()> callback) {
+    const std::set<std::string>& ids, const base::Callback<void()>& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  // This is a stub. The request will be made here, but the server is not up
-  // yet. For compatibility with current blacklist logic, mark all extensions
-  // as malicious.
+  if (!state_fetcher_)
+    state_fetcher_.reset(new BlacklistStateFetcher());
+
+  state_requests_.push_back(
+      make_pair(std::vector<std::string>(ids.begin(), ids.end()), callback));
   for (std::set<std::string>::const_iterator it = ids.begin();
        it != ids.end();
        ++it) {
-    blacklist_state_cache_[*it] = BLACKLISTED_MALWARE;
+    state_fetcher_->Request(

[mattm, 2013/11/25 21:48:55]

Have you tested this in a debug build? This function DCHECKS for UI thread and
BlacklistStateFetcher::Request DCHECKS for IO thread.

[Oleg Eterevsky, 2013/11/27 14:07:38]

Done.

+        *it,
+        base::Bind(&Blacklist::OnBlacklistStateReceived, AsWeakPtr(), *it));
   }
-  callback.Run();
+}
+
+void Blacklist::OnBlacklistStateReceived(const std::string& id,
+                                         BlacklistState state) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  blacklist_state_cache_[id] = state;
+
+  // Go through the opened requests and call the callbacks for those requests
+  // for which we already got all the required blacklist states.
+  StateRequestsList::iterator requests_it = state_requests_.begin();
+  while (requests_it != state_requests_.end()) {
+    const std::vector<std::string>& ids = requests_it->first;
+
+    bool have_all_in_cache = true;
+    for (std::vector<std::string>::const_iterator ids_it = ids.begin();
+         ids_it != ids.end();
+         ++ids_it) {
+      if (!ContainsKey(blacklist_state_cache_, *ids_it)) {
+        have_all_in_cache = false;
+        break;
+      }
+    }
+
+    // When erase an element from std::list, the iterator for the next element
+    // is returned, so no need to increment it.

[not at google - send to devlin, 2013/11/25 18:48:24]

a lot of words to explain some C++ library behaviour; could you replace this
with just

requests_it = state_requests_.erase(requests_it);  // returns next element

[Oleg Eterevsky, 2013/11/27 14:07:38]

Done.

+    if (have_all_in_cache) {
+      requests_it->second.Run();
+      requests_it = state_requests_.erase(requests_it);
+    } else {
+      ++requests_it;
+    }
+  }
+}
+
+void Blacklist::SetBlacklistStateFetcherForTest(
+    BlacklistStateFetcher* fetcher) {
+  state_fetcher_.reset(fetcher);
 }
 
 void Blacklist::AddObserver(Observer* observer) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   observers_.AddObserver(observer);
 }
 
 void Blacklist::RemoveObserver(Observer* observer) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   observers_.RemoveObserver(observer);
@@ skipping 11 matching lines @@
 }
 
 void Blacklist::Observe(int type,
                         const content::NotificationSource& source,
                         const content::NotificationDetails& details) {
   DCHECK_EQ(chrome::NOTIFICATION_SAFE_BROWSING_UPDATE_COMPLETE, type);
   FOR_EACH_OBSERVER(Observer, observers_, OnBlacklistUpdated());
 }
 
 }  // namespace extensions