Fetch extension blacklist states from SafeBrowsing server

chrome/browser/extensions/blacklist_fetcher.cc

+// Copyright 2013 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_fetcher.h"
+
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/safe_browsing/protocol_manager_helper.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/common/safe_browsing/crx_info.pb.h"
+#include "google_apis/google_api_keys.h"
+#include "net/base/escape.h"
+#include "net/url_request/url_request_status.h"
+
+using content::BrowserThread;
+
+namespace extensions {
+
+BlacklistFetcher::BlacklistFetcher()
+    : safe_browsing_config_initialized_(false),
+      url_fetcher_id_(0) {
+}
+
+void BlacklistFetcher::Request(const std::string& id,
+                               const RequestCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (!safe_browsing_config_initialized_) {
+    if (g_browser_process && g_browser_process->safe_browsing_service()) {
+      SetSafeBrowsingConfig(
+          g_browser_process->safe_browsing_service()->GetProtocolConfig());
+    } else {
+      base::MessageLoopProxy::current()->PostTask(
+          FROM_HERE, base::Bind(callback, Blacklist::NOT_BLACKLISTED));

[not at google - send to devlin, 2013/11/18 17:03:49]

are you sure you shouldn't wait until its initialized before responding?

[Oleg Eterevsky, 2013/11/20 12:58:45]

I was under impression that if safe_browsing_service() returns NULL, then
SafeBrowsing is turned off completely. If SafeBrowsingService was not created by
the time safe_browsing_service() is called, it is created synchronously and
returned
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/bro...)
unless SafeBrowsing is turned off.

That said, I think, I should return BLACKLISTED_UNKNOWN here.

Also added a comment.

[not at google - send to devlin, 2013/11/20 23:44:48]

Yes you're right, I misread this a little bit, sorry.

+      return;
+    }
+  }
+
+  bool request_already_sent = callbacks_.find(id) != callbacks_.end();

[not at google - send to devlin, 2013/11/18 17:03:49]

likewise since you're not using the iterator result of callbacks_.find you could
do callbacks._count(id) > 0 here.

[Oleg Eterevsky, 2013/11/20 12:58:45]

Ditto.

+  callbacks_.insert(std::make_pair(id, callback));
+  if (request_already_sent)
+    return;
+
+  ClientCRXListInfoRequest request;
+
+  request.set_id(id);
+  std::string request_str;
+  request.SerializeToString(&request_str);
+
+  GURL request_url = RequestUrl();

[not at google - send to devlin, 2013/11/18 17:03:49]

inline this?

[Oleg Eterevsky, 2013/11/20 12:58:45]

It looks more readable like this, and the compiler will inline it anyway.

[not at google - send to devlin, 2013/11/20 23:44:48]

It's not about inlining it's about readability, so I guess we disagree on that.

+  net::URLFetcher* fetcher = net::URLFetcher::Create(url_fetcher_id_++,
+                                                     request_url,
+                                                     net::URLFetcher::POST,
+                                                     this);
+  requests_[fetcher] = id;
+  fetcher->SetAutomaticallyRetryOn5xx(false);  // Don't retry on error.
+  fetcher->SetUploadData("application/octet-stream", request_str);
+  fetcher->Start();
+}
+
+void BlacklistFetcher::SetSafeBrowsingConfig(
+    const SafeBrowsingProtocolConfig& config) {
+  safe_browsing_config_ = config;
+  safe_browsing_config_initialized_ = true;
+}
+
+GURL BlacklistFetcher::RequestUrl() const {
+  std::string url = base::StringPrintf(
+      "%s/%s?client=%s&appver=%s&pver=2.2",
+      safe_browsing_config_.url_prefix.c_str(),
+      "clientreport/crx-list-info",
+      safe_browsing_config_.client_name.c_str(),
+      safe_browsing_config_.version.c_str());
+  std::string api_key = google_apis::GetAPIKey();
+  if (!api_key.empty()) {
+    base::StringAppendF(&url, "&key=%s",
+                        net::EscapeQueryParamValue(api_key, true).c_str());
+  }
+  return GURL(url);
+}
+
+void BlacklistFetcher::OnURLFetchComplete(const net::URLFetcher* source) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  std::map<const net::URLFetcher*, std::string>::iterator it =
+     requests_.find(source);
+  if (it == requests_.end())
+    return;
+
+  scoped_ptr<const net::URLFetcher> fetcher;
+
+  fetcher.reset(it->first);
+  std::string id = it->second;
+  requests_.erase(it);
+
+  Blacklist::BlacklistState state;
+
+  if (source->GetStatus().is_success() && source->GetResponseCode() == 200) {
+    std::string data;
+    source->GetResponseAsString(&data);
+    ClientCRXListInfoResponse response;
+    if (response.ParseFromString(data)) {
+      state = static_cast<Blacklist::BlacklistState>(response.verdict());
+    } else {
+      state = Blacklist::NOT_BLACKLISTED;
+    }
+  } else {
+    if (source->GetStatus().status() == net::URLRequestStatus::FAILED) {
+      VLOG(1) << "Blacklist request for: " << id
+              << " failed with error: " << source->GetStatus().error();
+    } else {
+      VLOG(1) << "Blacklist request for: " << id
+              << " failed with error: " << source->GetResponseCode();
+    }
+
+    state = Blacklist::BLACKLISTED_UNKNOWN;
+  }
+
+  std::pair<CallbackMultiMap::iterator, CallbackMultiMap::iterator> range =
+      callbacks_.equal_range(id);
+  for (CallbackMultiMap::const_iterator callback_it = range.first;
+       callback_it != range.second;
+       ++callback_it) {
+    callback_it->second.Run(state);
+  }
+
+  callbacks_.erase(range.first, range.second);
+}
+
+}  // namespace extensions
+