Deprecate Safe Browsing timing histograms

Deprecate Safe Browsing timing histograms

We haven't been using the Safe Browsing timing histograms. Remove the
timing code and mark the histograms obsolete.

BUG=

Committed: https://crrev.com/159e3bbd2ae9b208691244005d705cbe29efe75f
Cr-Commit-Position: refs/heads/master@{#295145}

[felt, 2014-09-16 04:36:53]

felt@chromium.org changed reviewers:
+ asvitkine@chromium.org, mattm@chromium.org

[felt, 2014-09-16 04:37:22]

Matt and Alexei, PTAL.

[mattm, 2014-09-16 07:29:54]

lgtm with nit

https://codereview.chromium.org/572163002/diff/1/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.h (right):

https://codereview.chromium.org/572163002/diff/1/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/safe_browsing_blocking_page.h:204: bool
has_expanded_see_more_section_;
Can remove this too?

[Alexei Svitkine (slow), 2014-09-16 13:19:32]

lgtm

[felt, 2014-09-16 14:56:35]

https://codereview.chromium.org/572163002/diff/1/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.h (right):

https://codereview.chromium.org/572163002/diff/1/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/safe_browsing_blocking_page.h:204: bool
has_expanded_see_more_section_;
On 2014/09/16 07:29:54, mattm wrote:
> Can remove this too?

Done.

[felt, 2014-09-16 14:57:15]

The CQ bit was checked by felt@chromium.org

[commit-bot: I haz the power, 2014-09-16 14:57:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/572163002/20001

[commit-bot: I haz the power, 2014-09-16 15:54:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-16 15:54:59]

Failed to apply patch for
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:677
  error: chrome/browser/safe_browsing/safe_browsing_blocking_page.cc: patch does
not apply

Patch:       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
b859f7343d24e682fb2807aaf19a8e5772a9faf5..9c9f34fcd8d672b6017c22d62d54283e869e3bc5
100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -102,10 +102,6 @@ const char kReportErrorCommand[] = "reportError";
 const char kShowDiagnosticCommand[] = "showDiagnostic";
 const char kShowPrivacyCommand[] = "showPrivacy";
 const char kTakeMeBackCommand[] = "takeMeBack";
-// Special command that we use when the user navigated away from the
-// page.  E.g., closed the tab or the window.  This is only used by
-// RecordUserReactionTime.
-const char kNavigatedAwayMetaCommand[] = "closed";
 
 // Other constants used to communicate with the JavaScript.
 const char kBoxChecked[] = "boxchecked";
@@ -188,7 +184,6 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
       web_contents_(web_contents),
       url_(unsafe_resources[0].url),
       interstitial_page_(NULL),
-      has_expanded_see_more_section_(false),
       create_view_(true),
       num_visits_(-1) {
   bool malware = false;
@@ -288,7 +283,6 @@ void SafeBrowsingBlockingPage::CommandReceived(const
std::string& cmd) {
   if (command.length() > 1 && command[0] == '"') {
     command = command.substr(1, command.length() - 2);
   }
-  RecordUserReactionTime(command);
   if (command == kDoReportCommand) {
     SetReportingPreference(true);
     return;
@@ -428,10 +422,6 @@ void SafeBrowsingBlockingPage::CommandReceived(const
std::string& cmd) {
   }
 
   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())
@@ -505,8 +495,6 @@ void SafeBrowsingBlockingPage::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_)
@@ -677,82 +665,6 @@ void
SafeBrowsingBlockingPage::RecordUserAction(BlockingPageEvent event) {
   }
 }
 
-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 ||
-      interstitial_type_ == TYPE_MALWARE_AND_PHISHING) {
-    // There are six ways in which the malware interstitial can go
-    // away.  We handle all of them here but we group two together: closing the
-    // tag / browser window and clicking on the back button in the browser (not
-    // the big green button) are considered the same action.
-    if (command == kProceedCommand) {
-      UMA_HISTOGRAM_MEDIUM_TIMES("SB2.MalwareInterstitialTimeProceed", dt);
-    } else if (command == kTakeMeBackCommand) {
-      UMA_HISTOGRAM_MEDIUM_TIMES("SB2.MalwareInterstitialTimeTakeMeBack", dt);
-    } else if (command == kShowDiagnosticCommand) {
-      UMA_HISTOGRAM_MEDIUM_TIMES("SB2.MalwareInterstitialTimeDiagnostic", dt);
-    } else if (command == kShowPrivacyCommand) {
-      UMA_HISTOGRAM_MEDIUM_TIMES("SB2.MalwareInterstitialTimePrivacyPolicy",
-                                 dt);
-    } 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;
-    }
-  }
-  if (recorded)  // Making sure we don't double-count reaction times.
-    interstitial_show_time_ = base::TimeTicks();  //  Resets the show time.
-}
-
 void SafeBrowsingBlockingPage::FinishMalwareDetails(int64 delay_ms) {
   if (malware_details_.get() == NULL)
     return;  // Not all interstitials have malware details (eg phishing).
@@ -879,8 +791,6 @@ std::string SafeBrowsingBlockingPage::GetHTMLContents() {
   else
     PopulateMalwareLoadTimeData(&load_time_data);
 
-  interstitial_show_time_ = base::TimeTicks::Now();
-
   base::StringPiece html(
       ResourceBundle::GetSharedInstance().GetRawDataResource(
           IRD_SECURITY_INTERSTITIAL_HTML));

[felt, 2014-09-16 20:03:00]

The CQ bit was checked by felt@chromium.org

[commit-bot: I haz the power, 2014-09-16 20:03:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/572163002/40001

[commit-bot: I haz the power, 2014-09-16 21:30:52]

Committed patchset #3 (id:40001) as a54ff0be9a935d245a2bd7c1c72c0f3020d631c2

[commit-bot: I haz the power, 2014-09-16 21:31:33]

Patchset 3 (id:??) landed as
https://crrev.com/159e3bbd2ae9b208691244005d705cbe29efe75f
Cr-Commit-Position: refs/heads/master@{#295145}