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);
+

[Randy Smith (Not in Mondays), 2011/10/22 17:08:07]

Can we localize the safebrowsing code in this file to a single callout in
ShouldStartDownload (or, alternatively, here)?  I'd like to minimize how
interwoven the download code and the safebrowing code are.  In my ideal world,
the only line in this file would be a call to a safebrowing function containing
a pointer to the download item and a callback to be called when the safebrowsing
checks were done.  Can we move in that direction?

My apologies for not weighing in earlier--this is partially based on my
conversation with Noe' yesterday, which got me paying more attention to this
patch.

+#if defined(ENABLE_SAFE_BROWSING)
+  // Begin the safe browsing download protection check.
+  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,
+                this, item))) {
+      CheckClientDownloadDone(item,
+                              safe_browsing::DownloadProtectionService::SAFE);
+    }  // otherwise we will be notified asynchronously
+  }
+#else
+  // Assume safe.
+  CheckClientDownloadDone(item,
+                          safe_browsing::DownloadProtectionService::SAFE);
+#endif
+}
+
+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;
 }