Add checkbox for reporting invalid TLS/SSL cert chains

chrome/browser/interstitials/security_interstitial_page.h

Index: chrome/browser/interstitials/security_interstitial_page.h
diff --git a/chrome/browser/interstitials/security_interstitial_page.h b/chrome/browser/interstitials/security_interstitial_page.h
index ce3eb438736b46ddb45d4277dd0da8b3622085d7..4bd2054d752d6170e8ed51b8ba58217adfcc7edd 100644
--- a/chrome/browser/interstitials/security_interstitial_page.h
+++ b/chrome/browser/interstitials/security_interstitial_page.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_PAGE_H_
 #define CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_PAGE_H_
 
+#include "base/callback.h"
 #include "base/strings/string16.h"
 #include "content/public/browser/interstitial_page_delegate.h"
 #include "url/gurl.h"
@@ -13,11 +14,23 @@ namespace base {
 class DictionaryValue;
 }
 
+namespace chrome_browser_net {
+class CertificateErrorReporter;
+}
+
 namespace content {
 class InterstitialPage;
 class WebContents;
 }
 
+namespace interstitials {
+// Constants used to communicate with the JavaScript.
+extern const char kBoxChecked[];
+extern const char kDisplayCheckBox[];
+extern const char kOptInLink[];
+extern const char kPrivacyLinkHtml[];
+}
+
 class SecurityInterstitialPage : public content::InterstitialPageDelegate {
  public:
   SecurityInterstitialPage(content::WebContents* web_contents,
@@ -30,6 +43,14 @@ class SecurityInterstitialPage : public content::InterstitialPageDelegate {
   // Prevents creating the actual interstitial view for testing.
   void DontCreateViewForTesting();
 
+  // Allows tests to be notified when an invalid cert chain report has
+  // been sent (or not sent).
+  void SetCertificateReportCallbackForTesting(const base::Closure& callback);
+  // Useful for tests to inject a mock reporter.

[Ryan Sleevi, 2015/03/14 03:09:42]

add newline between 48/49

The comment style between 46/47 and 49 is somewhat inconsistent. It'd be nice to
harmonize.

Do you really need both methods? It seems like either one or the other is
appropriate. That is, setting the reporter implicitly acts as notification.

[estark, 2015/03/16 23:40:52]

No longer applicable since the reporter now lives on ProfileIOData.

 
I'm not 100% sure what you mean by "setting the reporter implicitly acts as
notification" so this might be a non-sequitur: part of the purpose of the
callback is so that the SSLBlockingPage can notify the test when a report has
*not* been sent (e.g. in incognito mode). Setting the reporter isn't sufficient
to get that type of notification.

+  void SetCertificateReporterForTesting(
+      scoped_refptr<chrome_browser_net::CertificateErrorReporter>

[Ryan Sleevi, 2015/03/14 03:09:42]

always pass scoped_refptr's const and by-ref.

Put differently, the only time you get to use by-value semantics are
scoped_ptr<>. All other class and struct types are passed by const-ref,
including container objects like WeakPtr.

(Note: This is moot w/ my remarks about *not* making this object ref counted)

[estark, 2015/03/16 23:40:52]

no longer applicable but noted for future

+          certificate_reporter_for_testing);

[Ryan Sleevi, 2015/03/14 03:09:42]

Is this method vestigal? Didn't see it in the .cc

[estark, 2015/03/16 23:40:52]

Done.

+
  protected:
   // Returns true if the interstitial should create a new navigation entry.
   virtual bool ShouldCreateNewNavigation() const = 0;
@@ -48,6 +69,24 @@ class SecurityInterstitialPage : public content::InterstitialPageDelegate {
   content::WebContents* web_contents() const;
   GURL request_url() const;
 
+  // Record the user's preference for reporting information about
+  // malware and SSL errors.
+  void SetReportingPreference(bool report);
+
+  // Fills the passed dictionary with the values to be passed to the template
+  // when creating the HTML.
+  virtual void PopulateExtendedReportingOption(
+      base::DictionaryValue* load_time_data) = 0;
+
+  // Returns the boolean value of the given |pref| from the PrefService of the
+  // Profile associated with |web_contents_|.
+  bool IsPrefEnabled(const char* pref);
+
+  // This callback is run when an extended reporting certificate chain
+  // report has been sent, or when it is decided that it should not be
+  // sent (for example, when in incognito mode).
+  base::Closure certificate_report_callback_for_testing_;
+
  private:
   content::WebContents* web_contents_;
   const GURL request_url_;