Use DownloadItem directly in DownloadProtectionService.

chrome/browser/safe_browsing/download_protection_service.cc

 // Copyright (c) 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/safe_browsing/download_protection_service.h"
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/format_macros.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 121 matching lines @@
 
   DOWNLOAD_HASH_CHECKS_TOTAL,
   DOWNLOAD_HASH_CHECKS_MALWARE,
 
   // Memory space for histograms is determined by the max.
   // ALWAYS ADD NEW VALUES BEFORE THIS ONE.
   DOWNLOAD_CHECKS_MAX
 };
 }  // namespace
 
-DownloadProtectionService::DownloadInfo::DownloadInfo()
-    : total_bytes(0), user_initiated(false), zipped_executable(false) {}
-
-DownloadProtectionService::DownloadInfo::~DownloadInfo() {}
-
-std::string DownloadProtectionService::DownloadInfo::DebugString() const {
-  std::string chain;
-  for (size_t i = 0; i < download_url_chain.size(); ++i) {
-    chain += download_url_chain[i].spec();
-    if (i < download_url_chain.size() - 1) {
-      chain += " -> ";
-    }
-  }
-  return base::StringPrintf(
-      "DownloadInfo {addr:0x%p, download_url_chain:[%s], local_file:%"
-      PRFilePath ", target_file:%" PRFilePath ", referrer_url:%s, "
-      "sha256_hash:%s, total_bytes:%" PRId64 ", user_initiated: %s, "
-      "zipped_executable: %s}",
-      reinterpret_cast<const void*>(this),
-      chain.c_str(),
-      local_file.value().c_str(),
-      target_file.value().c_str(),
-      referrer_url.spec().c_str(),
-      base::HexEncode(sha256_hash.data(), sha256_hash.size()).c_str(),
-      total_bytes,
-      user_initiated ? "true" : "false",
-      zipped_executable ? "true" : "false");
-}
-
-// static
-DownloadProtectionService::DownloadInfo
-DownloadProtectionService::DownloadInfo::FromDownloadItem(
-    const content::DownloadItem& item) {
-  DownloadInfo download_info;
-  download_info.target_file = item.GetTargetFilePath();
-  download_info.sha256_hash = item.GetHash();
-  download_info.local_file = item.GetFullPath();
-  download_info.download_url_chain = item.GetUrlChain();
-  download_info.referrer_url = item.GetReferrerUrl();
-  download_info.total_bytes = item.GetTotalBytes();
-  download_info.remote_address = item.GetRemoteAddress();
-  download_info.user_initiated = item.HasUserGesture();
-  return download_info;
-}
-
 // Parent SafeBrowsing::Client class used to lookup the bad binary
 // URL and digest list.  There are two sub-classes (one for each list).
 class DownloadSBClient
     : public SafeBrowsingDatabaseManager::Client,
       public base::RefCountedThreadSafe<DownloadSBClient> {
  public:
   DownloadSBClient(
-      const DownloadProtectionService::DownloadInfo& info,
+      const content::DownloadItem& item,
       const DownloadProtectionService::CheckDownloadCallback& callback,
       const scoped_refptr<SafeBrowsingUIManager>& ui_manager,
       SBStatsType total_type,
       SBStatsType dangerous_type)
-      : info_(info),
+      : sha256_hash_(item.GetHash()),
+        url_chain_(item.GetUrlChain()),
+        referrer_url_(item.GetReferrerUrl()),
         callback_(callback),
         ui_manager_(ui_manager),
         start_time_(base::TimeTicks::Now()),
         total_type_(total_type),
         dangerous_type_(dangerous_type) {}
 
   virtual void StartCheck() = 0;
   virtual bool IsDangerous(SBThreatType threat_type) const = 0;
 
  protected:
@@ skipping 14 matching lines @@
       BrowserThread::PostTask(
           BrowserThread::UI,
           FROM_HERE,
           base::Bind(&DownloadSBClient::ReportMalware,
                      this, threat_type));
     }
   }
 
   void ReportMalware(SBThreatType threat_type) {
     std::string post_data;
-    if (!info_.sha256_hash.empty())
-      post_data += base::HexEncode(info_.sha256_hash.data(),
-                                   info_.sha256_hash.size()) + "\n";
-    for (size_t i = 0; i < info_.download_url_chain.size(); ++i) {
-      post_data += info_.download_url_chain[i].spec() + "\n";
+    if (!sha256_hash_.empty())
+      post_data += base::HexEncode(sha256_hash_.data(),
+                                   sha256_hash_.size()) + "\n";
+    for (size_t i = 0; i < url_chain_.size(); ++i) {
+      post_data += url_chain_[i].spec() + "\n";
     }
     ui_manager_->ReportSafeBrowsingHit(
-        info_.download_url_chain.back(),  // malicious_url
-        info_.download_url_chain.front(), // page_url
-        info_.referrer_url,
+        url_chain_.back(),  // malicious_url
+        url_chain_.front(), // page_url
+        referrer_url_,
         true,  // is_subresource
         threat_type,
         post_data);
   }
 
   void UpdateDownloadCheckStats(SBStatsType stat_type) {
     UMA_HISTOGRAM_ENUMERATION("SB2.DownloadChecks",
                               stat_type,
                               DOWNLOAD_CHECKS_MAX);
   }
 
-  DownloadProtectionService::DownloadInfo info_;
+  std::string sha256_hash_;
+  std::vector<GURL> url_chain_;
+  GURL referrer_url_;
   DownloadProtectionService::CheckDownloadCallback callback_;
   scoped_refptr<SafeBrowsingUIManager> ui_manager_;
   base::TimeTicks start_time_;
 
  private:
   const SBStatsType total_type_;
   const SBStatsType dangerous_type_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadSBClient);
 };
 
 class DownloadUrlSBClient : public DownloadSBClient {
  public:
   DownloadUrlSBClient(
-      const DownloadProtectionService::DownloadInfo& info,
+      const content::DownloadItem& item,
       const DownloadProtectionService::CheckDownloadCallback& callback,
       const scoped_refptr<SafeBrowsingUIManager>& ui_manager,
       const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager)
-      : DownloadSBClient(info, callback, ui_manager,
+      : DownloadSBClient(item, callback, ui_manager,
                          DOWNLOAD_URL_CHECKS_TOTAL,
                          DOWNLOAD_URL_CHECKS_MALWARE),
         database_manager_(database_manager) { }
 
   virtual void StartCheck() OVERRIDE {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
     if (!database_manager_ || database_manager_->CheckDownloadUrl(
-            info_.download_url_chain, this)) {
+           url_chain_, this)) {
       CheckDone(SB_THREAT_TYPE_SAFE);
     } else {
       AddRef();  // SafeBrowsingService takes a pointer not a scoped_refptr.
     }
   }
 
   virtual bool IsDangerous(SBThreatType threat_type) const OVERRIDE {
     return threat_type == SB_THREAT_TYPE_BINARY_MALWARE_URL;
   }
 
@@ skipping 11 matching lines @@
  private:
   scoped_refptr<SafeBrowsingDatabaseManager> database_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadUrlSBClient);
 };
 
 class DownloadProtectionService::CheckClientDownloadRequest
     : public base::RefCountedThreadSafe<
           DownloadProtectionService::CheckClientDownloadRequest,
           BrowserThread::DeleteOnUIThread>,
-      public net::URLFetcherDelegate {
+      public net::URLFetcherDelegate,
+      public content::DownloadItem::Observer {
  public:
   CheckClientDownloadRequest(
-      const DownloadInfo& info,
+      content::DownloadItem* item,
       const CheckDownloadCallback& callback,
       DownloadProtectionService* service,
       const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager,
       SignatureUtil* signature_util)
-      : info_(info),
+      : item_(item),
+        zipped_executable_(false),
         callback_(callback),
         service_(service),
         signature_util_(signature_util),
         database_manager_(database_manager),
         pingback_enabled_(service_->enabled()),
         finished_(false),
         type_(ClientDownloadRequest::WIN_EXECUTABLE),
         ALLOW_THIS_IN_INITIALIZER_LIST(weakptr_factory_(this)),
         start_time_(base::TimeTicks::Now()) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    item_->AddObserver(this);
   }
 
   void Start() {
     VLOG(2) << "Starting SafeBrowsing download check for: "
-            << info_.DebugString();
+            << item_->DebugString(true);
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     // TODO(noelutz): implement some cache to make sure we don't issue the same
     // request over and over again if a user downloads the same binary multiple
     // times.
     DownloadCheckResultReason reason = REASON_MAX;
-    if (!IsSupportedDownload(info_, &reason, &type_)) {
+    if (!IsSupportedDownload(
+        *item_, item_->GetTargetFilePath(), &reason, &type_)) {
       switch (reason) {
         case REASON_EMPTY_URL_CHAIN:
         case REASON_INVALID_URL:
           RecordImprovedProtectionStats(reason);
           PostFinishTask(SAFE);
           return;
 
         case REASON_NOT_BINARY_FILE:
-          RecordFileExtensionType(info_.target_file);
+          RecordFileExtensionType(item_->GetTargetFilePath());
           RecordImprovedProtectionStats(reason);
           PostFinishTask(SAFE);
           return;
 
         default:
           // We only expect the reasons explicitly handled above.
           NOTREACHED();
       }
     }
-    RecordFileExtensionType(info_.target_file);
+    RecordFileExtensionType(item_->GetTargetFilePath());
 
     // Compute features from the file contents. Note that we record histograms
     // based on the result, so this runs regardless of whether the pingbacks
     // are enabled.
-    if (info_.target_file.MatchesExtension(FILE_PATH_LITERAL(".zip"))) {
+    if (item_->GetTargetFilePath().MatchesExtension(
+        FILE_PATH_LITERAL(".zip"))) {
       StartExtractZipFeatures();
     } else {
-      DCHECK(!download_protection_util::IsArchiveFile(info_.target_file));
+      DCHECK(!download_protection_util::IsArchiveFile(
+          item_->GetTargetFilePath()));
       StartExtractSignatureFeatures();
     }
   }
 
   // Start a timeout to cancel the request if it takes too long.
   // This should only be called after we have finished accessing the file.
   void StartTimeout() {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     if (!service_) {
       // Request has already been cancelled.
@@ skipping 21 matching lines @@
       // might be destroyed before the URLFetcher completes.  Cancel the
       // fetcher so it does not try to invoke OnURLFetchComplete.
       fetcher_.reset();
     }
     // Note: If there is no fetcher, then some callback is still holding a
     // reference to this object.  We'll eventually wind up in some method on
     // the UI thread that will call FinishRequest() again.  If FinishRequest()
     // is called a second time, it will be a no-op.
   }
 
+  // content::DownloadItem::Observer implementation.
+  virtual void OnDownloadDestroyed(content::DownloadItem* download) OVERRIDE {
+    Cancel();
+    DCHECK(item_ == NULL);
+  }
+
   // From the net::URLFetcherDelegate interface.
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     DCHECK_EQ(source, fetcher_.get());
     VLOG(2) << "Received a response for URL: "
-            << info_.download_url_chain.back() << ": success="
+            << item_->GetUrlChain().back() << ": success="
             << source->GetStatus().is_success() << " response_code="
             << source->GetResponseCode();
     DownloadCheckResultReason reason = REASON_SERVER_PING_FAILED;
     DownloadCheckResult result = SAFE;
     if (source->GetStatus().is_success() &&
         net::HTTP_OK == source->GetResponseCode()) {
       ClientDownloadResponse response;
       std::string data;
       bool got_data = source->GetResponseAsString(&data);
       DCHECK(got_data);
       if (!response.ParseFromString(data)) {
         reason = REASON_INVALID_RESPONSE_PROTO;
       } else if (response.verdict() == ClientDownloadResponse::SAFE) {
         reason = REASON_DOWNLOAD_SAFE;
-      } else if (service_ && !service_->IsSupportedDownload(info_)) {
+      } else if (service_ && !service_->IsSupportedDownload(
+          *item_, item_->GetTargetFilePath())) {
         // The client of the download protection service assumes that we don't
         // support this download so we cannot return any other verdict than
         // SAFE even if the server says it's dangerous to download this file.
         // Note: if service_ is NULL we already cancelled the request and
         // returned SAFE.
         reason = REASON_DOWNLOAD_NOT_SUPPORTED;
       } else if (response.verdict() == ClientDownloadResponse::DANGEROUS) {
         reason = REASON_DOWNLOAD_DANGEROUS;
         result = DANGEROUS;
       } else if (response.verdict() == ClientDownloadResponse::UNCOMMON) {
         reason = REASON_DOWNLOAD_UNCOMMON;
         result = UNCOMMON;
       } else if (response.verdict() == ClientDownloadResponse::DANGEROUS_HOST) {
         reason = REASON_DOWNLOAD_DANGEROUS_HOST;
         result = DANGEROUS_HOST;
       } else {
         LOG(DFATAL) << "Unknown download response verdict: "
                     << response.verdict();
         reason = REASON_INVALID_RESPONSE_VERDICT;
       }
     }
     // We don't need the fetcher anymore.
     fetcher_.reset();
     RecordImprovedProtectionStats(reason);
     UMA_HISTOGRAM_TIMES("SBClientDownload.DownloadRequestDuration",
                         base::TimeTicks::Now() - start_time_);
     FinishRequest(result);
   }
 
-  static bool IsSupportedDownload(const DownloadInfo& info,
+  static bool IsSupportedDownload(const content::DownloadItem& item,
+                                  const base::FilePath& target_path,
                                   DownloadCheckResultReason* reason,
                                   ClientDownloadRequest::DownloadType* type) {
-    if (info.download_url_chain.empty()) {
+    if (item.GetUrlChain().empty()) {
       *reason = REASON_EMPTY_URL_CHAIN;
       return false;
     }
-    const GURL& final_url = info.download_url_chain.back();
+    const GURL& final_url = item.GetUrlChain().back();
     if (!final_url.is_valid() || final_url.is_empty() ||
         !final_url.IsStandard() || final_url.SchemeIsFile()) {
       *reason = REASON_INVALID_URL;
       return false;
     }
-    if (!download_protection_util::IsBinaryFile(info.target_file)) {
+    if (!download_protection_util::IsBinaryFile(target_path)) {
       *reason = REASON_NOT_BINARY_FILE;
       return false;
     }
-    *type = GetDownloadType(info.target_file);
+    *type = GetDownloadType(target_path);
     return true;
   }
 
  private:
   friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
   friend class base::DeleteHelper<CheckClientDownloadRequest>;
 
   virtual ~CheckClientDownloadRequest() {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    DCHECK(item_ == NULL);
   }
 
   void OnFileFeatureExtractionDone() {
     // This can run in any thread, since it just posts more messages.
 
     // TODO(noelutz): DownloadInfo should also contain the IP address of
     // every URL in the redirect chain.  We also should check whether the
     // download URL is hosted on the internal network.
     BrowserThread::PostTask(
         BrowserThread::IO,
         FROM_HERE,
         base::Bind(&CheckClientDownloadRequest::CheckWhitelists, this));

[noelutz, 2013/03/01 20:12:28]

this isn't a weak reference.  Does that mean that we increment the reference
count for this?  I think this means that item_ might be NULL in CheckWhitelists,
no?

[mattm, 2013/03/01 22:13:20]

Oh crap.  Good catch, I totally forgot the threading in this class.

Actually DownloadItem is not thread safe, so I made a bunch of changes to avoid
accessing it on other threads at all.

 
     // We wait until after the file checks finish to start the timeout, as
     // windows can cause permissions errors if the timeout fired while we were
     // checking the file signature and we tried to complete the download.
     BrowserThread::PostTask(
         BrowserThread::UI,
         FROM_HERE,
         base::Bind(&CheckClientDownloadRequest::StartTimeout, this));
   }
 
   void StartExtractSignatureFeatures() {
     // Since we do blocking I/O, offload this to a worker thread.
     // The task does not need to block shutdown.
     BrowserThread::GetBlockingPool()->PostWorkerTaskWithShutdownBehavior(
         FROM_HERE,
         base::Bind(&CheckClientDownloadRequest::ExtractSignatureFeatures,
                    this),
         base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
   }
 
   void ExtractSignatureFeatures() {
     base::TimeTicks start_time = base::TimeTicks::Now();

[noelutz, 2013/03/01 20:12:28]

Could the request get cancelled beween StartExtractSignatureFeatures() and
ExtractSignatureFeatures()?  If that's the case we should check here whether
item_ is NULL.

[mattm, 2013/03/01 22:13:20]

We can't just check item_ unless we also add locking, but I made it not depend
on item_ for the reasons above anyway.

-    signature_util_->CheckSignature(info_.local_file, &signature_info_);
+    signature_util_->CheckSignature(item_->GetFullPath(), &signature_info_);
     bool is_signed = (signature_info_.certificate_chain_size() > 0);
     if (is_signed) {
-      VLOG(2) << "Downloaded a signed binary: " << info_.local_file.value();
+      VLOG(2) << "Downloaded a signed binary: " << item_->GetFullPath().value();
     } else {
-      VLOG(2) << "Downloaded an unsigned binary: " << info_.local_file.value();
+      VLOG(2) << "Downloaded an unsigned binary: "
+              << item_->GetFullPath().value();
     }
     UMA_HISTOGRAM_BOOLEAN("SBClientDownload.SignedBinaryDownload", is_signed);
     UMA_HISTOGRAM_TIMES("SBClientDownload.ExtractSignatureFeaturesTime",
                         base::TimeTicks::Now() - start_time);
 
     OnFileFeatureExtractionDone();
   }
 
   void StartExtractZipFeatures() {
     zip_analysis_start_time_ = base::TimeTicks::Now();
     // We give the zip analyzer a weak pointer to this object.  Since the
     // analyzer is refcounted, it might outlive the request.
     analyzer_ = new SandboxedZipAnalyzer(
-        info_.local_file,
+        item_->GetFullPath(),
         base::Bind(&CheckClientDownloadRequest::OnZipAnalysisFinished,
                    weakptr_factory_.GetWeakPtr()));

[noelutz, 2013/03/01 20:12:28]

In that case it looks like we're safe.

[mattm, 2013/03/01 22:13:20]

Probably, yeah.  I added a test in OnZipAnalysisFinished just to be safe.

     analyzer_->Start();
   }
 
   void OnZipAnalysisFinished(const zip_analyzer::Results& results) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     if (results.success) {
-      info_.zipped_executable = results.has_executable;
-      VLOG(1) << "Zip analysis finished for " << info_.local_file.value()
+      zipped_executable_ = results.has_executable;
+      VLOG(1) << "Zip analysis finished for " << item_->GetFullPath().value()
               << ", has_executable=" << results.has_executable
               << " has_archive=" << results.has_archive;
     } else {
-      VLOG(1) << "Zip analysis failed for " << info_.local_file.value();
+      VLOG(1) << "Zip analysis failed for " << item_->GetFullPath().value();
     }
     UMA_HISTOGRAM_BOOLEAN("SBClientDownload.ZipFileHasExecutable",
-                          info_.zipped_executable);
+                          zipped_executable_);
     UMA_HISTOGRAM_BOOLEAN("SBClientDownload.ZipFileHasArchiveButNoExecutable",
-                          results.has_archive && !info_.zipped_executable);
+                          results.has_archive && !zipped_executable_);
     UMA_HISTOGRAM_TIMES("SBClientDownload.ExtractZipFeaturesTime",
                         base::TimeTicks::Now() - zip_analysis_start_time_);
 
-    if (!info_.zipped_executable) {
+    if (!zipped_executable_) {
       RecordImprovedProtectionStats(REASON_ARCHIVE_WITHOUT_BINARIES);
       PostFinishTask(SAFE);
       return;
     }
     OnFileFeatureExtractionDone();
   }
 
   void CheckWhitelists() {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
     DownloadCheckResultReason reason = REASON_MAX;
     if (!database_manager_) {
       reason = REASON_SB_DISABLED;
     } else {
-      for (size_t i = 0; i < info_.download_url_chain.size(); ++i) {
-        const GURL& url = info_.download_url_chain[i];
+      for (size_t i = 0; i < item_->GetUrlChain().size(); ++i) {
+        const GURL& url = item_->GetUrlChain()[i];
         if (url.is_valid() &&
             database_manager_->MatchDownloadWhitelistUrl(url)) {
           VLOG(2) << url << " is on the download whitelist.";
           reason = REASON_WHITELISTED_URL;
           break;
         }
       }
-      if (info_.referrer_url.is_valid() && reason == REASON_MAX &&
+      if (item_->GetReferrerUrl().is_valid() && reason == REASON_MAX &&
           database_manager_->MatchDownloadWhitelistUrl(
-              info_.referrer_url)) {
-        VLOG(2) << "Referrer url " << info_.referrer_url
+              item_->GetReferrerUrl())) {
+        VLOG(2) << "Referrer url " << item_->GetReferrerUrl()
                 << " is on the download whitelist.";
         reason = REASON_WHITELISTED_REFERRER;
       }
       if (reason != REASON_MAX || signature_info_.trusted()) {
         UMA_HISTOGRAM_COUNTS("SBClientDownload.SignedOrWhitelistedDownload", 1);
       }
     }
     if (reason == REASON_MAX && signature_info_.trusted()) {
       for (int i = 0; i < signature_info_.certificate_chain_size(); ++i) {
         if (CertificateChainIsWhitelisted(
@@ skipping 26 matching lines @@
 #endif
     }
   }
 
   void SendRequest() {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
     // This is our last chance to check whether the request has been canceled
     // before sending it.
     if (!service_) {
       RecordImprovedProtectionStats(REASON_REQUEST_CANCELED);

[noelutz, 2013/03/01 22:24:09]

nice catch.

       FinishRequest(SAFE);
       return;
     }
 
     ClientDownloadRequest request;
-    request.set_url(info_.download_url_chain.back().spec());
-    request.mutable_digests()->set_sha256(info_.sha256_hash);
-    request.set_length(info_.total_bytes);
-    for (size_t i = 0; i < info_.download_url_chain.size(); ++i) {
+    request.set_url(item_->GetUrlChain().back().spec());
+    request.mutable_digests()->set_sha256(item_->GetHash());
+    request.set_length(item_->GetReceivedBytes());
+    for (size_t i = 0; i < item_->GetUrlChain().size(); ++i) {
       ClientDownloadRequest::Resource* resource = request.add_resources();
-      resource->set_url(info_.download_url_chain[i].spec());
-      if (i == info_.download_url_chain.size() - 1) {
+      resource->set_url(item_->GetUrlChain()[i].spec());
+      if (i == item_->GetUrlChain().size() - 1) {
         // The last URL in the chain is the download URL.
         resource->set_type(ClientDownloadRequest::DOWNLOAD_URL);
-        resource->set_referrer(info_.referrer_url.spec());
-        if (!info_.remote_address.empty()) {
-          resource->set_remote_ip(info_.remote_address);
+        resource->set_referrer(item_->GetReferrerUrl().spec());
+        if (!item_->GetRemoteAddress().empty()) {
+          resource->set_remote_ip(item_->GetRemoteAddress());
         }
       } else {
         resource->set_type(ClientDownloadRequest::DOWNLOAD_REDIRECT);
       }
       // TODO(noelutz): fill out the remote IP addresses.
     }
-    request.set_user_initiated(info_.user_initiated);
-    request.set_file_basename(info_.target_file.BaseName().AsUTF8Unsafe());
+    request.set_user_initiated(item_->HasUserGesture());
+    request.set_file_basename(
+        item_->GetTargetFilePath().BaseName().AsUTF8Unsafe());
     request.set_download_type(type_);
     request.mutable_signature()->CopyFrom(signature_info_);
     std::string request_data;
     if (!request.SerializeToString(&request_data)) {
       RecordImprovedProtectionStats(REASON_INVALID_REQUEST_PROTO);
       FinishRequest(SAFE);
       return;
     }
 
     VLOG(2) << "Sending a request for URL: "
-            << info_.download_url_chain.back();
+            << item_->GetUrlChain().back();
     fetcher_.reset(net::URLFetcher::Create(0 /* ID used for testing */,
                                            GURL(GetDownloadRequestUrl()),
                                            net::URLFetcher::POST,
                                            this));
     fetcher_->SetLoadFlags(net::LOAD_DISABLE_CACHE);
     fetcher_->SetAutomaticallyRetryOn5xx(false);  // Don't retry on error.
     fetcher_->SetRequestContext(service_->request_context_getter_.get());
     fetcher_->SetUploadData("application/octet-stream", request_data);
     fetcher_->Start();
   }
 
   void PostFinishTask(DownloadCheckResult result) {
     BrowserThread::PostTask(
         BrowserThread::UI,
         FROM_HERE,
         base::Bind(&CheckClientDownloadRequest::FinishRequest, this, result));
   }
 
   void FinishRequest(DownloadCheckResult result) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     if (finished_) {
       return;
     }
     finished_ = true;
     if (service_) {
       callback_.Run(result);
+      item_->RemoveObserver(this);
+      item_ = NULL;
       DownloadProtectionService* service = service_;
       service_ = NULL;
       service->RequestFinished(this);
       // DownloadProtectionService::RequestFinished will decrement our refcount,
       // so we may be deleted now.
     } else {
       callback_.Run(SAFE);
     }
   }
 
   void RecordImprovedProtectionStats(DownloadCheckResultReason reason) {
     VLOG(2) << "SafeBrowsing download verdict for: "
-            << info_.DebugString() << " verdict:" << reason;
+            << item_->DebugString(true) << " verdict:" << reason;
     UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckDownloadStats",
                               reason,
                               REASON_MAX);
   }
 
   bool CertificateChainIsWhitelisted(
       const ClientDownloadRequest_CertificateChain& chain) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
     if (chain.element_size() < 2) {
       // We need to have both a signing certificate and its issuer certificate
@@ skipping 26 matching lines @@
                   << cert->subject().GetDisplayName()
                   << " issuer=" << issuer->subject().GetDisplayName();
           return true;
         }
       }
       cert = issuer;
     }
     return false;
   }
 
-  DownloadInfo info_;
+  // Will be NULL if the request has been canceled.
+  content::DownloadItem* item_;
+  bool zipped_executable_;
   ClientDownloadRequest_SignatureInfo signature_info_;
   CheckDownloadCallback callback_;
   // Will be NULL if the request has been canceled.
   DownloadProtectionService* service_;
   scoped_refptr<SignatureUtil> signature_util_;
   scoped_refptr<SafeBrowsingDatabaseManager> database_manager_;
   const bool pingback_enabled_;
   scoped_ptr<net::URLFetcher> fetcher_;
   scoped_refptr<SandboxedZipAnalyzer> analyzer_;
   base::TimeTicks zip_analysis_start_time_;
@@ skipping 29 matching lines @@
   if (enabled == enabled_) {
     return;
   }
   enabled_ = enabled;
   if (!enabled_) {
     CancelPendingRequests();
   }
 }
 
 void DownloadProtectionService::CheckClientDownload(
-    const DownloadProtectionService::DownloadInfo& info,
+    content::DownloadItem* item,
     const CheckDownloadCallback& callback) {
   scoped_refptr<CheckClientDownloadRequest> request(
-      new CheckClientDownloadRequest(info, callback, this,
+      new CheckClientDownloadRequest(item, callback, this,
                                      database_manager_, signature_util_.get()));
   download_requests_.insert(request);
   request->Start();
 }
 
 void DownloadProtectionService::CheckDownloadUrl(
-    const DownloadProtectionService::DownloadInfo& info,
+    const content::DownloadItem& item,
     const CheckDownloadCallback& callback) {
-  DCHECK(!info.download_url_chain.empty());
+  DCHECK(!item.GetUrlChain().empty());
   scoped_refptr<DownloadUrlSBClient> client(
-      new DownloadUrlSBClient(info, callback, ui_manager_, database_manager_));
+      new DownloadUrlSBClient(item, callback, ui_manager_, database_manager_));
   // The client will release itself once it is done.
   BrowserThread::PostTask(
         BrowserThread::IO,
         FROM_HERE,
         base::Bind(&DownloadUrlSBClient::StartCheck, client));
 }
 
 bool DownloadProtectionService::IsSupportedDownload(
-    const DownloadInfo& info) const {
+    const content::DownloadItem& item,
+    const base::FilePath& target_path) const {
   // Currently, the UI only works on Windows.  On Linux and Mac we still
   // want to show the dangerous file type warning if the file is possibly
   // dangerous which means we have to always return false here.
 #if defined(OS_WIN)
   DownloadCheckResultReason reason = REASON_MAX;
   ClientDownloadRequest::DownloadType type =
       ClientDownloadRequest::WIN_EXECUTABLE;
-  return (CheckClientDownloadRequest::IsSupportedDownload(info,
-                                                          &reason,
-                                                          &type) &&
+  return (CheckClientDownloadRequest::IsSupportedDownload(item, target_path,
+                                                          &reason, &type) &&
           (ClientDownloadRequest::ANDROID_APK == type ||
            ClientDownloadRequest::WIN_EXECUTABLE == type ||
            ClientDownloadRequest::ZIPPED_EXECUTABLE == type));
 #else
   return false;
 #endif
 }
 
 void DownloadProtectionService::CancelPendingRequests() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ skipping 11 matching lines @@
 void DownloadProtectionService::RequestFinished(
     CheckClientDownloadRequest* request) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   std::set<scoped_refptr<CheckClientDownloadRequest> >::iterator it =
       download_requests_.find(request);
   DCHECK(it != download_requests_.end());
   download_requests_.erase(*it);
 }
 
 void DownloadProtectionService::ShowDetailsForDownload(
-    const DownloadProtectionService::DownloadInfo& info,
+    const content::DownloadItem& item,
     content::PageNavigator* navigator) {
   navigator->OpenURL(
       content::OpenURLParams(GURL(chrome::kDownloadScanningLearnMoreURL),
                              content::Referrer(),
                              NEW_FOREGROUND_TAB,
                              content::PAGE_TRANSITION_LINK,
                              false));
 }
 
 namespace {
@@ skipping 76 matching lines @@
   std::string url = kDownloadRequestUrl;
   std::string api_key = google_apis::GetAPIKey();
   if (!api_key.empty()) {
     base::StringAppendF(&url, "?key=%s",
                         net::EscapeQueryParamValue(api_key, true).c_str());
   }
   return url;
 }
 
 }  // namespace safe_browsing