Simplify security_interstitials::ControllerClient and other related classes

chrome/browser/safe_browsing/safe_browsing_blocking_page.cc

Index: chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index e944af9613ae2866a57aec886acd27335ca73346..9c78afa989a423e944b1521219c292e8cd3d1d2f 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -131,57 +131,23 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
     WebContents* web_contents,
     const GURL& main_frame_url,
     const UnsafeResourceList& unsafe_resources)
-    : SecurityInterstitialPage(web_contents, unsafe_resources[0].url),
+    : SecurityInterstitialPage(
+          web_contents,
+          unsafe_resources[0].url,
+          CreateMetricsHelper(web_contents, unsafe_resources)),
       threat_details_proceed_delay_ms_(kThreatDetailsProceedDelayMilliSeconds),
       ui_manager_(ui_manager),
       is_main_frame_load_blocked_(IsMainPageLoadBlocked(unsafe_resources)),
       main_frame_url_(main_frame_url),
       unsafe_resources_(unsafe_resources),
-      proceeded_(false) {
-  bool malware = false;
-  bool harmful = false;
-  bool phishing = false;
-  for (UnsafeResourceList::const_iterator iter = unsafe_resources_.begin();
-       iter != unsafe_resources_.end(); ++iter) {
-    const UnsafeResource& resource = *iter;
-    SBThreatType threat_type = resource.threat_type;
-    if (threat_type == SB_THREAT_TYPE_URL_MALWARE ||
-        threat_type == SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL) {
-      malware = true;
-    } else if (threat_type == SB_THREAT_TYPE_URL_UNWANTED) {
-      harmful = true;
-    } else {
-      DCHECK(threat_type == SB_THREAT_TYPE_URL_PHISHING ||
-             threat_type == SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL);
-      phishing = true;
-    }
-  }
-  DCHECK(phishing || malware || harmful);
-  if (malware)
-    interstitial_reason_ = SB_REASON_MALWARE;
-  else if (harmful)
-    interstitial_reason_ = SB_REASON_HARMFUL;
-  else
-    interstitial_reason_ = SB_REASON_PHISHING;
-
-  // This must be done after calculating |interstitial_reason_| above.
-  security_interstitials::MetricsHelper::ReportDetails reporting_info;
-  reporting_info.metric_prefix = GetMetricPrefix();
-  reporting_info.extra_suffix = GetExtraMetricsSuffix();
-  reporting_info.rappor_prefix = GetRapporPrefix();
-  reporting_info.deprecated_rappor_prefix = GetDeprecatedRapporPrefix();
-  reporting_info.rappor_report_type =
-      rappor::LOW_FREQUENCY_SAFEBROWSING_RAPPOR_TYPE;
-  reporting_info.deprecated_rappor_report_type =
-      rappor::SAFEBROWSING_RAPPOR_TYPE;
-  set_metrics_helper(base::MakeUnique<ChromeMetricsHelper>(
-      web_contents, request_url(), reporting_info, GetSamplingEventName()));
-  metrics_helper()->RecordUserDecision(
+      proceeded_(false),
+      interstitial_reason_(GetInterstitialReason(unsafe_resources)) {
+  controller()->metrics_helper()->RecordUserDecision(
       security_interstitials::MetricsHelper::SHOW);
-  metrics_helper()->RecordUserInteraction(
+  controller()->metrics_helper()->RecordUserInteraction(
       security_interstitials::MetricsHelper::TOTAL_VISITS);
   if (IsPrefEnabled(prefs::kSafeBrowsingProceedAnywayDisabled)) {
-    metrics_helper()->RecordUserDecision(
+    controller()->metrics_helper()->RecordUserDecision(
         security_interstitials::MetricsHelper::PROCEEDING_DISABLED);
   }
 
@@ -355,7 +321,7 @@ void SafeBrowsingBlockingPage::OnProceed() {
   proceeded_ = true;
   // Send the threat details, if we opted to.
   FinishThreatDetails(threat_details_proceed_delay_ms_, true, /* did_proceed */
-                      metrics_helper()->NumVisits());
+                      controller()->metrics_helper()->NumVisits());
 
   ui_manager_->OnBlockingPageDone(unsafe_resources_, true);
 
@@ -398,13 +364,13 @@ void SafeBrowsingBlockingPage::OnDontProceed() {
     return;
 
   if (!IsPrefEnabled(prefs::kSafeBrowsingProceedAnywayDisabled)) {
-    metrics_helper()->RecordUserDecision(
+    controller()->metrics_helper()->RecordUserDecision(
         security_interstitials::MetricsHelper::DONT_PROCEED);
   }
 
   // Send the malware details, if we opted to.
   FinishThreatDetails(0, false /* did_proceed */,
-                      metrics_helper()->NumVisits());  // No delay
+                      controller()->metrics_helper()->NumVisits());  // No delay
 
   ui_manager_->OnBlockingPageDone(unsafe_resources_, false);
 
@@ -445,7 +411,7 @@ void SafeBrowsingBlockingPage::FinishThreatDetails(int64_t delay_ms,
   if (!enabled)
     return;
 
-  metrics_helper()->RecordUserInteraction(
+  controller()->metrics_helper()->RecordUserInteraction(
       security_interstitials::MetricsHelper::EXTENDED_REPORTING_IS_ENABLED);
   // Finish the malware details collection, send it over.
   BrowserThread::PostDelayedTask(
@@ -520,16 +486,19 @@ bool SafeBrowsingBlockingPage::IsMainPageLoadBlocked(
          unsafe_resources[0].IsMainPageLoadBlocked();
 }
 
-std::string SafeBrowsingBlockingPage::GetMetricPrefix() const {
-  bool primary_subresource = unsafe_resources_[0].is_subresource;
-  switch (interstitial_reason_) {
+// static
+std::string SafeBrowsingBlockingPage::GetMetricPrefix(
+    const UnsafeResourceList& unsafe_resources,
+    SBInterstitialReason interstitial_reason) {
+  bool primary_subresource = unsafe_resources[0].is_subresource;
+  switch (interstitial_reason) {
     case SB_REASON_MALWARE:
       return primary_subresource ? "malware_subresource" : "malware";
     case SB_REASON_HARMFUL:
       return primary_subresource ? "harmful_subresource" : "harmful";
     case SB_REASON_PHISHING:
       ThreatPatternType threat_pattern_type =
-          unsafe_resources_[0].threat_metadata.threat_pattern_type;
+          unsafe_resources[0].threat_metadata.threat_pattern_type;
       if (threat_pattern_type == ThreatPatternType::PHISHING ||
           threat_pattern_type == ThreatPatternType::NONE)
         return primary_subresource ? "phishing_subresource" : "phishing";
@@ -546,8 +515,10 @@ std::string SafeBrowsingBlockingPage::GetMetricPrefix() const {
 }
 
 // We populate a parallel set of metrics to differentiate some threat sources.
-std::string SafeBrowsingBlockingPage::GetExtraMetricsSuffix() const {
-  switch (unsafe_resources_[0].threat_source) {
+// static
+std::string SafeBrowsingBlockingPage::GetExtraMetricsSuffix(
+    const UnsafeResourceList& unsafe_resources) {
+  switch (unsafe_resources[0].threat_source) {
     case safe_browsing::ThreatSource::DATA_SAVER:
       return "from_data_saver";
     case safe_browsing::ThreatSource::REMOTE:
@@ -566,8 +537,10 @@ std::string SafeBrowsingBlockingPage::GetExtraMetricsSuffix() const {
   return std::string();
 }
 
-std::string SafeBrowsingBlockingPage::GetRapporPrefix() const {
-  switch (interstitial_reason_) {
+// static
+std::string SafeBrowsingBlockingPage::GetRapporPrefix(
+    SBInterstitialReason interstitial_reason) {
+  switch (interstitial_reason) {
     case SB_REASON_MALWARE:
       return "malware2";
     case SB_REASON_HARMFUL:
@@ -579,8 +552,10 @@ std::string SafeBrowsingBlockingPage::GetRapporPrefix() const {
   return std::string();
 }
 
-std::string SafeBrowsingBlockingPage::GetDeprecatedRapporPrefix() const {
-  switch (interstitial_reason_) {
+// static
+std::string SafeBrowsingBlockingPage::GetDeprecatedRapporPrefix(
+    SBInterstitialReason interstitial_reason) {
+  switch (interstitial_reason) {
     case SB_REASON_MALWARE:
       return "malware";
     case SB_REASON_HARMFUL:
@@ -592,8 +567,10 @@ std::string SafeBrowsingBlockingPage::GetDeprecatedRapporPrefix() const {
   return std::string();
 }
 
-std::string SafeBrowsingBlockingPage::GetSamplingEventName() const {
-  switch (interstitial_reason_) {
+// static
+std::string SafeBrowsingBlockingPage::GetSamplingEventName(
+    SBInterstitialReason interstitial_reason) {
+  switch (interstitial_reason) {
     case SB_REASON_MALWARE:
       return kEventNameMalware;
     case SB_REASON_HARMFUL:
@@ -605,6 +582,60 @@ std::string SafeBrowsingBlockingPage::GetSamplingEventName() const {
   }
 }
 
+// static
+SafeBrowsingBlockingPage::SBInterstitialReason
+SafeBrowsingBlockingPage::GetInterstitialReason(
+    const UnsafeResourceList& unsafe_resources) {
+  bool malware = false;
+  bool harmful = false;
+  bool phishing = false;
+  for (UnsafeResourceList::const_iterator iter = unsafe_resources.begin();
+       iter != unsafe_resources.end(); ++iter) {
+    const SafeBrowsingUIManager::UnsafeResource& resource = *iter;
+    safe_browsing::SBThreatType threat_type = resource.threat_type;
+    if (threat_type == SB_THREAT_TYPE_URL_MALWARE ||
+        threat_type == SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL) {
+      malware = true;
+    } else if (threat_type == SB_THREAT_TYPE_URL_UNWANTED) {
+      harmful = true;
+    } else {
+      DCHECK(threat_type == SB_THREAT_TYPE_URL_PHISHING ||
+             threat_type == SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL);
+      phishing = true;
+    }
+  }
+  DCHECK(phishing || malware || harmful);
+  if (malware)
+    return SB_REASON_MALWARE;
+  else if (harmful)
+    return SB_REASON_HARMFUL;
+  return SB_REASON_PHISHING;
+}
+
+// static
+std::unique_ptr<ChromeMetricsHelper>
+SafeBrowsingBlockingPage::CreateMetricsHelper(
+    WebContents* web_contents,
+    const UnsafeResourceList& unsafe_resources) {
+  SBInterstitialReason interstitial_reason =
+      GetInterstitialReason(unsafe_resources);
+  GURL request_url(unsafe_resources[0].url);
+  security_interstitials::MetricsHelper::ReportDetails reporting_info;
+  reporting_info.metric_prefix =
+      GetMetricPrefix(unsafe_resources, interstitial_reason);
+  reporting_info.extra_suffix = GetExtraMetricsSuffix(unsafe_resources);
+  reporting_info.rappor_prefix = GetRapporPrefix(interstitial_reason);
+  reporting_info.deprecated_rappor_prefix =
+      GetDeprecatedRapporPrefix(interstitial_reason);
+  reporting_info.rappor_report_type =
+      rappor::LOW_FREQUENCY_SAFEBROWSING_RAPPOR_TYPE;
+  reporting_info.deprecated_rappor_report_type =
+      rappor::SAFEBROWSING_RAPPOR_TYPE;
+  return std::unique_ptr<ChromeMetricsHelper>(
+      new ChromeMetricsHelper(web_contents, request_url, reporting_info,
+                              GetSamplingEventName(interstitial_reason)));
+}
+
 void SafeBrowsingBlockingPage::PopulateInterstitialStrings(
     base::DictionaryValue* load_time_data) {
   CHECK(load_time_data);