Commit 159e3bbd authored by felt's avatar felt Committed by Commit bot

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=

Review URL: https://codereview.chromium.org/572163002

Cr-Commit-Position: refs/heads/master@{#295145}
parent 874611d5
...@@ -98,10 +98,6 @@ const char kProceedCommand[] = "proceed"; ...@@ -98,10 +98,6 @@ const char kProceedCommand[] = "proceed";
const char kShowDiagnosticCommand[] = "showDiagnostic"; const char kShowDiagnosticCommand[] = "showDiagnostic";
const char kShowPrivacyCommand[] = "showPrivacy"; const char kShowPrivacyCommand[] = "showPrivacy";
const char kTakeMeBackCommand[] = "takeMeBack"; 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. // Other constants used to communicate with the JavaScript.
const char kBoxChecked[] = "boxchecked"; const char kBoxChecked[] = "boxchecked";
...@@ -182,7 +178,6 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage( ...@@ -182,7 +178,6 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
web_contents_(web_contents), web_contents_(web_contents),
url_(unsafe_resources[0].url), url_(unsafe_resources[0].url),
interstitial_page_(NULL), interstitial_page_(NULL),
has_expanded_see_more_section_(false),
create_view_(true), create_view_(true),
num_visits_(-1) { num_visits_(-1) {
bool malware = false; bool malware = false;
...@@ -277,7 +272,6 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) { ...@@ -277,7 +272,6 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) {
if (command.length() > 1 && command[0] == '"') { if (command.length() > 1 && command[0] == '"') {
command = command.substr(1, command.length() - 2); command = command.substr(1, command.length() - 2);
} }
RecordUserReactionTime(command);
if (command == kDoReportCommand) { if (command == kDoReportCommand) {
SetReportingPreference(true); SetReportingPreference(true);
return; return;
...@@ -398,10 +392,6 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) { ...@@ -398,10 +392,6 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) {
} }
if (command == kExpandedSeeMoreCommand) { 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) #if defined(ENABLE_EXTENSIONS)
// ExperienceSampling: We track that the user expanded the details. // ExperienceSampling: We track that the user expanded the details.
if (sampling_event_.get()) if (sampling_event_.get())
...@@ -475,8 +465,6 @@ void SafeBrowsingBlockingPage::Show() { ...@@ -475,8 +465,6 @@ void SafeBrowsingBlockingPage::Show() {
} }
void SafeBrowsingBlockingPage::OnDontProceed() { 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 // We could have already called Proceed(), in which case we must not notify
// the SafeBrowsingUIManager again, as the client has been deleted. // the SafeBrowsingUIManager again, as the client has been deleted.
if (proceeded_) if (proceeded_)
...@@ -631,81 +619,6 @@ void SafeBrowsingBlockingPage::RecordUserAction(BlockingPageEvent event) { ...@@ -631,81 +619,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) {
// 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) { void SafeBrowsingBlockingPage::FinishMalwareDetails(int64 delay_ms) {
if (malware_details_.get() == NULL) if (malware_details_.get() == NULL)
return; // Not all interstitials have malware details (eg phishing). return; // Not all interstitials have malware details (eg phishing).
...@@ -832,8 +745,6 @@ std::string SafeBrowsingBlockingPage::GetHTMLContents() { ...@@ -832,8 +745,6 @@ std::string SafeBrowsingBlockingPage::GetHTMLContents() {
else else
PopulateMalwareLoadTimeData(&load_time_data); PopulateMalwareLoadTimeData(&load_time_data);
interstitial_show_time_ = base::TimeTicks::Now();
base::StringPiece html( base::StringPiece html(
ResourceBundle::GetSharedInstance().GetRawDataResource( ResourceBundle::GetSharedInstance().GetRawDataResource(
IRD_SECURITY_INTERSTITIAL_HTML)); IRD_SECURITY_INTERSTITIAL_HTML));
......
...@@ -139,11 +139,6 @@ class SafeBrowsingBlockingPage : public content::InterstitialPageDelegate { ...@@ -139,11 +139,6 @@ class SafeBrowsingBlockingPage : public content::InterstitialPageDelegate {
// Used to query the HistoryService to see if the URL is in history. For UMA. // Used to query the HistoryService to see if the URL is in history. For UMA.
void OnGotHistoryCount(bool success, int num_visits, base::Time first_visit); void OnGotHistoryCount(bool success, int num_visits, base::Time first_visit);
// Records the time it took for the user to react to the
// interstitial. We won't double-count if this method is called
// multiple times.
void RecordUserReactionTime(const std::string& command);
// Checks if we should even show the malware details option. For example, we // Checks if we should even show the malware details option. For example, we
// don't show it in incognito mode. // don't show it in incognito mode.
bool CanShowMalwareDetailsOption(); bool CanShowMalwareDetailsOption();
...@@ -204,16 +199,6 @@ class SafeBrowsingBlockingPage : public content::InterstitialPageDelegate { ...@@ -204,16 +199,6 @@ class SafeBrowsingBlockingPage : public content::InterstitialPageDelegate {
GURL url_; GURL url_;
content::InterstitialPage* interstitial_page_; // Owns us content::InterstitialPage* interstitial_page_; // Owns us
// Time when the interstitial was show. This variable is set in
// GetHTMLContents() which is called right before the interstitial
// is shown to the user. Will return is_null() once we reported the
// user action.
base::TimeTicks interstitial_show_time_;
// Whether the user has expanded the "see more" section of the page already
// during this interstitial page.
bool has_expanded_see_more_section_;
// Whether the interstitial should create a view. // Whether the interstitial should create a view.
bool create_view_; bool create_view_;
......
...@@ -27553,6 +27553,9 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -27553,6 +27553,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
</histogram> </histogram>
<histogram name="SB2.MalwareInterstitialTimeClosed" units="milliseconds"> <histogram name="SB2.MalwareInterstitialTimeClosed" units="milliseconds">
<obsolete>
Deprecated 9/2014.
</obsolete>
<owner>felt@chromium.org</owner> <owner>felt@chromium.org</owner>
<summary> <summary>
The time between when we show the SafeBrowsing malware interstitial and the The time between when we show the SafeBrowsing malware interstitial and the
...@@ -27562,6 +27565,9 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -27562,6 +27565,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
</histogram> </histogram>
<histogram name="SB2.MalwareInterstitialTimeDiagnostic" units="milliseconds"> <histogram name="SB2.MalwareInterstitialTimeDiagnostic" units="milliseconds">
<obsolete>
Deprecated 9/2014.
</obsolete>
<owner>felt@chromium.org</owner> <owner>felt@chromium.org</owner>
<summary> <summary>
The time between when we show the SafeBrowsing malware interstitial and the The time between when we show the SafeBrowsing malware interstitial and the
...@@ -27571,6 +27577,9 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -27571,6 +27577,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
<histogram name="SB2.MalwareInterstitialTimeExpandedSeeMore" <histogram name="SB2.MalwareInterstitialTimeExpandedSeeMore"
units="milliseconds"> units="milliseconds">
<obsolete>
Deprecated 9/2014.
</obsolete>
<owner>felt@chromium.org</owner> <owner>felt@chromium.org</owner>
<summary> <summary>
The time between when we show the SafeBrowsing malware interstitial and the The time between when we show the SafeBrowsing malware interstitial and the
...@@ -27580,6 +27589,9 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -27580,6 +27589,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
</histogram> </histogram>
<histogram name="SB2.MalwareInterstitialTimeLearnMore" units="milliseconds"> <histogram name="SB2.MalwareInterstitialTimeLearnMore" units="milliseconds">
<obsolete>
Deprecated 9/2014.
</obsolete>
<owner>felt@chromium.org</owner> <owner>felt@chromium.org</owner>
<summary> <summary>
The time between when we show the SafeBrowsing malware interstitial and the The time between when we show the SafeBrowsing malware interstitial and the
...@@ -27588,6 +27600,9 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -27588,6 +27600,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
</histogram> </histogram>
<histogram name="SB2.MalwareInterstitialTimePrivacyPolicy" units="milliseconds"> <histogram name="SB2.MalwareInterstitialTimePrivacyPolicy" units="milliseconds">
<obsolete>
Deprecated 9/2014.
</obsolete>
<owner>felt@chromium.org</owner> <owner>felt@chromium.org</owner>
<summary> <summary>
The time between when we show the SafeBrowsing malware interstitial and the The time between when we show the SafeBrowsing malware interstitial and the
...@@ -27596,6 +27611,9 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -27596,6 +27611,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
</histogram> </histogram>
<histogram name="SB2.MalwareInterstitialTimeProceed" units="milliseconds"> <histogram name="SB2.MalwareInterstitialTimeProceed" units="milliseconds">
<obsolete>
Deprecated 9/2014.
</obsolete>
<owner>felt@chromium.org</owner> <owner>felt@chromium.org</owner>
<summary> <summary>
The time between when we show the SafeBrowsing malware interstitial and the The time between when we show the SafeBrowsing malware interstitial and the
...@@ -27604,6 +27622,9 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -27604,6 +27622,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
</histogram> </histogram>
<histogram name="SB2.MalwareInterstitialTimeTakeMeBack" units="milliseconds"> <histogram name="SB2.MalwareInterstitialTimeTakeMeBack" units="milliseconds">
<obsolete>
Deprecated 9/2014.
</obsolete>
<owner>felt@chromium.org</owner> <owner>felt@chromium.org</owner>
<summary> <summary>
The time between when we show the SafeBrowsing malware interstitial and the The time between when we show the SafeBrowsing malware interstitial and the
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment