Send safe browsing ThreatDetails to track download CTR when user tries to recover blocked downloads

chrome/browser/download/download_danger_prompt.h

Index: chrome/browser/download/download_danger_prompt.h
diff --git a/chrome/browser/download/download_danger_prompt.h b/chrome/browser/download/download_danger_prompt.h
index e1b5c612c08994378a2e598440e948009d6aef82..52ce6e9693656f45e3e0f6edba6aa494128016c2 100644
--- a/chrome/browser/download/download_danger_prompt.h
+++ b/chrome/browser/download/download_danger_prompt.h
@@ -5,7 +5,10 @@
 #ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_DANGER_PROMPT_H_
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_DANGER_PROMPT_H_
 
+#include <string>
+
 #include "base/callback_forward.h"
+#include "url/gurl.h"
 
 namespace content {
 class DownloadItem;
@@ -37,15 +40,28 @@ class DownloadDangerPrompt {
   // caller does not own the object and receive no guarantees about lifetime.
   // If |show_context|, then the prompt message will contain some information
   // about the download and its danger; otherwise it won't.
-  static DownloadDangerPrompt* Create(
-      content::DownloadItem* item,
-      content::WebContents* web_contents,
-      bool show_context,
-      const OnDone& done);
-
-  // Only to be used by tests. Subclasses must override to manually call the
-  // respective button click handler.
-  virtual void InvokeActionForTesting(Action action) = 0;
+  static DownloadDangerPrompt* Create(content::DownloadItem* item,
+                                      content::WebContents* web_contents,
+                                      bool show_context,
+                                      const OnDone& done);
+
+  // Helper function only used by tests. This function returns a serialized
+  // ClientSafeBrowsingReportRequest proto or an empty string. Subclasses must
+  // override to manually call the respective button click handler.
+  virtual std::string InvokeActionForTesting(Action action,
+                                             const GURL& url) = 0;

[asanka, 2015/11/18 21:23:23]

Let's separate out the ClientSafeBrowsingReportRequest tests from
InvokeActionForTesting(). 

The purpose of InvokeActionForTesting() is to be an interface into the
respective implementation of DownloadDangerPrompt.

[Jialiu Lin, 2015/11/23 17:57:53]

Done. Reverted changes to this function.

+
+ protected:
+  // Return a serialized ClientSafeBrowsingReportRequest proto.
+  std::string CreateSafeBrowsingDownloadRecoveryReport(bool did_proceed,

[asanka, 2015/11/18 21:23:23]

I was going to object to putting this here, but then realized to my dismay that
DownloadDangerPromptImpl and DownloadDangerPromptViews are independent
implementations of DownloadDanterPrompt and don't share code :-(.

Anyhoo, since these two methods can't depend on any DownloadDangerPrompt fields
and can't be overridden, let's make these static. Also test them independently
of the individual DownloadDangerPrompt implementations since these methods are
independent.

[Jialiu Lin, 2015/11/23 17:57:53]

Merged CreateSafeBrowsingDownloadRecoveryReport logic into
SendSafeBrowsingDownloadRecoveryReport function, since we don't call it
separately in InvokeActionForTesting anymore. 

+                                                       const GURL& url);
+
+  // Sends download recovery report to safe browsing backend.
+  // Since it only records download url and user's action (click through or,
+  // not), it isn't gated by extended_reporting. We should not put any extra
+  // information in this report.

[asanka, 2015/11/18 21:23:23]

Can you add a note about the parameters? There are multiple URLs associated with
a download.

[Jialiu Lin, 2015/11/23 17:57:53]

Comments added and mark this function as a static one.

+  void SendSafeBrowsingDownloadRecoveryReport(bool did_proceed,
+                                              const GURL& url);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_DANGER_PROMPT_H_