Remove redundancy in security interstitial UMA logic

chrome/browser/ssl/ssl_blocking_page.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/ssl/ssl_blocking_page.h"
 
 #include "base/build_time.h"
 #include "base/command_line.h"
 #include "base/i18n/rtl.h"
 #include "base/i18n/time_formatting.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/process/launch.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_preferences_util.h"
 #include "chrome/browser/ssl/ssl_error_classification.h"
 #include "chrome/browser/ssl/ssl_error_info.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/google/core/browser/google_util.h"
 #include "content/public/browser/cert_store.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/renderer_preferences.h"
 #include "content/public/common/ssl_status.h"
 #include "grit/browser_resources.h"
 #include "net/base/hash_value.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "ui/base/l10n/l10n_util.h"
 
-#if defined(ENABLE_EXTENSIONS)
-#include "chrome/browser/extensions/api/experience_sampling_private/experience_sampling.h"
-#endif
-
 #if defined(OS_WIN)
 #include "base/base_paths_win.h"
 #include "base/path_service.h"
 #include "base/strings/string16.h"
 #include "base/win/windows_version.h"
 #endif
 
 #if defined(OS_ANDROID)
 #include "chrome/browser/android/intent_helper.h"
 #endif
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/common/url_constants.h"
 #endif
 
 using base::ASCIIToUTF16;
 using base::TimeTicks;
 using content::InterstitialPage;
 using content::NavigationController;
 using content::NavigationEntry;
 
-#if defined(ENABLE_EXTENSIONS)
-using extensions::ExperienceSamplingEvent;
-#endif
-
 namespace {
 
 // URL for help page.
 const char kHelpURL[] = "https://support.google.com/chrome/answer/4454607";
 
 // Constants for the Experience Sampling instrumentation.
 #if defined(ENABLE_EXTENSIONS)
 const char kEventNameBase[] = "ssl_interstitial_";
 const char kEventNotOverridable[] = "notoverridable_";
 const char kEventOverridable[] = "overridable_";
 #endif
 
 // Events for UMA. Do not reorder or change!
-enum SSLBlockingPageEvent {
-  SHOW_ALL,
-  SHOW_OVERRIDABLE,
-  PROCEED_OVERRIDABLE,
-  PROCEED_NAME,
-  PROCEED_DATE,
-  PROCEED_AUTHORITY,
-  DONT_PROCEED_OVERRIDABLE,
-  DONT_PROCEED_NAME,
-  DONT_PROCEED_DATE,
-  DONT_PROCEED_AUTHORITY,
-  MORE,
-  SHOW_UNDERSTAND,  // Used by the summer 2013 Finch trial. Deprecated.
-  SHOW_INTERNAL_HOSTNAME,
-  PROCEED_INTERNAL_HOSTNAME,
-  SHOW_NEW_SITE,
-  PROCEED_NEW_SITE,
-  PROCEED_MANUAL_NONOVERRIDABLE,
-  // Captive Portal errors moved to ssl_error_classification.
-  DEPRECATED_CAPTIVE_PORTAL_DETECTION_ENABLED,
-  DEPRECATED_CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE,
-  DEPRECATED_CAPTIVE_PORTAL_PROBE_COMPLETED,
-  DEPRECATED_CAPTIVE_PORTAL_PROBE_COMPLETED_OVERRIDABLE,
-  DEPRECATED_CAPTIVE_PORTAL_NO_RESPONSE,
-  DEPRECATED_CAPTIVE_PORTAL_NO_RESPONSE_OVERRIDABLE,
-  DEPRECATED_CAPTIVE_PORTAL_DETECTED,
-  DEPRECATED_CAPTIVE_PORTAL_DETECTED_OVERRIDABLE,
-  DISPLAYED_CLOCK_INTERSTITIAL,
-  UNUSED_BLOCKING_PAGE_EVENT,
-};
-
-// Events for UMA. Do not reorder or change!
 enum SSLExpirationAndDecision {
   EXPIRED_AND_PROCEED,
   EXPIRED_AND_DO_NOT_PROCEED,
   NOT_EXPIRED_AND_PROCEED,
   NOT_EXPIRED_AND_DO_NOT_PROCEED,
   END_OF_SSL_EXPIRATION_AND_DECISION,
 };
 
-void RecordSSLBlockingPageEventStats(SSLBlockingPageEvent event) {
-  UMA_HISTOGRAM_ENUMERATION("interstitial.ssl",
-                            event,
-                            UNUSED_BLOCKING_PAGE_EVENT);
-}
-
 void RecordSSLExpirationPageEventState(bool expired_but_previously_allowed,
                                        bool proceed,
                                        bool overridable) {
   SSLExpirationAndDecision event;
   if (expired_but_previously_allowed && proceed)
     event = EXPIRED_AND_PROCEED;
   else if (expired_but_previously_allowed && !proceed)
     event = EXPIRED_AND_DO_NOT_PROCEED;
   else if (!expired_but_previously_allowed && proceed)
     event = NOT_EXPIRED_AND_PROCEED;
   else
     event = NOT_EXPIRED_AND_DO_NOT_PROCEED;
 
   if (overridable) {
     UMA_HISTOGRAM_ENUMERATION(
         "interstitial.ssl.expiration_and_decision.overridable",
         event,
         END_OF_SSL_EXPIRATION_AND_DECISION);
   } else {
     UMA_HISTOGRAM_ENUMERATION(
         "interstitial.ssl.expiration_and_decision.nonoverridable",
         event,
         END_OF_SSL_EXPIRATION_AND_DECISION);
   }
 }
 
-void RecordSSLBlockingPageDetailedStats(bool proceed,
-                                        int cert_error,
-                                        bool overridable,
-                                        bool internal,
-                                        int num_visits,
-                                        bool expired_but_previously_allowed) {
-  UMA_HISTOGRAM_ENUMERATION("interstitial.ssl_error_type",

[mattm, 2015/01/14 00:33:04]

does this histogram need to be marked obsolete?

[felt, 2015/01/14 00:55:18]

I'm moving it into SSLErrorClassification::RecordUMAStatistics. I thought maybe
I'd do that in a follow-up CL, but I've added it in here to avoid confusion.

-      SSLErrorInfo::NetErrorToErrorType(cert_error), SSLErrorInfo::END_OF_ENUM);
-  RecordSSLExpirationPageEventState(
-      expired_but_previously_allowed, proceed, overridable);
-  if (!overridable) {
-    if (proceed) {
-      RecordSSLBlockingPageEventStats(PROCEED_MANUAL_NONOVERRIDABLE);
-    }
-    // Overridable is false if the user didn't have any option except to turn
-    // back. If that's the case, don't record some of the metrics.
-    return;
-  }
-  if (num_visits == 0)
-    RecordSSLBlockingPageEventStats(SHOW_NEW_SITE);
-  if (proceed) {
-    RecordSSLBlockingPageEventStats(PROCEED_OVERRIDABLE);
-    if (internal)
-      RecordSSLBlockingPageEventStats(PROCEED_INTERNAL_HOSTNAME);
-    if (num_visits == 0)
-      RecordSSLBlockingPageEventStats(PROCEED_NEW_SITE);
-  } else if (!proceed) {
-    RecordSSLBlockingPageEventStats(DONT_PROCEED_OVERRIDABLE);
-  }
-  SSLErrorInfo::ErrorType type = SSLErrorInfo::NetErrorToErrorType(cert_error);
-  switch (type) {
-    case SSLErrorInfo::CERT_COMMON_NAME_INVALID: {
-      if (proceed)
-        RecordSSLBlockingPageEventStats(PROCEED_NAME);
-      else
-        RecordSSLBlockingPageEventStats(DONT_PROCEED_NAME);
-      break;
-    }
-    case SSLErrorInfo::CERT_DATE_INVALID: {
-      if (proceed)
-        RecordSSLBlockingPageEventStats(PROCEED_DATE);
-      else
-        RecordSSLBlockingPageEventStats(DONT_PROCEED_DATE);
-      break;
-    }
-    case SSLErrorInfo::CERT_AUTHORITY_INVALID: {
-      if (proceed)
-        RecordSSLBlockingPageEventStats(PROCEED_AUTHORITY);
-      else
-        RecordSSLBlockingPageEventStats(DONT_PROCEED_AUTHORITY);
-      break;
-    }
-    default: {
-      break;
-    }
-  }
-}
-
 void LaunchDateAndTimeSettings() {
   // The code for each OS is completely separate, in order to avoid bugs like
   // https://crbug.com/430877 .
 #if defined(OS_ANDROID)
   chrome::android::OpenDateAndTimeSettings();
 
 #elif defined(OS_CHROMEOS)
   std::string sub_page = std::string(chrome::kSearchSubPage) + "#" +
       l10n_util::GetStringUTF8(IDS_OPTIONS_SETTINGS_SECTION_TITLE_DATETIME);
   chrome::ShowSettingsSubPageForProfile(
@@ skipping 94 matching lines @@
                                  const GURL& request_url,
                                  int options_mask,
                                  const base::Callback<void(bool)>& callback)
     : SecurityInterstitialPage(web_contents, request_url),
       callback_(callback),
       cert_error_(cert_error),
       ssl_info_(ssl_info),
       overridable_(IsOptionsOverridable(options_mask)),
       danger_overridable_(true),
       strict_enforcement_((options_mask & STRICT_ENFORCEMENT) != 0),
-      internal_(false),
-      num_visits_(-1),
       expired_but_previously_allowed_(
           (options_mask & EXPIRED_BUT_PREVIOUSLY_ALLOWED) != 0) {
-  Profile* profile = Profile::FromBrowserContext(
-      web_contents->GetBrowserContext());
-  // For UMA stats.
-  if (SSLErrorClassification::IsHostnameNonUniqueOrDotless(
-          request_url.HostNoBrackets()))
-    internal_ = true;
-  RecordSSLBlockingPageEventStats(SHOW_ALL);
-  if (overridable_) {
-    RecordSSLBlockingPageEventStats(SHOW_OVERRIDABLE);
-    if (internal_)
-      RecordSSLBlockingPageEventStats(SHOW_INTERNAL_HOSTNAME);
-    HistoryService* history_service = HistoryServiceFactory::GetForProfile(
-        profile, ServiceAccessType::EXPLICIT_ACCESS);
-    if (history_service) {
-      history_service->GetVisibleVisitCountToHost(
-          request_url,
-          base::Bind(&SSLBlockingPage::OnGotHistoryCount,
-                     base::Unretained(this)),
-          &request_tracker_);
-    }
-  }
+  interstitial_reason_ =
+      IsErrorDueToBadClock(base::Time::NowFromSystemTime(), cert_error_) ?
+      SSL_REASON_BAD_CLOCK : SSL_REASON_SSL;
+
+  uma_helper_.reset(new SecurityInterstitialUmaHelper(
+      web_contents, request_url, GetHistogramPrefix(), GetSamplingEventName()));
+  uma_helper_->RecordUserDecision(SecurityInterstitialUmaHelper::SHOW);
+  uma_helper_->RecordUserInteraction(
+      SecurityInterstitialUmaHelper::TOTAL_VISITS);
 
   ssl_error_classification_.reset(new SSLErrorClassification(
       web_contents,
       base::Time::NowFromSystemTime(),
       request_url,
       cert_error_,
       *ssl_info_.cert.get()));
   ssl_error_classification_->RecordUMAStatistics(overridable_);
-
 #if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
   ssl_error_classification_->RecordCaptivePortalUMAStatistics(overridable_);
 #endif
 
-#if defined(ENABLE_EXTENSIONS)
-  // ExperienceSampling: Set up new sampling event for this interstitial.
-  std::string event_name(kEventNameBase);
-  if (overridable_ && !strict_enforcement_)
-    event_name.append(kEventOverridable);
-  else
-    event_name.append(kEventNotOverridable);
-  event_name.append(net::ErrorToString(cert_error_));
-  sampling_event_.reset(new ExperienceSamplingEvent(
-      event_name,
-      request_url,
-      web_contents->GetLastCommittedURL(),
-      web_contents->GetBrowserContext()));
-#endif
-
   // Creating an interstitial without showing (e.g. from chrome://interstitials)
   // it leaks memory, so don't create it here.
 }
 
 bool SSLBlockingPage::ShouldCreateNewNavigation() const {
   return true;
 }
 
 const void* SSLBlockingPage::GetTypeForTesting() const {
   return SSLBlockingPage::kTypeForTesting;
@@ skipping 12 matching lines @@
     case SSLErrorInfo::CERT_COMMON_NAME_INVALID:
       ssl_error_classification_->InvalidCommonNameSeverityScore();
       break;
     case SSLErrorInfo::CERT_AUTHORITY_INVALID:
       ssl_error_classification_->InvalidAuthoritySeverityScore();
       break;
     default:
       break;
   }
   if (!callback_.is_null()) {
-    RecordSSLBlockingPageDetailedStats(false,
-                                       cert_error_,
-                                       overridable_,
-                                       internal_,
-                                       num_visits_,
-                                       expired_but_previously_allowed_);
     // The page is closed without the user having chosen what to do, default to
     // deny.
+    uma_helper_->RecordUserDecision(
+        SecurityInterstitialUmaHelper::DONT_PROCEED);
+    RecordSSLExpirationPageEventState(
+        expired_but_previously_allowed_, false, overridable_);
     NotifyDenyCertificate();
   }
 }
 
 void SSLBlockingPage::PopulateInterstitialStrings(
     base::DictionaryValue* load_time_data) {
   CHECK(load_time_data);
   base::string16 url(GetFormattedHostName());
   // Shared values for both the overridable and non-overridable versions.
   load_time_data->SetString("type", "SSL");
 
   // Shared UI configuration for all SSL interstitials.
   base::Time now = base::Time::NowFromSystemTime();
-  bool bad_clock = IsErrorDueToBadClock(now, cert_error_);
-
   load_time_data->SetString("errorCode", net::ErrorToString(cert_error_));
   load_time_data->SetString(
       "openDetails",
       l10n_util::GetStringUTF16(IDS_SSL_V2_OPEN_DETAILS_BUTTON));
   load_time_data->SetString(
       "closeDetails",
       l10n_util::GetStringUTF16(IDS_SSL_V2_CLOSE_DETAILS_BUTTON));
 
   // Conditional UI configuration.
-  if (bad_clock) {
-    RecordSSLBlockingPageEventStats(DISPLAYED_CLOCK_INTERSTITIAL);
-
+  if (interstitial_reason_ == SSL_REASON_BAD_CLOCK) {
     load_time_data->SetBoolean("bad_clock", true);
     load_time_data->SetBoolean("overridable", false);
 
 #if defined(OS_IOS)
     load_time_data->SetBoolean("hide_primary_button", true);
 #else
     load_time_data->SetBoolean("hide_primary_button", false);
 #endif
 
     // We're showing the SSL clock warning to be helpful, but we haven't warned
@@ skipping 137 matching lines @@
       interstitial_page()->DontProceed();
       break;
     }
     case CMD_PROCEED: {
       if (danger_overridable_) {
         interstitial_page()->Proceed();
       }
       break;
     }
     case CMD_MORE: {
-      RecordSSLBlockingPageEventStats(MORE);
-#if defined(ENABLE_EXTENSIONS)
-      if (sampling_event_.get())
-        sampling_event_->set_has_viewed_details(true);
-#endif
+      uma_helper_->RecordUserInteraction(
+          SecurityInterstitialUmaHelper::SHOW_ADVANCED);
       break;
     }
     case CMD_RELOAD: {
+      uma_helper_->RecordUserInteraction(
+          SecurityInterstitialUmaHelper::RELOAD);
       // The interstitial can't refresh itself.
       web_contents()->GetController().Reload(true);
       break;
     }
     case CMD_HELP: {
+      uma_helper_->RecordUserInteraction(
+          SecurityInterstitialUmaHelper::SHOW_LEARN_MORE);
       content::NavigationController::LoadURLParams help_page_params(
           google_util::AppendGoogleLocaleParam(
               GURL(kHelpURL), g_browser_process->GetApplicationLocale()));
-#if defined(ENABLE_EXTENSIONS)
-      if (sampling_event_.get())
-        sampling_event_->set_has_viewed_learn_more(true);
-#endif
       web_contents()->GetController().LoadURLWithParams(help_page_params);
       break;
     }
     case CMD_CLOCK: {
+      uma_helper_->RecordUserInteraction(
+          SecurityInterstitialUmaHelper::OPEN_TIME_SETTINGS);
       LaunchDateAndTimeSettings();
       break;
     }
     default: {
       NOTREACHED();
     }
   }
 }
 
 void SSLBlockingPage::OverrideRendererPrefs(
       content::RendererPreferences* prefs) {
   Profile* profile = Profile::FromBrowserContext(
       web_contents()->GetBrowserContext());
   renderer_preferences_util::UpdateFromSystemSettings(
       prefs, profile, web_contents());
 }
 
 void SSLBlockingPage::OnProceed() {
-  RecordSSLBlockingPageDetailedStats(true,
-                                     cert_error_,
-                                     overridable_,
-                                     internal_,
-                                     num_visits_,
-                                     expired_but_previously_allowed_);
-#if defined(ENABLE_EXTENSIONS)
-  // ExperienceSampling: Notify that user decided to proceed.
-  if (sampling_event_.get())
-    sampling_event_->CreateUserDecisionEvent(ExperienceSamplingEvent::kProceed);
-#endif
-
+  uma_helper_->RecordUserDecision(SecurityInterstitialUmaHelper::PROCEED);
+  RecordSSLExpirationPageEventState(
+      expired_but_previously_allowed_, true, overridable_);
   // Accepting the certificate resumes the loading of the page.
   NotifyAllowCertificate();
 }
 
 void SSLBlockingPage::OnDontProceed() {
-  RecordSSLBlockingPageDetailedStats(false,
-                                     cert_error_,
-                                     overridable_,
-                                     internal_,
-                                     num_visits_,
-                                     expired_but_previously_allowed_);
-#if defined(ENABLE_EXTENSIONS)
-  // ExperienceSampling: Notify that user decided to not proceed.
-  // This also occurs if the user navigates away or closes the tab.
-  if (sampling_event_.get())
-    sampling_event_->CreateUserDecisionEvent(ExperienceSamplingEvent::kDeny);
-#endif
+  uma_helper_->RecordUserDecision(SecurityInterstitialUmaHelper::DONT_PROCEED);
+  RecordSSLExpirationPageEventState(
+      expired_but_previously_allowed_, false, overridable_);
   NotifyDenyCertificate();
 }
 
 void SSLBlockingPage::NotifyDenyCertificate() {
   // It's possible that callback_ may not exist if the user clicks "Proceed"
   // followed by pressing the back button before the interstitial is hidden.
   // In that case the certificate will still be treated as allowed.
   if (callback_.is_null())
     return;
 
   callback_.Run(false);
   callback_.Reset();
 }
 
 void SSLBlockingPage::NotifyAllowCertificate() {
   DCHECK(!callback_.is_null());
 
   callback_.Run(true);
   callback_.Reset();
 }
 
-// static
-void SSLBlockingPage::SetExtraInfo(
-    base::DictionaryValue* strings,
-    const std::vector<base::string16>& extra_info) {
-  DCHECK_LT(extra_info.size(), 5U);  // We allow 5 paragraphs max.
-  const char* keys[5] = {
-      "moreInfo1", "moreInfo2", "moreInfo3", "moreInfo4", "moreInfo5"
-  };
-  int i;
-  for (i = 0; i < static_cast<int>(extra_info.size()); i++) {
-    strings->SetString(keys[i], extra_info[i]);
+std::string SSLBlockingPage::GetHistogramPrefix() const {
+  switch (interstitial_reason_) {
+    case SSL_REASON_SSL:
+      if (overridable_)
+        return "ssl_overridable";
+      else
+        return "ssl_nonoverridable";

[mattm, 2015/01/14 00:33:04]

Is there a reason for the difference here vs how the SB ones use
PROCEED_DISABLED?

[felt, 2015/01/14 00:55:18]

ssl_overridable and ssl_nonoverridable are different warnings, visually -- they
have different strings, different buttons, etc.

For Safe Browsing, the overridable and nonoverridable ones are basically the
same.

+    case SSL_REASON_BAD_CLOCK:
+      return "bad_clock";
   }
-  for (; i < 5; i++) {
-    strings->SetString(keys[i], std::string());
-  }
+  NOTREACHED();
+}
+
+std::string SSLBlockingPage::GetSamplingEventName() const {
+  std::string event_name(kEventNameBase);
+  if (overridable_)
+    event_name.append(kEventOverridable);
+  else
+    event_name.append(kEventNotOverridable);
+  event_name.append(net::ErrorToString(cert_error_));
+  return event_name;
 }
 
 // static
 bool SSLBlockingPage::IsOptionsOverridable(int options_mask) {
   return (options_mask & SSLBlockingPage::OVERRIDABLE) &&
          !(options_mask & SSLBlockingPage::STRICT_ENFORCEMENT);
 }
-
-void SSLBlockingPage::OnGotHistoryCount(bool success,
-                                        int num_visits,
-                                        base::Time first_visit) {
-  num_visits_ = num_visits;
-}