Update the Safe Browsing interstitial histograms

chrome/browser/safe_browsing/safe_browsing_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.
 //
 // Implementation of the SafeBrowsingBlockingPage class.
 
 #include "chrome/browser/safe_browsing/safe_browsing_blocking_page.h"
 
 #include <string>
 
@@ skipping 99 matching lines @@
 // Constants for the Experience Sampling instrumentation.
 #if defined(ENABLE_EXTENSIONS)
 const char kEventNameMalware[] = "safebrowsing_interstitial_";
 const char kEventNamePhishing[] = "phishing_interstitial_";
 const char kEventNameOther[] = "safebrowsing_other_interstitial_";
 #endif
 
 base::LazyInstance<SafeBrowsingBlockingPage::UnsafeResourceMap>
     g_unsafe_resource_map = LAZY_INSTANCE_INITIALIZER;
 
-// This enum is used for a histogram.  Don't reorder, delete, or insert
-// elements.  New elements should be added before MAX_ACTION only.
-enum DetailedDecision {
-  MALWARE_SHOW_NEW_SITE = 0,
-  MALWARE_PROCEED_NEW_SITE,
-  MALWARE_SHOW_CROSS_SITE,
-  MALWARE_PROCEED_CROSS_SITE,
-  PHISHING_SHOW_NEW_SITE,
-  PHISHING_PROCEED_NEW_SITE,
-  PHISHING_SHOW_CROSS_SITE,
-  PHISHING_PROCEED_CROSS_SITE,
-  MAX_DETAILED_ACTION
-};
-
-void RecordDetailedUserAction(DetailedDecision decision) {
-  UMA_HISTOGRAM_ENUMERATION("SB2.InterstitialActionDetails",
-                            decision,
-                            MAX_DETAILED_ACTION);
-}
-
 }  // namespace
 
 // static
 SafeBrowsingBlockingPageFactory* SafeBrowsingBlockingPage::factory_ = NULL;
 
 // The default SafeBrowsingBlockingPageFactory.  Global, made a singleton so we
 // don't leak it.
 class SafeBrowsingBlockingPageFactoryImpl
     : public SafeBrowsingBlockingPageFactory {
  public:
@@ skipping 49 matching lines @@
              threat_type == SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL);
       phishing = true;
     }
   }
   DCHECK(phishing || malware);
   if (malware)
     interstitial_type_ = TYPE_MALWARE;
   else
     interstitial_type_ = TYPE_PHISHING;
 
-  RecordUserAction(SHOW);
+  RecordUserDecision(SHOW);
+  RecordUserInteraction(TOTAL_VISITS);
+  if (IsPrefEnabled(prefs::kSafeBrowsingProceedAnywayDisabled))
+    RecordUserDecision(PROCEEDING_DISABLED);
+
   HistoryService* history_service = HistoryServiceFactory::GetForProfile(
           Profile::FromBrowserContext(web_contents->GetBrowserContext()),
           Profile::EXPLICIT_ACCESS);
   if (history_service) {
     history_service->GetVisibleVisitCountToHost(
         url_,
         base::Bind(&SafeBrowsingBlockingPage::OnGotHistoryCount,
                    base::Unretained(this)),
         &request_tracker_);
   }
@@ skipping 63 matching lines @@
     return;
   }
 
   if (command == kDontReportCommand) {
     SetReportingPreference(false);
     return;
   }
 
   if (command == kLearnMoreCommand) {
     // User pressed "Learn more".
-#if defined(ENABLE_EXTENSIONS)
-    if (sampling_event_.get())
-      sampling_event_->set_has_viewed_learn_more(true);
-#endif
+    RecordUserInteraction(SHOW_LEARN_MORE);
     GURL learn_more_url(interstitial_type_ == TYPE_PHISHING ?
                         kLearnMorePhishingUrlV2 : kLearnMoreMalwareUrlV2);
     learn_more_url = google_util::AppendGoogleLocaleParam(
         learn_more_url, g_browser_process->GetApplicationLocale());
     OpenURLParams params(learn_more_url,
                          Referrer(),
                          CURRENT_TAB,
                          content::PAGE_TRANSITION_LINK,
                          false);
     web_contents_->OpenURL(params);
     return;
   }
 
   if (command == kShowPrivacyCommand) {
     // User pressed "Safe Browsing privacy policy".
+    RecordUserInteraction(SHOW_PRIVACY_POLICY);
     GURL privacy_url(
         l10n_util::GetStringUTF8(IDS_SAFE_BROWSING_PRIVACY_POLICY_URL));
     privacy_url = google_util::AppendGoogleLocaleParam(
         privacy_url, g_browser_process->GetApplicationLocale());
     OpenURLParams params(privacy_url,
                          Referrer(),
                          CURRENT_TAB,
                          content::PAGE_TRANSITION_LINK,
                          false);
     web_contents_->OpenURL(params);
     return;
   }
 
   bool proceed_blocked = false;
   if (command == kProceedCommand) {
     if (IsPrefEnabled(prefs::kSafeBrowsingProceedAnywayDisabled)) {
       proceed_blocked = true;
     } else {
+      RecordUserDecision(PROCEED);
       interstitial_page_->Proceed();
       // |this| has been deleted after Proceed() returns.
       return;
     }
   }
 
   if (command == kTakeMeBackCommand || proceed_blocked) {
+    // Don't record the user action here because there are other ways of
+    // triggering DontProceed, like clicking the back button.
     if (is_main_frame_load_blocked_) {
       // If the load is blocked, we want to close the interstitial and discard
       // the pending entry.
       interstitial_page_->DontProceed();
       // |this| has been deleted after DontProceed() returns.
       return;
     }
 
     // Otherwise the offending entry has committed, and we need to go back or
     // to a safe page.  We will close the interstitial when that page commits.
@@ skipping 26 matching lines @@
   }
 
   if (element_index >= unsafe_resources_.size()) {
     NOTREACHED();
     return;
   }
 
   std::string bad_url_spec = unsafe_resources_[element_index].url.spec();
   if (command == kShowDiagnosticCommand) {
     // We're going to take the user to Google's SafeBrowsing diagnostic page.
+    RecordUserInteraction(SHOW_DIAGNOSTIC);
     std::string diagnostic =
         base::StringPrintf(kSbDiagnosticUrl,
             net::EscapeQueryParamValue(bad_url_spec, true).c_str());
     GURL diagnostic_url(diagnostic);
     diagnostic_url = google_util::AppendGoogleLocaleParam(
         diagnostic_url, g_browser_process->GetApplicationLocale());
     DCHECK(unsafe_resources_[element_index].threat_type ==
            SB_THREAT_TYPE_URL_MALWARE ||
            unsafe_resources_[element_index].threat_type ==
            SB_THREAT_TYPE_CLIENT_SIDE_MALWARE_URL);
     OpenURLParams params(
         diagnostic_url, Referrer(), CURRENT_TAB, content::PAGE_TRANSITION_LINK,
         false);
     web_contents_->OpenURL(params);
     return;
   }
 
   if (command == kExpandedSeeMoreCommand) {
-    // User expanded the "see more info" section of the page.  We don't actually
-    // do any action based on this, it's just so that RecordUserReactionTime can
-    // track it.
-
-#if defined(ENABLE_EXTENSIONS)
-    // ExperienceSampling: We track that the user expanded the details.
-    if (sampling_event_.get())
-      sampling_event_->set_has_viewed_details(true);
-#endif
+    RecordUserInteraction(SHOW_ADVANCED);
     return;
   }
 
   NOTREACHED() << "Unexpected command: " << command;
 }
 
 void SafeBrowsingBlockingPage::OverrideRendererPrefs(
       content::RendererPreferences* prefs) {
   Profile* profile = Profile::FromBrowserContext(
       web_contents_->GetBrowserContext());
   renderer_preferences_util::UpdateFromSystemSettings(prefs, profile);
  }
 
 void SafeBrowsingBlockingPage::SetReportingPreference(bool report) {
   Profile* profile = Profile::FromBrowserContext(
       web_contents_->GetBrowserContext());
   PrefService* pref = profile->GetPrefs();
   pref->SetBoolean(prefs::kSafeBrowsingExtendedReportingEnabled, report);
   UMA_HISTOGRAM_BOOLEAN("SB2.SetExtendedReportingEnabled", report);
 }
 
 void SafeBrowsingBlockingPage::OnProceed() {
   proceeded_ = true;
-  RecordUserAction(PROCEED);
   // Send the malware details, if we opted to.
   FinishMalwareDetails(malware_details_proceed_delay_ms_);
 
   NotifySafeBrowsingUIManager(ui_manager_, unsafe_resources_, true);
 
   // Check to see if some new notifications of unsafe resources have been
   // received while we were showing the interstitial.
   UnsafeResourceMap* unsafe_resource_map = GetUnsafeResourcesMap();
   UnsafeResourceMap::iterator iter = unsafe_resource_map->find(web_contents_);
   SafeBrowsingBlockingPage* blocking_page = NULL;
   if (iter != unsafe_resource_map->end() && !iter->second.empty()) {
     // Build an interstitial for all the unsafe resources notifications.
     // Don't show it now as showing an interstitial while an interstitial is
     // already showing would cause DontProceed() to be invoked.
     blocking_page = factory_->CreateSafeBrowsingPage(ui_manager_, web_contents_,
                                                      iter->second);
     unsafe_resource_map->erase(iter);
   }
 
-#if defined(ENABLE_EXTENSIONS)
-  // ExperienceSampling: Notify that user decided to proceed.
-  if (sampling_event_.get())
-    sampling_event_->CreateUserDecisionEvent(ExperienceSamplingEvent::kProceed);
-#endif
-
   // Now that this interstitial is gone, we can show the new one.
   if (blocking_page)
     blocking_page->Show();
 }
 
 void SafeBrowsingBlockingPage::DontCreateViewForTesting() {
   create_view_ = false;
 }
 
 void SafeBrowsingBlockingPage::Show() {
   DCHECK(!interstitial_page_);
   interstitial_page_ = InterstitialPage::Create(
       web_contents_, is_main_frame_load_blocked_, url_, this);
   if (!create_view_)
     interstitial_page_->DontCreateViewForTesting();
   interstitial_page_->Show();
 }
 
 void SafeBrowsingBlockingPage::OnDontProceed() {
   // Calling this method twice will not double-count.
   RecordUserReactionTime(kNavigatedAwayMetaCommand);
   // We could have already called Proceed(), in which case we must not notify
   // the SafeBrowsingUIManager again, as the client has been deleted.
   if (proceeded_)
     return;
 
-  RecordUserAction(DONT_PROCEED);
+  if (!IsPrefEnabled(prefs::kSafeBrowsingProceedAnywayDisabled))
+    RecordUserDecision(DONT_PROCEED);
+
   // Send the malware details, if we opted to.
   FinishMalwareDetails(0);  // No delay
 
   NotifySafeBrowsingUIManager(ui_manager_, unsafe_resources_, false);
 
   // The user does not want to proceed, clear the queued unsafe resources
   // notifications we received while the interstitial was showing.
   UnsafeResourceMap* unsafe_resource_map = GetUnsafeResourcesMap();
   UnsafeResourceMap::iterator iter = unsafe_resource_map->find(web_contents_);
   if (iter != unsafe_resource_map->end() && !iter->second.empty()) {
     NotifySafeBrowsingUIManager(ui_manager_, iter->second, false);
     unsafe_resource_map->erase(iter);
   }
 
   // We don't remove the navigation entry if the tab is being destroyed as this
   // would trigger a navigation that would cause trouble as the render view host
   // for the tab has by then already been destroyed.  We also don't delete the
   // current entry if it has been committed again, which is possible on a page
   // that had a subresource warning.
   int last_committed_index =
       web_contents_->GetController().GetLastCommittedEntryIndex();
   if (navigation_entry_index_to_remove_ != -1 &&
       navigation_entry_index_to_remove_ != last_committed_index &&
       !web_contents_->IsBeingDestroyed()) {
     CHECK(web_contents_->GetController().RemoveEntryAtIndex(
         navigation_entry_index_to_remove_));
     navigation_entry_index_to_remove_ = -1;
   }
-
-#if defined(ENABLE_EXTENSIONS)
-  // ExperienceSampling: Notify that user decided to go back.
-  // This also occurs if the user navigates away or closes the tab.
-  if (sampling_event_.get())
-    sampling_event_->CreateUserDecisionEvent(ExperienceSamplingEvent::kDeny);
-#endif
 }
 
 void SafeBrowsingBlockingPage::OnGotHistoryCount(bool success,
                                                  int num_visits,
                                                  base::Time first_visit) {
   if (success)
     num_visits_ = num_visits;
 }
 
-void SafeBrowsingBlockingPage::RecordUserAction(BlockingPageEvent event) {
-  // This enum is used for a histogram.  Don't reorder, delete, or insert
-  // elements.  New elements should be added before MAX_ACTION only.
-  enum {
-    MALWARE_SHOW = 0,
-    MALWARE_DONT_PROCEED,
-    MALWARE_FORCED_DONT_PROCEED,
-    MALWARE_PROCEED,
-    MULTIPLE_SHOW,
-    MULTIPLE_DONT_PROCEED,
-    MULTIPLE_FORCED_DONT_PROCEED,
-    MULTIPLE_PROCEED,
-    PHISHING_SHOW,
-    PHISHING_DONT_PROCEED,
-    PHISHING_FORCED_DONT_PROCEED,
-    PHISHING_PROCEED,
-    MALWARE_SHOW_ADVANCED,
-    MULTIPLE_SHOW_ADVANCED,
-    PHISHING_SHOW_ADVANCED,
-    MAX_ACTION
-  } histogram_action = MAX_ACTION;
-
-  switch (event) {
-    case SHOW:
-      switch (interstitial_type_) {
-        case TYPE_MALWARE:
-          histogram_action = MALWARE_SHOW;
-          break;
-        case TYPE_PHISHING:
-          histogram_action = PHISHING_SHOW;
-          break;
-      }
+void SafeBrowsingBlockingPage::RecordUserDecision(Decision decision) {
+  switch (interstitial_type_) {
+    case TYPE_MALWARE:
+      UMA_HISTOGRAM_ENUMERATION("interstitial.malware.decision",
+                                decision,
+                                MAX_DECISION);
       break;
-    case PROCEED:
-      switch (interstitial_type_) {
-        case TYPE_MALWARE:
-          histogram_action = MALWARE_PROCEED;
-          break;
-        case TYPE_PHISHING:
-          histogram_action = PHISHING_PROCEED;
-          break;
-      }
-      break;
-    case DONT_PROCEED:
-      if (IsPrefEnabled(prefs::kSafeBrowsingProceedAnywayDisabled)) {
-        switch (interstitial_type_) {
-          case TYPE_MALWARE:
-            histogram_action = MALWARE_FORCED_DONT_PROCEED;
-            break;
-          case TYPE_PHISHING:
-            histogram_action = PHISHING_FORCED_DONT_PROCEED;
-            break;
-        }
-      } else {
-        switch (interstitial_type_) {
-          case TYPE_MALWARE:
-            histogram_action = MALWARE_DONT_PROCEED;
-            break;
-          case TYPE_PHISHING:
-            histogram_action = PHISHING_DONT_PROCEED;
-            break;
-        }
-      }
-      break;
-    case SHOW_ADVANCED:
-      switch (interstitial_type_) {
-        case TYPE_MALWARE:
-          histogram_action = MALWARE_SHOW_ADVANCED;
-          break;
-        case TYPE_PHISHING:
-          histogram_action = PHISHING_SHOW_ADVANCED;
-          break;
-      }
+    case TYPE_PHISHING:
+      UMA_HISTOGRAM_ENUMERATION("interstitial.phishing.decision",
+                                decision,
+                                MAX_DECISION);
       break;
     default:
-      NOTREACHED() << "Unexpected event: " << event;
-  }
-  if (histogram_action == MAX_ACTION) {
-    NOTREACHED();
-  } else {
-    UMA_HISTOGRAM_ENUMERATION("SB2.InterstitialAction", histogram_action,
-                              MAX_ACTION);
+      NOTREACHED();
   }
 
-  if (event == PROCEED || event == DONT_PROCEED) {
-    if (num_visits_ == 0) {
-      RecordDetailedUserAction((interstitial_type_ == TYPE_MALWARE) ?
-                               MALWARE_SHOW_NEW_SITE : PHISHING_SHOW_NEW_SITE);
-      if (event == PROCEED) {
-        RecordDetailedUserAction((interstitial_type_ == TYPE_MALWARE) ?
-            MALWARE_PROCEED_NEW_SITE : PHISHING_PROCEED_NEW_SITE);
-      }
+#if defined(ENABLE_EXTENSIONS)
+  if (sampling_event_.get()) {
+    switch (decision) {
+      case PROCEED:
+        sampling_event_->CreateUserDecisionEvent(
+            ExperienceSamplingEvent::kProceed);
+        break;
+      case DONT_PROCEED:
+        sampling_event_->CreateUserDecisionEvent(
+            ExperienceSamplingEvent::kDeny);
+        break;
+      case SHOW:
+      case PROCEEDING_DISABLED:
+      case MAX_DECISION:
+        break;
     }
-    if (unsafe_resources_[0].is_subresource) {
-      RecordDetailedUserAction((interstitial_type_ == TYPE_MALWARE) ?
-          MALWARE_SHOW_CROSS_SITE : PHISHING_SHOW_CROSS_SITE);
-      if (event == PROCEED) {
-        RecordDetailedUserAction((interstitial_type_ == TYPE_MALWARE) ?
-            MALWARE_PROCEED_CROSS_SITE : PHISHING_PROCEED_CROSS_SITE);
-      }
-    }
+  }
+#endif
+
+  // Record additional information about malware sites that users have
+  // visited before.
+  if (num_visits_ < 1 || interstitial_type_ != TYPE_MALWARE)
+    return;
+  if (decision == PROCEED || decision == DONT_PROCEED) {
+    UMA_HISTOGRAM_ENUMERATION("interstitial.malware.decision.repeat_visit",
+                              SHOW,
+                              MAX_DECISION);
+    UMA_HISTOGRAM_ENUMERATION("interstitial.malware.decision.repeat_visit",
+                              decision,
+                              MAX_DECISION);
   }
 }
 
+void SafeBrowsingBlockingPage::RecordUserInteraction(Interaction interaction) {
+  switch (interstitial_type_) {
+    case TYPE_MALWARE:
+      UMA_HISTOGRAM_ENUMERATION("interstitial.malware.interaction",
+                                interaction,
+                                MAX_INTERACTION);
+      break;
+    case TYPE_PHISHING:
+      UMA_HISTOGRAM_ENUMERATION("interstitial.phishing.interaction",
+                                interaction,
+                                MAX_INTERACTION);
+      break;
+    default:
+      NOTREACHED();
+  }
+
+#if defined(ENABLE_EXTENSIONS)
+  if (!sampling_event_.get())
+    return;
+  switch (interaction) {

[mattm, 2014/09/17 01:22:21]

Do you need TOTAL_VISITS in this switch?

[felt, 2014/09/17 01:27:12]

Done.

+    case SHOW_LEARN_MORE:
+      sampling_event_->set_has_viewed_learn_more(true);
+      break;
+    case SHOW_ADVANCED:
+      sampling_event_->set_has_viewed_details(true);
+      break;
+    case SHOW_PRIVACY_POLICY:
+    case SHOW_DIAGNOSTIC:
+    case MAX_INTERACTION:
+      break;
+  }
+#endif
+}
+
 void SafeBrowsingBlockingPage::RecordUserReactionTime(
     const std::string& command) {
   if (interstitial_show_time_.is_null())
     return;  // We already reported the user reaction time.
   base::TimeDelta dt = base::TimeTicks::Now() - interstitial_show_time_;
   DVLOG(1) << "User reaction time for command:" << command
            << " on interstitial_type_:" << interstitial_type_
            << " warning took " << dt.InMilliseconds() << "ms";
   bool recorded = true;
   if (interstitial_type_ == TYPE_MALWARE) {
@@ skipping 13 matching lines @@
     } else if (command == kLearnMoreCommand) {
       UMA_HISTOGRAM_MEDIUM_TIMES("SB2.MalwareInterstitialLearnMore",
                                  dt);
     } else if (command == kNavigatedAwayMetaCommand) {
       UMA_HISTOGRAM_MEDIUM_TIMES("SB2.MalwareInterstitialTimeClosed", dt);
     } else if (command == kExpandedSeeMoreCommand) {
       // Only record the expanded histogram once per display of the
       // interstitial.
       if (has_expanded_see_more_section_)
         return;
-      RecordUserAction(SHOW_ADVANCED);
       UMA_HISTOGRAM_MEDIUM_TIMES("SB2.MalwareInterstitialTimeExpandedSeeMore",
                                  dt);
       has_expanded_see_more_section_ = true;
       // Expanding the "See More" section doesn't finish the interstitial, so
       // don't mark the reaction time as recorded.
       recorded = false;
     } else {
       recorded = false;
     }
   } else {
     // Same as above but for phishing warnings.
     if (command == kProceedCommand) {
       UMA_HISTOGRAM_MEDIUM_TIMES("SB2.PhishingInterstitialTimeProceed", dt);
     } else if (command == kTakeMeBackCommand) {
       UMA_HISTOGRAM_MEDIUM_TIMES("SB2.PhishingInterstitialTimeTakeMeBack", dt);
     } else if (command == kShowDiagnosticCommand) {
       UMA_HISTOGRAM_MEDIUM_TIMES("SB2.PhishingInterstitialTimeReportError", dt);
     } else if (command == kLearnMoreCommand) {
       UMA_HISTOGRAM_MEDIUM_TIMES("SB2.PhishingInterstitialTimeLearnMore", dt);
     } else if (command == kNavigatedAwayMetaCommand) {
       UMA_HISTOGRAM_MEDIUM_TIMES("SB2.PhishingInterstitialTimeClosed", dt);
     } else if (command == kExpandedSeeMoreCommand) {
       // Only record the expanded histogram once per display of the
       // interstitial.
       if (has_expanded_see_more_section_)
         return;
-      RecordUserAction(SHOW_ADVANCED);
       UMA_HISTOGRAM_MEDIUM_TIMES("SB2.PhishingInterstitialTimeExpandedSeeMore",
                                  dt);
       has_expanded_see_more_section_ = true;
       // Expanding the "See More" section doesn't finish the interstitial, so
       // don't mark the reaction time as recorded.
       recorded = false;
     } else {
       recorded = false;
     }
   }
@@ skipping 188 matching lines @@
           IDS_PHISHING_V3_PRIMARY_PARAGRAPH,
           base::UTF8ToUTF16(url_.host())));
   load_time_data->SetString(
       "explanationParagraph",
       l10n_util::GetStringFUTF16(IDS_PHISHING_V3_EXPLANATION_PARAGRAPH,
                                  base::UTF8ToUTF16(url_.host())));
   load_time_data->SetString(
       "finalParagraph",
       l10n_util::GetStringUTF16(IDS_PHISHING_V3_PROCEED_PARAGRAPH));
 }