Send malware reports when a user opts-in from the safe browsing interstitial ...

chrome/browser/safe_browsing/safe_browsing_blocking_page.cc

Index: chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
===================================================================
--- chrome/browser/safe_browsing/safe_browsing_blocking_page.cc	(revision 66642)
+++ chrome/browser/safe_browsing/safe_browsing_blocking_page.cc	(working copy)
@@ -19,12 +19,16 @@
 #include "chrome/browser/dom_ui/new_tab_ui.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/metrics/user_metrics.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/prefs/pref_service.h"

[Scott Hess - ex-Googler, 2010/11/23 01:59:39]

'e' < 'o'.

[panayiotis, 2010/11/24 22:40:15]

Done.

 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/safe_browsing/malware_report.h"

[Scott Hess - ex-Googler, 2010/11/23 01:59:39]

'm' < 's'.

[panayiotis, 2010/11/24 22:40:15]

Done.

 #include "chrome/browser/tab_contents/navigation_controller.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/jstemplate_builder.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "grit/browser_resources.h"
 #include "grit/generated_resources.h"
@@ -101,7 +105,8 @@
                        unsafe_resources[0].url),
       sb_service_(sb_service),
       is_main_frame_(IsMainPage(unsafe_resources)),
-      unsafe_resources_(unsafe_resources) {
+      unsafe_resources_(unsafe_resources),
+      malware_report_(NULL) {
   RecordUserAction(SHOW);
   if (!is_main_frame_) {
     navigation_entry_index_to_remove_ =
@@ -109,8 +114,22 @@
   } else {
     navigation_entry_index_to_remove_ = -1;
   }
+
+  // Start computing a detailed malware report. It will be sent only
+  // if the user opts-in on the blocking page later.
+  if (unsafe_resources.size() == 1 &&
+      unsafe_resources[0].threat_type == SafeBrowsingService::URL_MALWARE &&
+      malware_report_ == NULL &&
+      CanShowMalwareReportOption()) {
+    malware_report_ = new SafeBrowsingMalwareReport(tab(), unsafe_resources[0]);
+  }
 }
 
+bool SafeBrowsingBlockingPage::CanShowMalwareReportOption() {
+  return (!tab()->profile()->IsOffTheRecord() &&
+          tab()->GetURL().SchemeIs("http"));
+}
+
 SafeBrowsingBlockingPage::~SafeBrowsingBlockingPage() {
 }
 
@@ -390,6 +409,7 @@
 
 void SafeBrowsingBlockingPage::Proceed() {
   RecordUserAction(PROCEED);
+  FinishMalwareReport();  // Send the malware report, if we opted to.
 
   NotifySafeBrowsingService(sb_service_, unsafe_resources_, true);
 
@@ -427,6 +447,7 @@
   }
 
   RecordUserAction(DONT_PROCEED);
+  FinishMalwareReport();  // Send the malware report, if we opted to.
 
   NotifySafeBrowsingService(sb_service_, unsafe_resources_, false);
 
@@ -492,6 +513,27 @@
   UserMetrics::RecordComputedAction(action);
 }
 
+void SafeBrowsingBlockingPage::FinishMalwareReport() {
+  if (malware_report_ == NULL) {
+    // Not all interstitials have reports (eg phishing).
+    return;
+  }
+
+  const PrefService::Preference* pref =
+      tab()->profile()->GetPrefs()->FindPreference(
+          prefs::kSafeBrowsingReportingEnabled);
+
+  bool value;
+  if (pref && pref->GetValue()->GetAsBoolean(&value) && value) {
+    // Give the report object to the service class, so it can send it.
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        NewRunnableMethod(
+            sb_service_, &SafeBrowsingService::SendMalwareReport,
+            malware_report_));

[Scott Hess - ex-Googler, 2010/11/23 01:59:39]

Is there any advantage to sending |malware_report_| rather than the serialized
string, here?  [I'm wondering if you could avoid scoped_refptr<> that way.]

[panayiotis, 2010/11/24 22:40:15]

As it is now, no, but as soon as the malware_report_ starts doing asynchronous
calls to eg the HistoryService, it needs to outlive the
safe_browsing_blocking_page. 

So at that point I though I'd pass it on to the service, which will wait for it
to be done and send it over.

I'm happy to do what you suggest now, and change it back in the future, if you
prefer. Please let me know.

+  }
+}
+
 // static
 void SafeBrowsingBlockingPage::NotifySafeBrowsingService(
     SafeBrowsingService* sb_service,