Collect some histograms about signed binary downloads.

chrome/browser/download/chrome_download_manager_delegate.cc

 // Copyright (c) 2011 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/download/chrome_download_manager_delegate.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/file_util.h"
@@ skipping 207 matching lines @@
 
 void ChromeDownloadManagerDelegate::UpdateItemInPersistentStore(
     DownloadItem* item) {
   download_history_->UpdateEntry(item);
 }
 
 void ChromeDownloadManagerDelegate::UpdatePathForItemInPersistentStore(
     DownloadItem* item,
     const FilePath& new_path) {
   download_history_->UpdateDownloadPath(item, new_path);
+
+  // Begin the safe browsing download protection check.

[noelutz, 2011/10/19 16:12:49]

Should you also add '#if defined(ENABLE_SAFE_BROWSING)' here?

[Brian Ryner, 2011/10/19 18:06:59]

Good catch, done.  Will also make sure to run this on the ChromeOS trybot since
that will have SB disabled.

+  SafeBrowsingService* sb_service = g_browser_process->safe_browsing_service();
+  if (sb_service && sb_service->download_protection_service()) {
+    safe_browsing::DownloadProtectionService::DownloadInfo download_info;
+    download_info.local_file = new_path;
+    download_info.download_url_chain = item->url_chain();
+    download_info.referrer_url = item->referrer_url();
+    // TODO(bryner): Fill in the hash (we shouldn't compute it again)
+    download_info.total_bytes = item->total_bytes();
+    // TODO(bryner): Populate user_initiated
+    if (sb_service->download_protection_service()->CheckClientDownload(
+            download_info, base::Bind(
+                &ChromeDownloadManagerDelegate::CheckClientDownloadDone,
+                base::Unretained(this),

[noelutz, 2011/10/19 16:12:49]

Not sure you need base::Unretained because this is ref-counted.  Wouldn't it be
safer not to use it?  In particular, what if this goes away before the callback
is called?

[Brian Ryner, 2011/10/19 18:06:59]

I think you're right -- changed.  I'm not sure about all of the other existing
call sites in this file that use Unretained.

On 2011/10/19 16:12:49, noelutz wrote:

+                item))) {
+      CheckClientDownloadDone(item,
+                              safe_browsing::DownloadProtectionService::SAFE);
+    }  // otherwise we will be notified asynchronously
+  }
+}
+
+void ChromeDownloadManagerDelegate::CheckClientDownloadDone(
+    DownloadItem* item,
+    safe_browsing::DownloadProtectionService::DownloadCheckResult result) {
+  // TODO(bryner): notify the user based on this result
 }
 
 void ChromeDownloadManagerDelegate::RemoveItemFromPersistentStore(
     DownloadItem* item) {
   download_history_->RemoveEntry(item);
 }
 
 void ChromeDownloadManagerDelegate::RemoveItemsFromPersistentStoreBetween(
     const base::Time remove_begin,
     const base::Time remove_end) {
@@ skipping 302 matching lines @@
 // TODO(noelutz): This function currently works as a callback place holder.
 // Once we decide the hash check is reliable, we could move the
 // MaybeCompleteDownload in OnAllDataSaved to this function.
 void ChromeDownloadManagerDelegate::CheckDownloadHashDone(
     int32 download_id,
     bool is_dangerous_hash) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DVLOG(1) << "CheckDownloadHashDone, download_id: " << download_id
            << " is dangerous_hash: " << is_dangerous_hash;
 }