Have SSLErrorHandler decide which type of interstitial to display

chrome/browser/ssl/bad_clock_blocking_page.cc

Index: chrome/browser/ssl/bad_clock_blocking_page.cc
diff --git a/chrome/browser/ssl/bad_clock_blocking_page.cc b/chrome/browser/ssl/bad_clock_blocking_page.cc
index 6d7075398810e6ad4e007815400da910a97c5aeb..5eb23d06f392abb98c432f1f2cfdd74913599dc4 100644
--- a/chrome/browser/ssl/bad_clock_blocking_page.cc
+++ b/chrome/browser/ssl/bad_clock_blocking_page.cc
@@ -20,6 +20,7 @@
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/interstitials/chrome_metrics_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_preferences_util.h"
 #include "chrome/browser/ssl/ssl_error_classification.h"
@@ -70,6 +71,8 @@ using content::NavigationEntry;
 
 namespace {
 
+const char kMetricsName[] = "bad_clock";
+
 void LaunchDateAndTimeSettings() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);
 // The code for each OS is completely separate, in order to avoid bugs like
@@ -177,7 +180,14 @@ BadClockBlockingPage::BadClockBlockingPage(
       callback_(callback),
       cert_error_(cert_error),
       ssl_info_(ssl_info),
-      time_triggered_(time_triggered) {}
+      time_triggered_(time_triggered) {
+  security_interstitials::MetricsHelper::ReportDetails reporting_info;
+  reporting_info.metric_prefix = kMetricsName;
+  set_metrics_helper(new ChromeMetricsHelper(web_contents, request_url,
+                                             reporting_info, kMetricsName));
+  metrics_helper()->RecordUserInteraction(

[estark, 2015/09/01 13:28:28]

In SSLBlockingPage we also record security_interstitials::MetricsHelper::SHOW
here. Is there a reason we don't do that here?

This also doesn't call SSLErrorClassification::RecordUMAStatistics() which would
have been used to be called for this interstitial from the SSLBlockingPage
constructor. Is that intentional?

[felt, 2015/09/01 15:49:41]

Yes. That's for the decision UMA metric which doesn't really make sense for the
bad ssl warning, where you can't proceed (meaning there is no decision to make).
SSLErrorClassification should probably be updated to keep separate stats for
different types of interstitials. With that being said, perhaps I should keep
the recording here and then make a change to how SSLErrorClassification works in
a follow-up CL. (Done that, and added a TODO.)

+      security_interstitials::MetricsHelper::TOTAL_VISITS);
+}
 
 bool BadClockBlockingPage::ShouldCreateNewNavigation() const {
   return true;
@@ -294,8 +304,12 @@ void BadClockBlockingPage::CommandReceived(const std::string& command) {
       SetReportingPreference(false);
       break;
     case CMD_SHOW_MORE_SECTION:
+      metrics_helper()->RecordUserInteraction(
+          security_interstitials::MetricsHelper::SHOW_ADVANCED);
       break;
     case CMD_OPEN_DATE_SETTINGS:
+      metrics_helper()->RecordUserInteraction(
+          security_interstitials::MetricsHelper::OPEN_TIME_SETTINGS);
       content::BrowserThread::PostTask(content::BrowserThread::FILE, FROM_HERE,
                                        base::Bind(&LaunchDateAndTimeSettings));
       break;