Skip to content

Commit

Permalink
Add a histogram for each time the HTTP Bad UI is shown
Browse files Browse the repository at this point in the history
Log a sample each time a HTTP Bad UI warning is shown in either the console or
the omnibox.

BUG=647567

Review-Url: https://codereview.chromium.org/2451293002
Cr-Commit-Position: refs/heads/master@{#428441}
  • Loading branch information
ericlaw1979 authored and Commit bot committed Oct 28, 2016
1 parent 5f1173f commit c346a85
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 0 deletions.
5 changes: 5 additions & 0 deletions chrome/browser/ssl/chrome_security_state_model_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -338,14 +338,17 @@ void ChromeSecurityStateModelClient::VisibleSecurityStateChanged() {
return;

std::string warning;
bool warning_is_user_visible = false;
switch (security_info.security_level) {
case security_state::SecurityStateModel::HTTP_SHOW_WARNING:
warning =
"This page includes a password or credit card input in a non-secure "
"context. A warning has been added to the URL bar. For more "
"information, see https://goo.gl/zmWq3m.";
warning_is_user_visible = true;
break;
case security_state::SecurityStateModel::NONE:
case security_state::SecurityStateModel::DANGEROUS:
warning =
"This page includes a password or credit card input in a non-secure "
"context. A warning will be added to the URL bar in Chrome 56 (Jan "
Expand All @@ -358,6 +361,8 @@ void ChromeSecurityStateModelClient::VisibleSecurityStateChanged() {
logged_http_warning_on_current_navigation_ = true;
web_contents_->GetMainFrame()->AddMessageToConsole(
content::CONSOLE_MESSAGE_LEVEL_WARNING, warning);
UMA_HISTOGRAM_BOOLEAN("Security.HTTPBad.UserWarnedAboutSensitiveInput",
warning_is_user_visible);
}

void ChromeSecurityStateModelClient::DidFinishNavigation(
Expand Down
83 changes: 83 additions & 0 deletions chrome/browser/ssl/chrome_security_state_model_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@

#include "chrome/browser/ssl/chrome_security_state_model_client.h"

#include "base/command_line.h"
#include "base/test/histogram_tester.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "components/security_state/security_state_model.h"
#include "components/security_state/switches.h"
#include "content/public/browser/security_style_explanation.h"
#include "content/public/browser/security_style_explanations.h"
#include "net/cert/cert_status_flags.h"
Expand All @@ -14,6 +18,9 @@

namespace {

const char kHTTPBadHistogram[] =
"Security.HTTPBad.UserWarnedAboutSensitiveInput";

// Tests that SecurityInfo flags for subresources with certificate
// errors are reflected in the SecurityStyleExplanations produced by
// ChromeSecurityStateModelClient.
Expand Down Expand Up @@ -242,4 +249,80 @@ TEST(ChromeSecurityStateModelClientTest, HTTPWarningInFuture) {
EXPECT_EQ(1u, explanations.info_explanations.size());
}

class ChromeSecurityStateModelClientHistogramTest
: public ChromeRenderViewHostTestHarness {
public:
ChromeSecurityStateModelClientHistogramTest() {}
~ChromeSecurityStateModelClientHistogramTest() override {}

void SetUp() override {
ChromeRenderViewHostTestHarness::SetUp();

ChromeSecurityStateModelClient::CreateForWebContents(web_contents());
client_ = ChromeSecurityStateModelClient::FromWebContents(web_contents());
navigate_to_http();
}

protected:
ChromeSecurityStateModelClient* client() { return client_; }

void signal_password() {
web_contents()->OnPasswordInputShownOnHttp();
client_->VisibleSecurityStateChanged();
}

void navigate_to_http() { NavigateAndCommit(GURL("http://example.test")); }

private:
ChromeSecurityStateModelClient* client_;
DISALLOW_COPY_AND_ASSIGN(ChromeSecurityStateModelClientHistogramTest);
};

// Tests that UMA logs the omnibox warning when security level is
// HTTP_SHOW_WARNING.
TEST_F(ChromeSecurityStateModelClientHistogramTest,
HTTPOmniboxWarningHistogram) {
// Show Warning Chip.
base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
security_state::switches::kMarkHttpAs,
security_state::switches::kMarkHttpWithPasswordsOrCcWithChip);

base::HistogramTester histograms;
signal_password();
histograms.ExpectUniqueSample(kHTTPBadHistogram, true, 1);

// Fire again and ensure no sample is recorded.
signal_password();
histograms.ExpectUniqueSample(kHTTPBadHistogram, true, 1);

// Navigate to a new page and ensure a sample is recorded.
navigate_to_http();
histograms.ExpectUniqueSample(kHTTPBadHistogram, true, 1);
signal_password();
histograms.ExpectUniqueSample(kHTTPBadHistogram, true, 2);
}

// Tests that UMA logs the console warning when security level is NONE.
TEST_F(ChromeSecurityStateModelClientHistogramTest,
HTTPConsoleWarningHistogram) {
// Show Neutral for HTTP
base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
security_state::switches::kMarkHttpAs,
security_state::switches::kMarkHttpAsNeutral);

base::HistogramTester histograms;
signal_password();
histograms.ExpectUniqueSample(kHTTPBadHistogram, false, 1);

// Fire again and ensure no sample is recorded.
signal_password();
histograms.ExpectUniqueSample(kHTTPBadHistogram, false, 1);

// Navigate to a new page and ensure a sample is recorded.
navigate_to_http();
histograms.ExpectUniqueSample(kHTTPBadHistogram, false, 1);
signal_password();
histograms.ExpectUniqueSample(kHTTPBadHistogram, false, 2);
}

} // namespace
11 changes: 11 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56155,6 +56155,17 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>

<histogram name="Security.HTTPBad.UserWarnedAboutSensitiveInput"
enum="BooleanShown">
<owner>elawrence@chromium.org</owner>
<owner>estark@chromium.org</owner>
<summary>
Whether a &quot;Not Secure&quot; warning was shown in the omnibox because a
security-sensitive form field was rendered in a non-secure context. Logged
at most once per main-frame navigation.
</summary>
</histogram>

<histogram name="Security.PageInfo.Action.HttpsUrl.Dangerous"
enum="WebsiteSettingsAction">
<owner>estark@chromium.org</owner>
Expand Down

0 comments on commit c346a85

Please sign in to comment.