The optional Malware Details also collects HTTP cache information. See design doc for background (http://goo.gl/omvhq)

BUG=None
TEST=None

Review URL: http://codereview.chromium.org/6611023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@82070 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 850 insertions, 105 deletions

diff --git a/chrome/browser/safe_browsing/malware_details.cc b/chrome/browser/safe_browsing/malware_details.cc
index 9fdd43e..cb2c318 100644
--- a/chrome/browser/safe_browsing/malware_details.cc
+++ b/chrome/browser/safe_browsing/malware_details.cc
@@ -6,14 +6,21 @@
 
 #include "chrome/browser/safe_browsing/malware_details.h"
 
+#include "base/callback.h"
 #include "base/lazy_instance.h"
-#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/net/chrome_url_request_context.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/safe_browsing/malware_details_cache.h"
 #include "chrome/browser/safe_browsing/report.pb.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/safe_browsing/safebrowsing_messages.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/renderer_host/render_view_host.h"
 #include "content/browser/tab_contents/navigation_entry.h"
 #include "content/browser/tab_contents/tab_contents.h"
+#include "net/base/io_buffer.h"
+#include "net/disk_cache/disk_cache.h"
+#include "net/url_request/url_request_context_getter.h"
 
 using safe_browsing::ClientMalwareReportRequest;
 
@@ -29,9 +36,10 @@ class MalwareDetailsFactoryImpl
     : public MalwareDetailsFactory {
  public:
   MalwareDetails* CreateMalwareDetails(
+      SafeBrowsingService* sb_service,
       TabContents* tab_contents,
       const SafeBrowsingService::UnsafeResource& unsafe_resource) {
-    return new MalwareDetails(tab_contents, unsafe_resource);
+    return new MalwareDetails(sb_service, tab_contents, unsafe_resource);
   }
 
  private:
@@ -49,25 +57,31 @@ static base::LazyInstance<MalwareDetailsFactoryImpl>
 // Create a MalwareDetails for the given tab.
 /* static */
 MalwareDetails* MalwareDetails::NewMalwareDetails(
+    SafeBrowsingService* sb_service,
     TabContents* tab_contents,
     const SafeBrowsingService::UnsafeResource& resource) {
   // Set up the factory if this has not been done already (tests do that
   // before this method is called).
   if (!factory_)
     factory_ = g_malware_details_factory_impl.Pointer();
-  return factory_->CreateMalwareDetails(tab_contents, resource);
+  return factory_->CreateMalwareDetails(sb_service, tab_contents, resource);
 }
 
 // Create a MalwareDetails for the given tab. Runs in the UI thread.
 MalwareDetails::MalwareDetails(
+    SafeBrowsingService* sb_service,
     TabContents* tab_contents,
     const SafeBrowsingService::UnsafeResource& resource)
     : TabContentsObserver(tab_contents),
-      resource_(resource) {
+      request_context_getter_(tab_contents->profile()->GetRequestContext()),
+      sb_service_(sb_service),
+      resource_(resource),
+      cache_collector_(new MalwareDetailsCacheCollector) {
   StartCollection();
 }
 
-MalwareDetails::~MalwareDetails() {}
+MalwareDetails::~MalwareDetails() {
+}
 
 bool MalwareDetails::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
@@ -88,7 +102,7 @@ bool MalwareDetails::IsPublicUrl(const GURL& url) const {
 // resources_ and updates |resource| to point to it.
 ClientMalwareReportRequest::Resource* MalwareDetails::FindOrCreateResource(
     const GURL& url) {
-  ResourceMap::iterator it = resources_.find(url.spec());
+  safe_browsing::ResourceMap::iterator it = resources_.find(url.spec());
   if (it != resources_.end()) {
     return it->second.get();
   }
@@ -207,9 +221,15 @@ void MalwareDetails::OnReceivedMalwareDOMDetails(
 void MalwareDetails::AddDOMDetails(
     const std::vector<SafeBrowsingHostMsg_MalwareDOMDetails_Node>& params) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DVLOG(1) << "Nodes from the DOM: " << params.size();
+
+  // If we have already started collecting data from the HTTP cache, don't
+  // modify our state.
+  if (cache_collector_->HasStarted())
+    return;
+
   // Add the urls from the DOM to |resources_|.  The renderer could be
   // sending bogus messages, so limit the number of nodes we accept.
-  DVLOG(1) << "Nodes from the DOM: " << params.size();
   for (uint32 i = 0; i < params.size() && i < kMaxDomNodes; ++i) {
     SafeBrowsingHostMsg_MalwareDOMDetails_Node node = params[i];
     DVLOG(1) << node.url << ", " << node.tag_name << ", " << node.parent;
@@ -222,21 +242,34 @@ void MalwareDetails::AddDOMDetails(
 // to take an action, we expect this to be called after
 // OnReceivedMalwareDOMDetails in most cases. If not, we don't include
 // the DOM data in our report.
-const std::string* MalwareDetails::GetSerializedReport() {
+void MalwareDetails::FinishCollection() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  // The |report_| protocol buffer is now generated: We add all the
-  // urls in our |resources_| maps.
-  for (ResourceMap::const_iterator it = resources_.begin();
+
+  cache_collector_->StartCacheCollection(
+      request_context_getter_,
+      &resources_,
+      &cache_result_,
+      NewRunnableMethod(this, &MalwareDetails::OnCacheCollectionReady));
+}
+
+void MalwareDetails::OnCacheCollectionReady() {
+  DVLOG(1) << "OnCacheCollectionReady.";
+  // Add all the urls in our |resources_| maps to the |report_| protocol buffer.
+  for (safe_browsing::ResourceMap::const_iterator it = resources_.begin();
        it != resources_.end(); it++) {
     ClientMalwareReportRequest::Resource* pb_resource =
         report_->add_resources();
     pb_resource->CopyFrom(*(it->second));
   }
 
-  scoped_ptr<std::string> request_data(new std::string());
-  if (!report_->SerializeToString(request_data.get())) {
+  report_->set_complete(cache_result_);
+
+  // Send the report, using the SafeBrowsingService.
+  std::string serialized;
+  if (!report_->SerializeToString(&serialized)) {
     DLOG(ERROR) << "Unable to serialize the malware report.";
+    return;
   }
 
-  return request_data.release();
+  sb_service_->SendSerializedMalwareDetails(serialized);
 }
diff --git a/chrome/browser/safe_browsing/malware_details.h b/chrome/browser/safe_browsing/malware_details.h
index 3cd6b55..494837e 100644
--- a/chrome/browser/safe_browsing/malware_details.h
+++ b/chrome/browser/safe_browsing/malware_details.h
@@ -10,29 +10,39 @@
 // users opt-in to do so from the malware warning page.
 
 // An instance of this class is generated when a malware warning page
-// is shown (SafeBrowsingBlockingPage). It is passed on to the
-// SafeBrowsing service when the warning goes away.
+// is shown (SafeBrowsingBlockingPage).
 
 #include <string>
 #include <vector>
 
 #include "base/hash_tables.h"
 #include "base/memory/linked_ptr.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/safe_browsing/report.pb.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "content/browser/tab_contents/tab_contents_observer.h"
+#include "net/base/completion_callback.h"
 
 class TabContents;
 struct SafeBrowsingHostMsg_MalwareDOMDetails_Node;
 
+class MalwareDetailsCacheCollector;
 class MalwareDetailsFactory;
 
+namespace safe_browsing {
+// Maps a URL to its Resource.
+typedef base::hash_map<
+  std::string,
+  linked_ptr<safe_browsing::ClientMalwareReportRequest::Resource> > ResourceMap;
+}
+
 class MalwareDetails : public base::RefCountedThreadSafe<MalwareDetails>,
                        public TabContentsObserver {
  public:
   // Constructs a new MalwareDetails instance, using the factory.
   static MalwareDetails* NewMalwareDetails(
+      SafeBrowsingService* sb_service,
       TabContents* tab_contents,
       const SafeBrowsingService::UnsafeResource& resource);
 
@@ -42,9 +52,13 @@ class MalwareDetails : public base::RefCountedThreadSafe<MalwareDetails>,
     factory_ = factory;
   }
 
-  // The SafeBrowsingService calls this from the IO thread, to get the
-  // serialized report as a string and send it over.
-  const std::string* GetSerializedReport();
+  // The SafeBrowsingBlockingPage calls this from the IO thread when
+  // the user is leaving the blocking page and has opted-in to sending
+  // the report. We start the cache collection, and when we are done,
+  // we send the report.
+  void FinishCollection();
+
+  void OnCacheCollectionReady();
 
   // TabContentsObserver implementation.
   virtual bool OnMessageReceived(const IPC::Message& message);
@@ -52,24 +66,25 @@ class MalwareDetails : public base::RefCountedThreadSafe<MalwareDetails>,
  protected:
   friend class MalwareDetailsFactoryImpl;
 
-  MalwareDetails(TabContents* tab_contents,
+  MalwareDetails(SafeBrowsingService* sb_service,
+                 TabContents* tab_contents,
                  const SafeBrowsingService::UnsafeResource& resource);
 
+  virtual ~MalwareDetails();
+
   // Called on the IO thread with the DOM details.
   virtual void AddDOMDetails(
       const std::vector<SafeBrowsingHostMsg_MalwareDOMDetails_Node>& params);
 
-  virtual ~MalwareDetails();
+  // The report protocol buffer.
+  scoped_ptr<safe_browsing::ClientMalwareReportRequest> report_;
+
+  // Used to get a pointer to the HTTP cache.
+  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
 
  private:
   friend class base::RefCountedThreadSafe<MalwareDetails>;
 
-  // Maps a URL to its Resource.
-  typedef base::hash_map<
-    std::string,
-    linked_ptr<safe_browsing::ClientMalwareReportRequest::Resource> >
-  ResourceMap;
-
   // Starts the collection of the report.
   void StartCollection();
 
@@ -93,21 +108,28 @@ class MalwareDetails : public base::RefCountedThreadSafe<MalwareDetails>,
   void OnReceivedMalwareDOMDetails(
       const std::vector<SafeBrowsingHostMsg_MalwareDOMDetails_Node>& params);
 
+  scoped_refptr<SafeBrowsingService> sb_service_;
+
   const SafeBrowsingService::UnsafeResource resource_;
 
   // For every Url we collect we create a Resource message. We keep
   // them in a map so we can avoid duplicates.
-  ResourceMap resources_;
+  safe_browsing::ResourceMap resources_;
 
-  // The report protocol buffer.
-  scoped_ptr<safe_browsing::ClientMalwareReportRequest> report_;
+  // Result from the cache extractor.
+  bool cache_result_;
 
   // The factory used to instanciate SafeBrowsingBlockingPage objects.
   // Usefull for tests, so they can provide their own implementation of
   // SafeBrowsingBlockingPage.
   static MalwareDetailsFactory* factory_;
 
+  // Used to collect details from the HTTP Cache.
+  scoped_refptr<MalwareDetailsCacheCollector> cache_collector_;
+
   FRIEND_TEST_ALL_PREFIXES(MalwareDetailsTest, MalwareDOMDetails);
+  FRIEND_TEST_ALL_PREFIXES(MalwareDetailsTest, HTTPCache);
+  FRIEND_TEST_ALL_PREFIXES(MalwareDetailsTest, HTTPCacheNoEntries);
 
   DISALLOW_COPY_AND_ASSIGN(MalwareDetails);
 };
@@ -118,6 +140,7 @@ class MalwareDetailsFactory {
   virtual ~MalwareDetailsFactory() { }
 
   virtual MalwareDetails* CreateMalwareDetails(
+      SafeBrowsingService* sb_service,
       TabContents* tab_contents,
       const SafeBrowsingService::UnsafeResource& unsafe_resource) = 0;
 };
diff --git a/chrome/browser/safe_browsing/malware_details_cache.cc b/chrome/browser/safe_browsing/malware_details_cache.cc
new file mode 100644
index 0000000..e41a792
--- /dev/null
+++ b/chrome/browser/safe_browsing/malware_details_cache.cc
@@ -0,0 +1,204 @@
+// 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.
+//
+// Implementation of the MalwareDetails class.
+
+#include "chrome/browser/safe_browsing/malware_details.h"
+
+#include "base/callback.h"
+#include "base/lazy_instance.h"
+#include "base/md5.h"
+#include "base/string_util.h"
+#include "chrome/browser/net/chrome_url_request_context.h"
+#include "chrome/browser/safe_browsing/malware_details_cache.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/safe_browsing/report.pb.h"
+#include "content/browser/browser_thread.h"
+#include "net/base/load_flags.h"
+#include "net/http/http_response_headers.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "net/url_request/url_request_status.h"
+
+using safe_browsing::ClientMalwareReportRequest;
+
+// Only send small files for now, a better strategy would use the size
+// of the whole report and the user's bandwidth.
+static const uint32 kMaxBodySizeBytes = 1024;
+
+MalwareDetailsCacheCollector::MalwareDetailsCacheCollector()
+    : has_started_(false),
+      current_fetch_(NULL) {
+}
+
+MalwareDetailsCacheCollector::~MalwareDetailsCacheCollector() {
+}
+
+void MalwareDetailsCacheCollector::StartCacheCollection(
+    net::URLRequestContextGetter* request_context_getter,
+    safe_browsing::ResourceMap* resources,
+    bool* result,
+    Task* callback) {
+  // Start the data collection from the HTTP cache. We use a URLFetcher
+  // and set the right flags so we only hit the cache.
+  DVLOG(1) << "Getting cache data for all urls...";
+  request_context_getter_ = request_context_getter;
+  resources_ = resources;
+  resources_it_ = resources_->begin();
+  result_ = result;
+  callback_ = callback;
+  has_started_ = true;
+
+  // Post a task in the message loop, so the callers don't need to
+  // check if we call their callback immediately.
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      NewRunnableMethod(this, &MalwareDetailsCacheCollector::OpenEntry));
+}
+
+bool MalwareDetailsCacheCollector::HasStarted() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  return has_started_;
+}
+
+// Fetch a URL and advance to the next one when done.
+void MalwareDetailsCacheCollector::OpenEntry() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DVLOG(1) << "OpenEntry";
+
+  if (resources_it_ == resources_->end()) { // We are done.
+    AllDone(true);
+    return;
+  }
+
+  if (!request_context_getter_) {
+    DVLOG(1) << "Missing request context getter";
+    AllDone(false);
+    return;
+  }
+
+  current_fetch_.reset(new URLFetcher(
+      GURL(resources_it_->first),
+      URLFetcher::GET,
+      this));
+  current_fetch_->set_request_context(request_context_getter_);
+  // Only from cache, and don't save cookies.
+  current_fetch_->set_load_flags(net::LOAD_ONLY_FROM_CACHE |
+                                 net::LOAD_DO_NOT_SAVE_COOKIES);
+  current_fetch_->set_automatically_retry_on_5xx(false);  // No retries.
+  current_fetch_->Start();  // OnURLFetchComplete will be called when done.
+}
+
+ClientMalwareReportRequest::Resource* MalwareDetailsCacheCollector::GetResource(
+    const GURL& url) {
+  safe_browsing::ResourceMap::iterator it = resources_->find(url.spec());
+  if (it != resources_->end()) {
+    return it->second.get();
+  }
+  return NULL;
+}
+
+void MalwareDetailsCacheCollector::OnURLFetchComplete(
+    const URLFetcher* source,
+    const GURL& url,
+    const net::URLRequestStatus& status,
+    int response_code,
+    const ResponseCookies& cookies,
+    const std::string& data) {
+  DVLOG(1) << "OnUrlFetchComplete";
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(current_fetch_.get());
+  if (status.status() != net::URLRequestStatus::SUCCESS &&
+      status.os_error() == net::ERR_CACHE_MISS) {
+    // Cache miss, skip this resource.
+    DVLOG(1) << "Cache miss for url: " << url;
+    AdvanceEntry();
+    return;
+  }
+
+  if (status.status() != net::URLRequestStatus::SUCCESS) {
+    // Some other error occurred, e.g. the request could have been cancelled.
+    DVLOG(1) << "Unsuccessful fetch: " << url;
+    AdvanceEntry();
+    return;
+  }
+
+  // Set the response headers and body to the right resource, which
+  // might not be the same as the one we asked for.
+  // For redirects, resources_it_->first != url.spec().
+  ClientMalwareReportRequest::Resource* resource = GetResource(url);
+  if (!resource) {
+    DVLOG(1) << "Cannot find resource for url:" << url;
+    AdvanceEntry();
+    return;
+  }
+
+  ReadResponse(resource, source);
+  ReadData(resource, data);
+  AdvanceEntry();
+}
+
+void MalwareDetailsCacheCollector::ReadResponse(
+    ClientMalwareReportRequest::Resource* pb_resource,
+    const URLFetcher* source) {
+  DVLOG(1) << "ReadResponse";
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  net::HttpResponseHeaders* headers = source->response_headers();
+  if (!headers) {
+    DVLOG(1) << "Missing response headers.";
+    return;
+  }
+
+  ClientMalwareReportRequest::HTTPResponse* pb_response =
+      pb_resource->mutable_response();
+  pb_response->mutable_firstline()->set_code(headers->response_code());
+  void* iter = NULL;
+  std::string name, value;
+  while (headers->EnumerateHeaderLines(&iter, &name, &value)) {
+    ClientMalwareReportRequest::HTTPHeader* pb_header =
+        pb_response->add_headers();
+    pb_header->set_name(name);
+    // Strip any Set-Cookie headers.
+    if (LowerCaseEqualsASCII(name, "set-cookie")) {
+      pb_header->set_value("");
+    } else {
+      pb_header->set_value(value);
+    }
+  }
+}
+
+void MalwareDetailsCacheCollector::ReadData(
+    ClientMalwareReportRequest::Resource* pb_resource,
+    const std::string& data) {
+  DVLOG(1) << "ReadData";
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  ClientMalwareReportRequest::HTTPResponse* pb_response =
+      pb_resource->mutable_response();
+  if (data.size() <= kMaxBodySizeBytes) {  // Only send small bodies for now.
+    pb_response->set_body(data);
+  }
+  pb_response->set_bodylength(data.size());
+  MD5Digest digest;
+  MD5Sum(data.c_str(), data.size(), &digest);
+  pb_response->set_bodydigest(MD5DigestToBase16(digest));
+}
+
+void MalwareDetailsCacheCollector::AdvanceEntry() {
+  DVLOG(1) << "AdvanceEntry";
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  // Advance to the next resource.
+  ++resources_it_;
+  current_fetch_.reset(NULL);
+
+  // Create a task so we don't take over the IO thread for too long.
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      NewRunnableMethod(this, &MalwareDetailsCacheCollector::OpenEntry));
+}
+
+void MalwareDetailsCacheCollector::AllDone(bool success) {
+  DVLOG(1) << "AllDone";
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  *result_ = success;
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, callback_);
+}
diff --git a/chrome/browser/safe_browsing/malware_details_cache.h b/chrome/browser/safe_browsing/malware_details_cache.h
new file mode 100644
index 0000000..3f002c7
--- /dev/null
+++ b/chrome/browser/safe_browsing/malware_details_cache.h
@@ -0,0 +1,114 @@
+// 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.
+
+#ifndef CHROME_BROWSER_SAFE_BROWSING_MALWARE_DETAILS_CACHE_H_
+#define CHROME_BROWSER_SAFE_BROWSING_MALWARE_DETAILS_CACHE_H_
+#pragma once
+
+// A class that gets malware details from the HTTP Cache.
+// An instance of this class is generated by MalwareDetails.
+
+#include <string>
+#include <vector>
+
+#include "base/hash_tables.h"
+#include "base/memory/linked_ptr.h"
+#include "base/memory/ref_counted.h"
+#include "chrome/browser/safe_browsing/report.pb.h"
+#include "chrome/common/net/url_fetcher.h"
+#include "net/base/completion_callback.h"
+
+namespace net {
+class URLRequestContext;
+}
+
+class MalwareDetailsFactory;
+
+namespace safe_browsing {
+
+// Maps a URL to its Resource.
+typedef base::hash_map<
+  std::string,
+  linked_ptr<safe_browsing::ClientMalwareReportRequest::Resource> > ResourceMap;
+}
+
+class MalwareDetailsCacheCollector
+    : public base::RefCountedThreadSafe<MalwareDetailsCacheCollector>,
+      public URLFetcher::Delegate {
+
+ public:
+  MalwareDetailsCacheCollector();
+  virtual ~MalwareDetailsCacheCollector();
+
+  // We use |request_context_getter|, we modify |resources| and
+  // |result|, and we call |callback|, so they must all remain alive
+  // for the lifetime of this object.
+  void StartCacheCollection(
+      net::URLRequestContextGetter* request_context_getter,
+      safe_browsing::ResourceMap* resources,
+      bool* result,
+      Task* callback);
+
+  // Returns whether or not StartCacheCollection has been called.
+  bool HasStarted();
+
+ protected:
+  // Implementation of URLFetcher::Delegate. Called after the request
+  // completes (either successfully or with failure).
+  virtual void OnURLFetchComplete(const URLFetcher* source,
+                                  const GURL& url,
+                                  const net::URLRequestStatus& status,
+                                  int response_code,
+                                  const ResponseCookies& cookies,
+                                  const std::string& data);
+
+ private:
+  // Points to the url for which we are fetching the HTTP cache entry or
+  // redirect chain.
+  safe_browsing::ResourceMap::iterator resources_it_;
+
+  // Points to the resources_ map in the MalwareDetails.
+  safe_browsing::ResourceMap* resources_;
+
+  // Points to the cache_result_ in the MalwareDetails.
+  bool* result_;
+
+  // Method we call when we are done. The caller must be alive for the
+  // whole time, we are modifying its state (see above).
+  Task* callback_;
+
+  // Set to true as soon as StartCacheCollection is called.
+  bool has_started_;
+
+  // Used to get a pointer to the current request context.
+  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+
+  // The current URLFetcher.
+  scoped_ptr<URLFetcher> current_fetch_;
+
+  // Returns the resource from resources_ that corresponds to |url|
+  safe_browsing::ClientMalwareReportRequest::Resource* GetResource(
+      const GURL& url);
+
+  // Creates a new URLFetcher and starts it.
+  void OpenEntry();
+
+  // Read the HTTP response from |source| and add it to |pb_resource|.
+  void ReadResponse(
+      safe_browsing::ClientMalwareReportRequest::Resource* pb_resource,
+      const URLFetcher* source);
+
+  // Read the body |data| and add it to |pb_resource|.
+  void ReadData(
+      safe_browsing::ClientMalwareReportRequest::Resource* pb_resource,
+      const std::string& data);
+
+  // Called when we are done.
+  void AllDone(bool success);
+
+  // Advances to the next entry in resources_it_.
+  void AdvanceEntry();
+};
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_MALWARE_DETAILS_CACHE_H_
diff --git a/chrome/browser/safe_browsing/malware_details_unittest.cc b/chrome/browser/safe_browsing/malware_details_unittest.cc
index bd48abd..f13469b 100644
--- a/chrome/browser/safe_browsing/malware_details_unittest.cc
+++ b/chrome/browser/safe_browsing/malware_details_unittest.cc
@@ -4,34 +4,182 @@
 
 #include <algorithm>
 
+#include "base/pickle.h"
+#include "base/time.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/malware_details.h"
 #include "chrome/browser/safe_browsing/report.pb.h"
+#include "chrome/common/render_messages.h"
 #include "chrome/common/safe_browsing/safebrowsing_messages.h"
+#include "chrome/test/test_url_request_context_getter.h"
+#include "chrome/test/testing_profile.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/renderer_host/test_render_view_host.h"
 #include "content/browser/tab_contents/navigation_entry.h"
 #include "content/browser/tab_contents/test_tab_contents.h"
+#include "net/base/io_buffer.h"
+#include "net/base/test_completion_callback.h"
+#include "net/disk_cache/disk_cache.h"
+#include "net/http/http_cache.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_response_info.h"
+#include "net/http/http_util.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.h"
 
 static const char* kOriginalLandingURL = "http://www.originallandingpage.com/";
-static const char* kLandingURL = "http://www.landingpage.com/";
-static const char* kMalwareURL = "http://www.malware.com/";
 static const char* kHttpsURL = "https://www.url.com/";
 static const char* kDOMChildURL = "http://www.domparent.com/";
 static const char* kDOMParentURL = "http://www.domchild.com/";
 static const char* kFirstRedirectURL = "http://redirectone.com/";
 static const char* kSecondRedirectURL = "http://redirecttwo.com/";
 
+static const char* kMalwareURL = "http://www.malware.com/";
+static const char* kMalwareHeaders =
+    "HTTP/1.1 200 OK\n"
+    "Content-Type: image/jpeg\n";
+static const char* kMalwareData = "exploit();";
+
+static const char* kLandingURL = "http://www.landingpage.com/";
+static const char* kLandingHeaders =
+    "HTTP/1.1 200 OK\n"
+    "Content-Type: text/html\n"
+    "Content-Length: 1024\n"
+    "Set-Cookie: tastycookie\n";  // This header is stripped.
+static const char* kLandingData = "<iframe src='http://www.malware.com'>";
+
 using safe_browsing::ClientMalwareReportRequest;
 
+namespace {
+
+void WriteHeaders(disk_cache::Entry* entry, const std::string headers) {
+  net::HttpResponseInfo responseinfo;
+  std::string raw_headers = net::HttpUtil::AssembleRawHeaders(
+      headers.c_str(), headers.size());
+  responseinfo.headers = new net::HttpResponseHeaders(raw_headers);
+
+  Pickle pickle;
+  responseinfo.Persist(&pickle, false, false);
+
+  scoped_refptr<net::WrappedIOBuffer> buf(new net::WrappedIOBuffer(
+      reinterpret_cast<const char*>(pickle.data())));
+  int len = static_cast<int>(pickle.size());
+
+  TestCompletionCallback cb;
+  int rv = entry->WriteData(0, 0, buf, len, &cb, true);
+  ASSERT_EQ(len, cb.GetResult(rv));
+}
+
+void WriteData(disk_cache::Entry* entry, const std::string data) {
+  if (data.empty())
+    return;
+
+  int len = data.length();
+  scoped_refptr<net::IOBuffer> buf(new net::IOBuffer(len));
+  memcpy(buf->data(), data.data(), data.length());
+
+  TestCompletionCallback cb;
+  int rv = entry->WriteData(1, 0, buf, len, &cb, true);
+  ASSERT_EQ(len, cb.GetResult(rv));
+}
+
+void WriteToEntry(disk_cache::Backend* cache, const std::string key,
+                  const std::string headers, const std::string data) {
+  TestCompletionCallback cb;
+  disk_cache::Entry* entry;
+  int rv = cache->CreateEntry(key, &entry, &cb);
+  rv = cb.GetResult(rv);
+  if (rv != net::OK) {
+    rv = cache->OpenEntry(key, &entry, &cb);
+    ASSERT_EQ(net::OK, cb.GetResult(rv));
+  }
+
+  WriteHeaders(entry, headers);
+  WriteData(entry, data);
+
+  entry->Close();
+}
+
+void FillCache(net::URLRequestContext* context) {
+  TestCompletionCallback cb;
+  disk_cache::Backend* cache;
+  int rv =
+      context->http_transaction_factory()->GetCache()->GetBackend(&cache, &cb);
+  ASSERT_EQ(net::OK, cb.GetResult(rv));
+
+  std::string empty;
+  WriteToEntry(cache, kMalwareURL, kMalwareHeaders, kMalwareData);
+  WriteToEntry(cache, kLandingURL, kLandingHeaders, kLandingData);
+}
+
+void QuitUIMessageLoop() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  BrowserThread::PostTask(BrowserThread::UI,
+                          FROM_HERE,
+                          new MessageLoop::QuitTask());
+}
+
+// Lets us provide a MockURLRequestContext with an HTTP Cache we pre-populate.
+// Also exposes the constructor.
+class MalwareDetailsWrap : public MalwareDetails {
+ public:
+  MalwareDetailsWrap(SafeBrowsingService* sb_service,
+                     TabContents* tab_contents,
+                     const SafeBrowsingService::UnsafeResource& unsafe_resource,
+                     net::URLRequestContextGetter* request_context_getter)
+      : MalwareDetails(sb_service, tab_contents, unsafe_resource) {
+    request_context_getter_ = request_context_getter;
+  }
+
+  virtual ~MalwareDetailsWrap() {}
+};
+
+class MockSafeBrowsingService : public SafeBrowsingService {
+ public:
+  MockSafeBrowsingService() {}
+  virtual ~MockSafeBrowsingService() {}
+
+  // When the MalwareDetails is done, this is called.
+  virtual void SendSerializedMalwareDetails(const std::string& serialized) {
+    DVLOG(1) << "SendSerializedMalwareDetails";
+    // Notify WaitForSerializedReport.
+    BrowserThread::PostTask(BrowserThread::IO,
+                            FROM_HERE,
+                            NewRunnableFunction(&QuitUIMessageLoop));
+    serialized_ = serialized;
+  }
+
+  const std::string& GetSerialized() {
+    return serialized_;
+  }
+
+ private:
+  std::string serialized_;
+  DISALLOW_COPY_AND_ASSIGN(MockSafeBrowsingService);
+};
+
+}  // namespace.
+
 class MalwareDetailsTest : public RenderViewHostTestHarness {
  public:
   MalwareDetailsTest()
-      : ui_thread_(BrowserThread::UI, MessageLoop::current()),
-        io_thread_(BrowserThread::IO, MessageLoop::current()) {
+      : ui_thread_(BrowserThread::UI, &message_loop_),
+        io_thread_(BrowserThread::IO),
+        sb_service_(new MockSafeBrowsingService()) {
   }
 
   virtual void SetUp() {
     RenderViewHostTestHarness::SetUp();
+    // request_context_getter_ = new TestURLRequestContextGetter();
+
+    // The URLFetcher checks that the messageloop type is IO.
+    ASSERT_TRUE(io_thread_.StartWithOptions(
+        base::Thread::Options(MessageLoop::TYPE_IO, 0)));
+  }
+
+  virtual void TearDown() {
+    io_thread_.Stop();
+    RenderViewHostTestHarness::TearDown();
   }
 
   static bool ResourceLessThan(
@@ -40,6 +188,18 @@ class MalwareDetailsTest : public RenderViewHostTestHarness {
     return lhs->id() < rhs->id();
   }
 
+  std::string WaitForSerializedReport(MalwareDetails* report) {
+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        NewRunnableMethod(
+            report, &MalwareDetails::FinishCollection));
+    // Wait for the callback (SendSerializedMalwareDetails).
+    DVLOG(1) << "Waiting for SendSerializedMalwareDetails";
+    MessageLoop::current()->Run();
+    return sb_service_->GetSerialized();
+  }
+
  protected:
   void InitResource(SafeBrowsingService::UnsafeResource* resource,
                     ResourceType::Type resource_type,
@@ -79,18 +239,48 @@ class MalwareDetailsTest : public RenderViewHostTestHarness {
               &MalwareDetailsTest::ResourceLessThan);
 
     for (uint32 i = 0; i < expected.size(); ++i) {
-      EXPECT_EQ(expected[i]->id(), resources[i]->id());
-      EXPECT_EQ(expected[i]->url(), resources[i]->url());
-      EXPECT_EQ(expected[i]->parent_id(), resources[i]->parent_id());
-      ASSERT_EQ(expected[i]->child_ids_size(), resources[i]->child_ids_size());
-      for (int j = 0; j < expected[i]->child_ids_size(); j++) {
-        EXPECT_EQ(expected[i]->child_ids(j), resources[i]->child_ids(j));
+      VerifyResource(resources[i], expected[i]);
+    }
+
+    EXPECT_EQ(expected_pb.complete(), report_pb.complete());
+  }
+
+  void VerifyResource(const ClientMalwareReportRequest::Resource* resource,
+                      const ClientMalwareReportRequest::Resource* expected) {
+    EXPECT_EQ(expected->id(), resource->id());
+    EXPECT_EQ(expected->url(), resource->url());
+    EXPECT_EQ(expected->parent_id(), resource->parent_id());
+    ASSERT_EQ(expected->child_ids_size(), resource->child_ids_size());
+    for (int i = 0; i < expected->child_ids_size(); i++) {
+      EXPECT_EQ(expected->child_ids(i), resource->child_ids(i));
+    }
+
+    // Verify HTTP Responses
+    if (expected->has_response()) {
+      ASSERT_TRUE(resource->has_response());
+      EXPECT_EQ(expected->response().firstline().code(),
+                resource->response().firstline().code());
+
+      ASSERT_EQ(expected->response().headers_size(),
+                resource->response().headers_size());
+      for (int i = 0; i < expected->response().headers_size(); ++i) {
+        EXPECT_EQ(expected->response().headers(i).name(),
+                  resource->response().headers(i).name());
+        EXPECT_EQ(expected->response().headers(i).value(),
+                  resource->response().headers(i).value());
       }
+
+      EXPECT_EQ(expected->response().body(), resource->response().body());
+      EXPECT_EQ(expected->response().bodylength(),
+                resource->response().bodylength());
+      EXPECT_EQ(expected->response().bodydigest(),
+                resource->response().bodydigest());
     }
   }
 
   BrowserThread ui_thread_;
   BrowserThread io_thread_;
+  scoped_refptr<MockSafeBrowsingService> sb_service_;
 };
 
 // Tests creating a simple malware report.
@@ -101,12 +291,13 @@ TEST_F(MalwareDetailsTest, MalwareSubResource) {
   SafeBrowsingService::UnsafeResource resource;
   InitResource(&resource, ResourceType::SUB_RESOURCE, GURL(kMalwareURL));
 
-  scoped_refptr<MalwareDetails> report = MalwareDetails::NewMalwareDetails(
-      contents(), resource);
+  scoped_refptr<MalwareDetailsWrap> report = new MalwareDetailsWrap(
+      sb_service_, contents(), resource, NULL);
+
+  std::string serialized = WaitForSerializedReport(report);
 
-  scoped_ptr<const std::string> serialized(report->GetSerializedReport());
   ClientMalwareReportRequest actual;
-  actual.ParseFromString(*serialized);
+  actual.ParseFromString(serialized);
 
   ClientMalwareReportRequest expected;
   expected.set_malware_url(kMalwareURL);
@@ -132,12 +323,13 @@ TEST_F(MalwareDetailsTest, MalwareSubResourceWithOriginalUrl) {
   InitResource(&resource, ResourceType::SUB_RESOURCE, GURL(kMalwareURL));
   resource.original_url = GURL(kOriginalLandingURL);
 
-  scoped_refptr<MalwareDetails> report = MalwareDetails::NewMalwareDetails(
-      contents(), resource);
+  scoped_refptr<MalwareDetailsWrap> report = new MalwareDetailsWrap(
+      sb_service_.get(), contents(), resource, NULL);
+
+  std::string serialized = WaitForSerializedReport(report);
 
-  scoped_ptr<const std::string> serialized(report->GetSerializedReport());
   ClientMalwareReportRequest actual;
-  actual.ParseFromString(*serialized);
+  actual.ParseFromString(serialized);
 
   ClientMalwareReportRequest expected;
   expected.set_malware_url(kMalwareURL);
@@ -169,8 +361,8 @@ TEST_F(MalwareDetailsTest, MalwareDOMDetails) {
   SafeBrowsingService::UnsafeResource resource;
   InitResource(&resource, ResourceType::SUB_RESOURCE, GURL(kMalwareURL));
 
-  scoped_refptr<MalwareDetails> report = MalwareDetails::NewMalwareDetails(
-      contents(), resource);
+  scoped_refptr<MalwareDetailsWrap> report = new MalwareDetailsWrap(
+      sb_service_.get(), contents(), resource, NULL);
 
   // Send a message from the DOM, with 2 nodes, a parent and a child.
   std::vector<SafeBrowsingHostMsg_MalwareDOMDetails_Node> params;
@@ -187,9 +379,9 @@ TEST_F(MalwareDetailsTest, MalwareDOMDetails) {
 
   MessageLoop::current()->RunAllPending();
 
-  scoped_ptr<const std::string> serialized(report->GetSerializedReport());
+  std::string serialized = WaitForSerializedReport(report);
   ClientMalwareReportRequest actual;
-  actual.ParseFromString(*serialized);
+  actual.ParseFromString(serialized);
 
   ClientMalwareReportRequest expected;
   expected.set_malware_url(kMalwareURL);
@@ -213,6 +405,7 @@ TEST_F(MalwareDetailsTest, MalwareDOMDetails) {
   pb_resource->set_id(3);
   pb_resource->set_url(kDOMParentURL);
   pb_resource->add_child_ids(2);
+  expected.set_complete(false);  // Since the cache was missing.
 
   VerifyResults(actual, expected);
 }
@@ -222,12 +415,12 @@ TEST_F(MalwareDetailsTest, NotPublicUrl) {
   controller().LoadURL(GURL(kHttpsURL), GURL(), PageTransition::TYPED);
   SafeBrowsingService::UnsafeResource resource;
   InitResource(&resource, ResourceType::SUB_RESOURCE, GURL(kMalwareURL));
-  scoped_refptr<MalwareDetails> report = MalwareDetails::NewMalwareDetails(
-      contents(), resource);
+  scoped_refptr<MalwareDetailsWrap> report = new MalwareDetailsWrap(
+      sb_service_.get(), contents(), resource, NULL);
 
-  scoped_ptr<const std::string> serialized(report->GetSerializedReport());
+  std::string serialized = WaitForSerializedReport(report);
   ClientMalwareReportRequest actual;
-  actual.ParseFromString(*serialized);
+  actual.ParseFromString(serialized);
 
   ClientMalwareReportRequest expected;
   expected.set_malware_url(kMalwareURL);  // No page_url
@@ -253,12 +446,12 @@ TEST_F(MalwareDetailsTest, MalwareWithRedirectUrl) {
   resource.redirect_urls.push_back(GURL(kSecondRedirectURL));
   resource.redirect_urls.push_back(GURL(kMalwareURL));
 
-  scoped_refptr<MalwareDetails> report = MalwareDetails::NewMalwareDetails(
-      contents(), resource);
+  scoped_refptr<MalwareDetailsWrap> report = new MalwareDetailsWrap(
+      sb_service_.get(), contents(), resource, NULL);
 
-  scoped_ptr<const std::string> serialized(report->GetSerializedReport());
+  std::string serialized = WaitForSerializedReport(report);
   ClientMalwareReportRequest actual;
-  actual.ParseFromString(*serialized);
+  actual.ParseFromString(serialized);
 
   ClientMalwareReportRequest expected;
   expected.set_malware_url(kMalwareURL);
@@ -290,3 +483,112 @@ TEST_F(MalwareDetailsTest, MalwareWithRedirectUrl) {
 
   VerifyResults(actual, expected);
 }
+
+// Tests the interaction with the HTTP cache.
+TEST_F(MalwareDetailsTest, HTTPCache) {
+  controller().LoadURL(GURL(kLandingURL), GURL(), PageTransition::TYPED);
+
+  SafeBrowsingService::UnsafeResource resource;
+  InitResource(&resource, ResourceType::SUB_RESOURCE, GURL(kMalwareURL));
+
+  profile()->CreateRequestContext();
+  scoped_refptr<MalwareDetailsWrap> report = new MalwareDetailsWrap(
+      sb_service_.get(), contents(), resource
+      , profile()->GetRequestContext());
+
+  FillCache(profile()->GetRequestContext()->GetURLRequestContext());
+
+  // The cache collection starts after the IPC from the DOM is fired.
+  std::vector<SafeBrowsingHostMsg_MalwareDOMDetails_Node> params;
+  report->OnReceivedMalwareDOMDetails(params);
+
+  // Let the cache callbacks complete
+  MessageLoop::current()->RunAllPending();
+
+  DVLOG(1) << "Getting serialized report";
+  std::string serialized = WaitForSerializedReport(report);
+  ClientMalwareReportRequest actual;
+  actual.ParseFromString(serialized);
+
+  ClientMalwareReportRequest expected;
+  expected.set_malware_url(kMalwareURL);
+  expected.set_page_url(kLandingURL);
+  expected.set_referrer_url("");
+
+  ClientMalwareReportRequest::Resource* pb_resource = expected.add_resources();
+  pb_resource->set_id(0);
+  pb_resource->set_url(kLandingURL);
+  safe_browsing::ClientMalwareReportRequest::HTTPResponse* pb_response =
+      pb_resource->mutable_response();
+  pb_response->mutable_firstline()->set_code(200);
+  safe_browsing::ClientMalwareReportRequest::HTTPHeader* pb_header =
+      pb_response->add_headers();
+  pb_header->set_name("Content-Type");
+  pb_header->set_value("text/html");
+  pb_header = pb_response->add_headers();
+  pb_header->set_name("Content-Length");
+  pb_header->set_value("1024");
+  pb_header = pb_response->add_headers();
+  pb_header->set_name("Set-Cookie");
+  pb_header->set_value("");  // The cookie is dropped.
+  pb_response->set_body(kLandingData);
+  pb_response->set_bodylength(37);
+  pb_response->set_bodydigest("9ca97475598a79bc1e8fc9bd6c72cd35");
+
+  pb_resource = expected.add_resources();
+  pb_resource->set_id(1);
+  pb_resource->set_url(kMalwareURL);
+  pb_response = pb_resource->mutable_response();
+  pb_response->mutable_firstline()->set_code(200);
+  pb_header = pb_response->add_headers();
+  pb_header->set_name("Content-Type");
+  pb_header->set_value("image/jpeg");
+  pb_response->set_body(kMalwareData);
+  pb_response->set_bodylength(10);
+  pb_response->set_bodydigest("581373551c43d4cf33bfb3b26838ff95");
+  expected.set_complete(true);
+
+  VerifyResults(actual, expected);
+}
+
+// Tests the interaction with the HTTP cache (where the cache is empty).
+TEST_F(MalwareDetailsTest, HTTPCacheNoEntries) {
+  controller().LoadURL(GURL(kLandingURL), GURL(), PageTransition::TYPED);
+
+  SafeBrowsingService::UnsafeResource resource;
+  InitResource(&resource, ResourceType::SUB_RESOURCE, GURL(kMalwareURL));
+
+  profile()->CreateRequestContext();
+  scoped_refptr<MalwareDetailsWrap> report = new MalwareDetailsWrap(
+      sb_service_.get(), contents(), resource,
+      profile()->GetRequestContext());
+
+  // No call to FillCache
+
+  // The cache collection starts after the IPC from the DOM is fired.
+  std::vector<SafeBrowsingHostMsg_MalwareDOMDetails_Node> params;
+  report->OnReceivedMalwareDOMDetails(params);
+
+  // Let the cache callbacks complete
+  MessageLoop::current()->RunAllPending();
+
+  DVLOG(1) << "Getting serialized report";
+  std::string serialized = WaitForSerializedReport(report);
+  ClientMalwareReportRequest actual;
+  actual.ParseFromString(serialized);
+
+  ClientMalwareReportRequest expected;
+  expected.set_malware_url(kMalwareURL);
+  expected.set_page_url(kLandingURL);
+  expected.set_referrer_url("");
+
+  ClientMalwareReportRequest::Resource* pb_resource = expected.add_resources();
+  pb_resource->set_id(0);
+  pb_resource->set_url(kLandingURL);
+  pb_resource = expected.add_resources();
+  pb_resource->set_id(1);
+  pb_resource->set_url(kMalwareURL);
+  expected.set_complete(true);
+
+  VerifyResults(actual, expected);
+}
diff --git a/chrome/browser/safe_browsing/report.proto b/chrome/browser/safe_browsing/report.proto
index 719c918..5e203a7 100644
--- a/chrome/browser/safe_browsing/report.proto
+++ b/chrome/browser/safe_browsing/report.proto
@@ -87,4 +87,7 @@ message ClientMalwareReportRequest {
 
   optional string referrer_url = 3;
   repeated Resource resources = 4;
+
+  // Whether the report has HTTP Responses.
+  optional bool complete = 5;
 }
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index 7e13311..70d1ac5 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -147,7 +147,7 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
       malware_details_ == NULL &&
       CanShowMalwareDetailsOption()) {
     malware_details_ = MalwareDetails::NewMalwareDetails(
-        tab(), unsafe_resources[0]);
+        sb_service_, tab(), unsafe_resources[0]);
   }
 }
 
@@ -605,12 +605,11 @@ void SafeBrowsingBlockingPage::FinishMalwareDetails() {
 
   bool value;
   if (pref && pref->GetValue()->GetAsBoolean(&value) && value) {
-    // Give the details object to the service class, so it can send it.
+    // Finish the malware details collection, send it over.
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         NewRunnableMethod(
-            sb_service_, &SafeBrowsingService::ReportMalwareDetails,
-            malware_details_));
+            malware_details_.get(), &MalwareDetails::FinishCollection));
   }
 }
 
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
index 3d3f5d3..5285125 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
 //
@@ -59,12 +59,14 @@ class FakeSafeBrowsingService :  public SafeBrowsingService {
     badurls[url.spec()] = checkresult;
   }
 
-  virtual void ReportMalwareDetails(scoped_refptr<MalwareDetails> details) {
-    details_.push_back(details);
-    // Notify the UI thread, that we got a report.
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            NewRunnableMethod(this,
-                            &FakeSafeBrowsingService::OnMalwareDetailsDone));
+  // Overrides SafeBrowsingService.
+  virtual void SendSerializedMalwareDetails(const std::string& serialized) {
+    reports_.push_back(serialized);
+    // Notify the UI thread that we got a report.
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        NewRunnableMethod(this,
+                          &FakeSafeBrowsingService::OnMalwareDetailsDone));
   }
 
   void OnMalwareDetailsDone() {
@@ -72,11 +74,12 @@ class FakeSafeBrowsingService :  public SafeBrowsingService {
     MessageLoopForUI::current()->Quit();
   }
 
-  std::list<scoped_refptr<MalwareDetails> >* GetDetails() {
-    return &details_;
+  std::string GetReport() {
+    EXPECT_TRUE(reports_.size() == 1);
+    return reports_[0];
   }
 
-  std::list<scoped_refptr<MalwareDetails> > details_;
+  std::vector<std::string> reports_;
 
  private:
   base::hash_map<std::string, UrlCheckResult> badurls;
@@ -96,9 +99,10 @@ class TestSafeBrowsingServiceFactory : public SafeBrowsingServiceFactory {
 // A MalwareDetails class lets us intercept calls from the renderer.
 class FakeMalwareDetails : public MalwareDetails {
  public:
-  FakeMalwareDetails(TabContents* tab_contents,
+  FakeMalwareDetails(SafeBrowsingService* sb_service,
+                     TabContents* tab_contents,
                      const SafeBrowsingService::UnsafeResource& unsafe_resource)
-      : MalwareDetails(tab_contents, unsafe_resource) { }
+      : MalwareDetails(sb_service, tab_contents, unsafe_resource) { }
 
   virtual ~FakeMalwareDetails() {}
 
@@ -136,6 +140,10 @@ class FakeMalwareDetails : public MalwareDetails {
     waiting_ = waiting;
   }
 
+  safe_browsing::ClientMalwareReportRequest* get_report() {
+    return report_.get();
+  }
+
  private:
   // Some logic to figure out if we should wait for the dom details or not.
   // These variables should only be accessed in the UI thread.
@@ -150,9 +158,11 @@ class TestMalwareDetailsFactory : public MalwareDetailsFactory {
   virtual ~TestMalwareDetailsFactory() { }
 
   virtual MalwareDetails* CreateMalwareDetails(
+      SafeBrowsingService* sb_service,
       TabContents* tab_contents,
       const SafeBrowsingService::UnsafeResource& unsafe_resource) {
-    details_ = new FakeMalwareDetails(tab_contents, unsafe_resource);
+    details_ = new FakeMalwareDetails(sb_service, tab_contents,
+                                      unsafe_resource);
     return details_;
   }
 
@@ -164,6 +174,46 @@ class TestMalwareDetailsFactory : public MalwareDetailsFactory {
   FakeMalwareDetails* details_;
 };
 
+// A SafeBrowingBlockingPage class that lets us wait until it's hidden.
+class TestSafeBrowsingBlockingPage :  public SafeBrowsingBlockingPage {
+ public:
+  TestSafeBrowsingBlockingPage(SafeBrowsingService* service,
+                               TabContents* tab_contents,
+                               const UnsafeResourceList& unsafe_resources)
+      : SafeBrowsingBlockingPage(service, tab_contents, unsafe_resources) {
+    wait_for_delete_ = false;
+  }
+
+  ~TestSafeBrowsingBlockingPage() {
+    if (wait_for_delete_) {
+      // Notify that we are gone
+      MessageLoopForUI::current()->Quit();
+    }
+  }
+
+  void set_wait_for_delete() {
+    wait_for_delete_ = true;
+  }
+
+ private:
+  bool wait_for_delete_;
+};
+
+class TestSafeBrowsingBlockingPageFactory
+    : public SafeBrowsingBlockingPageFactory {
+ public:
+  TestSafeBrowsingBlockingPageFactory() { }
+  ~TestSafeBrowsingBlockingPageFactory() { }
+
+  virtual SafeBrowsingBlockingPage* CreateSafeBrowsingPage(
+      SafeBrowsingService* service,
+      TabContents* tab_contents,
+      const SafeBrowsingBlockingPage::UnsafeResourceList& unsafe_resources) {
+    return new TestSafeBrowsingBlockingPage(service, tab_contents,
+                                            unsafe_resources);
+  }
+};
+
 // Tests the safe browsing blocking page in a browser.
 class SafeBrowsingBlockingPageTest : public InProcessBrowserTest,
                                      public SafeBrowsingService::Client {
@@ -173,12 +223,14 @@ class SafeBrowsingBlockingPageTest : public InProcessBrowserTest,
 
   virtual void SetUp() {
     SafeBrowsingService::RegisterFactory(&factory_);
+    SafeBrowsingBlockingPage::RegisterFactory(&blocking_page_factory_);
     MalwareDetails::RegisterFactory(&details_factory_);
     InProcessBrowserTest::SetUp();
   }
 
   virtual void TearDown() {
     InProcessBrowserTest::TearDown();
+    SafeBrowsingBlockingPage::RegisterFactory(NULL);
     SafeBrowsingService::RegisterFactory(NULL);
     MalwareDetails::RegisterFactory(NULL);
   }
@@ -236,12 +288,19 @@ class SafeBrowsingBlockingPageTest : public InProcessBrowserTest,
     interstitial_page->Proceed();
   }
 
-  void AssertNoInterstitial() {
-    // ui_test_utils::RunAllPendingInMessageLoop();
+  void AssertNoInterstitial(bool wait_for_delete) {
     TabContents* contents = browser()->GetSelectedTabContents();
-    InterstitialPage* interstitial_page = InterstitialPage::GetInterstitialPage(
-        contents);
-    ASSERT_FALSE(interstitial_page);
+
+    if (contents->showing_interstitial_page() && wait_for_delete) {
+      // We'll get notified when the interstitial is deleted.
+      static_cast<TestSafeBrowsingBlockingPage*>(
+          contents->interstitial_page())->set_wait_for_delete();
+      ui_test_utils::RunMessageLoop();
+    }
+
+    // Can't use InterstitialPage::GetInterstitialPage() because that
+    // gets updated after the TestSafeBrowsingBlockingPage destructor
+    ASSERT_FALSE(contents->showing_interstitial_page());
   }
 
   void WaitForNavigation() {
@@ -258,7 +317,14 @@ class SafeBrowsingBlockingPageTest : public InProcessBrowserTest,
         static_cast<FakeSafeBrowsingService*>(
             g_browser_process->resource_dispatcher_host()->
             safe_browsing_service());
-    ASSERT_EQ(1u, service->GetDetails()->size());
+
+    std::string serialized = service->GetReport();
+
+    safe_browsing::ClientMalwareReportRequest report;
+    ASSERT_TRUE(report.ParseFromString(serialized));
+
+    // Verify the report is complete.
+    EXPECT_TRUE(report.complete());
   }
 
  protected:
@@ -266,6 +332,7 @@ class SafeBrowsingBlockingPageTest : public InProcessBrowserTest,
 
  private:
   TestSafeBrowsingServiceFactory factory_;
+  TestSafeBrowsingBlockingPageFactory blocking_page_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingBlockingPageTest);
 };
@@ -283,7 +350,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest, MalwareDontProceed) {
   ui_test_utils::NavigateToURL(browser(), url);
 
   SendCommand("\"takeMeBack\"");   // Simulate the user clicking "back"
-  AssertNoInterstitial();   // Assert the interstitial is gone
+  AssertNoInterstitial(false);   // Assert the interstitial is gone
   EXPECT_EQ(GURL(chrome::kAboutBlankURL),   // Back to "about:blank"
             browser()->GetSelectedTabContents()->GetURL());
 }
@@ -296,7 +363,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest, MalwareProceed) {
 
   SendCommand("\"proceed\"");    // Simulate the user clicking "proceed"
   WaitForNavigation();    // Wait until we finish the navigation.
-  AssertNoInterstitial();    // Assert the interstitial is gone.
+  AssertNoInterstitial(true);    // Assert the interstitial is gone.
   EXPECT_EQ(url, browser()->GetSelectedTabContents()->GetURL());
 }
 
@@ -307,7 +374,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest, PhishingDontProceed) {
   ui_test_utils::NavigateToURL(browser(), url);
 
   SendCommand("\"takeMeBack\"");   // Simulate the user clicking "proceed"
-  AssertNoInterstitial();    // Assert the interstitial is gone
+  AssertNoInterstitial(false);    // Assert the interstitial is gone
   EXPECT_EQ(GURL(chrome::kAboutBlankURL),  // We are back to "about:blank".
             browser()->GetSelectedTabContents()->GetURL());
 }
@@ -320,7 +387,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest, PhishingProceed) {
 
   SendCommand("\"proceed\"");   // Simulate the user clicking "proceed".
   WaitForNavigation();    // Wait until we finish the navigation.
-  AssertNoInterstitial();    // Assert the interstitial is gone
+  AssertNoInterstitial(true);    // Assert the interstitial is gone
   EXPECT_EQ(url, browser()->GetSelectedTabContents()->GetURL());
 }
 
@@ -332,7 +399,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest, PhishingReportError) {
 
   SendCommand("\"reportError\"");   // Simulate the user clicking "report error"
   WaitForNavigation();    // Wait until we finish the navigation.
-  AssertNoInterstitial();    // Assert the interstitial is gone
+  AssertNoInterstitial(false);    // Assert the interstitial is gone
 
   // We are in the error reporting page.
   EXPECT_EQ("/safebrowsing/report_error/",
@@ -347,7 +414,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest, PhishingLearnMore) {
 
   SendCommand("\"learnMore\"");   // Simulate the user clicking "learn more"
   WaitForNavigation();    // Wait until we finish the navigation.
-  AssertNoInterstitial();    // Assert the interstitial is gone
+  AssertNoInterstitial(false);    // Assert the interstitial is gone
 
   // We are in the help page.
   EXPECT_EQ("/support/bin/answer.py",
@@ -362,7 +429,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest, MalwareIframeDontProceed) {
   ui_test_utils::NavigateToURL(browser(), url);
 
   SendCommand("\"takeMeBack\"");    // Simulate the user clicking "back"
-  AssertNoInterstitial();  // Assert the interstitial is gone
+  AssertNoInterstitial(false);  // Assert the interstitial is gone
 
   EXPECT_EQ(GURL(chrome::kAboutBlankURL),    // Back to "about:blank"
             browser()->GetSelectedTabContents()->GetURL());
@@ -378,7 +445,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest,
   ui_test_utils::NavigateToURL(browser(), url);
 
   SendCommand("\"proceed\"");   // Simulate the user clicking "proceed"
-  AssertNoInterstitial();    // Assert the interstitial is gone
+  AssertNoInterstitial(true);    // Assert the interstitial is gone
 
   EXPECT_EQ(url, browser()->GetSelectedTabContents()->GetURL());
 }
@@ -407,7 +474,7 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest,
       prefs::kSafeBrowsingReportingEnabled));
 
   SendCommand("\"proceed\"");  // Simulate the user clicking "back"
-  AssertNoInterstitial();  // Assert the interstitial is gone
+  AssertNoInterstitial(true);  // Assert the interstitial is gone
 
   EXPECT_EQ(url, browser()->GetSelectedTabContents()->GetURL());
   AssertReportSent();
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
index 1bf5ddf..12399ce 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
@@ -38,15 +38,16 @@ class TestSafeBrowsingBlockingPage :  public SafeBrowsingBlockingPage {
 class TestSafeBrowsingService: public SafeBrowsingService {
  public:
   virtual ~TestSafeBrowsingService() {}
-  virtual void ReportMalwareDetails(scoped_refptr<MalwareDetails> details) {
-    details_.push_back(details);
+
+  virtual void SendSerializedMalwareDetails(const std::string& serialized) {
+    details_.push_back(serialized);
   }
 
-  std::list<scoped_refptr<MalwareDetails> >* GetDetails() {
+  std::list<std::string>* GetDetails() {
     return &details_;
   }
 
-  std::list<scoped_refptr<MalwareDetails> > details_;
+  std::list<std::string> details_;
 };
 
 class TestSafeBrowsingBlockingPageFactory
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index e070cb6..71208c6 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -1066,14 +1066,13 @@ void SafeBrowsingService::ReportSafeBrowsingHitOnIOThread(
 }
 
 // If the user had opted-in to send MalwareDetails, this gets called
-// at the time that the blocking page is going away.
-void SafeBrowsingService::ReportMalwareDetails(
-    scoped_refptr<MalwareDetails> details) {
+// when the report is ready.
+void SafeBrowsingService::SendSerializedMalwareDetails(
+    const std::string& serialized) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  scoped_ptr<const std::string> serialized(details->GetSerializedReport());
-  if (!serialized->empty()) {
+  if (!serialized.empty()) {
     DVLOG(1) << "Sending serialized malware details.";
-    protocol_manager_->ReportMalwareDetails(*serialized);
+    protocol_manager_->ReportMalwareDetails(serialized);
   }
 }
 
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index 1fbac1c..5e763a8 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -239,9 +239,9 @@ class SafeBrowsingService
   // the current page is 'safe'.
   void LogPauseDelay(base::TimeDelta time);
 
-  // When a safebrowsing blocking page goes away, it calls this method
-  // so the service can serialize and send MalwareDetails.
-  virtual void ReportMalwareDetails(scoped_refptr<MalwareDetails> details);
+  // Called on the IO thread by the MalwareDetails with the serialized
+  // protocol buffer, so the service can send it over.
+  virtual void SendSerializedMalwareDetails(const std::string& serialized);
 
   // Report hits to the unsafe contents (malware, phishing, unsafe download URL)
   // to the server. Can only be called on UI thread.