Commit 15e2549e authored by Mustafa Emre Acer's avatar Mustafa Emre Acer Committed by Commit Bot

Add URL display experiment info to password protection request pings

This CL expands the PhishGuard ping proto and adds a new field to
LoginReputationClientRequest containing information about the Delayed
Warnings experiment and Simplified URL Display experiments.

Note that the presubmit gives a reminder to update
chrome/common/extensions/api/safe_browsing_private.idl. Since this isn't
an API change, this CL doesn't modify this file.

Bug: 1114922
Change-Id: I118c74c2b7ef2b0bd1ea18c091575540cc464dd8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347975
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: default avatarDaniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800742}
parent 8cb49565
...@@ -38,6 +38,7 @@ ...@@ -38,6 +38,7 @@
#include "chrome/common/url_constants.h" #include "chrome/common/url_constants.h"
#include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/google/core/common/google_util.h" #include "components/google/core/common/google_util.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/password_manager/core/browser/compromised_credentials_table.h" #include "components/password_manager/core/browser/compromised_credentials_table.h"
#include "components/password_manager/core/browser/form_parsing/form_parser.h" #include "components/password_manager/core/browser/form_parsing/form_parser.h"
#include "components/password_manager/core/browser/leak_detection_dialog_utils.h" #include "components/password_manager/core/browser/leak_detection_dialog_utils.h"
...@@ -353,6 +354,24 @@ bool ChromePasswordProtectionService::ShouldShowPasswordReusePageInfoBubble( ...@@ -353,6 +354,24 @@ bool ChromePasswordProtectionService::ShouldShowPasswordReusePageInfoBubble(
: false; : false;
} }
safe_browsing::LoginReputationClientRequest::UrlDisplayExperiment
ChromePasswordProtectionService::GetUrlDisplayExperiment() const {
safe_browsing::LoginReputationClientRequest::UrlDisplayExperiment experiment;
// Delayed warnings parameters:
experiment.set_delayed_warnings_enabled(
base::FeatureList::IsEnabled(safe_browsing::kDelayedWarnings));
experiment.set_delayed_warnings_mouse_clicks_enabled(
safe_browsing::kDelayedWarningsEnableMouseClicks.Get());
// Actual URL display experiments:
experiment.set_reveal_on_hover(base::FeatureList::IsEnabled(
omnibox::kRevealSteadyStateUrlPathQueryAndRefOnHover));
experiment.set_hide_on_interaction(base::FeatureList::IsEnabled(
omnibox::kHideSteadyStateUrlPathQueryAndRefOnInteraction));
experiment.set_elide_to_registrable_domain(
base::FeatureList::IsEnabled(omnibox::kMaybeElideToRegistrableDomain));
return experiment;
}
void ChromePasswordProtectionService::ShowModalWarning( void ChromePasswordProtectionService::ShowModalWarning(
content::WebContents* web_contents, content::WebContents* web_contents,
RequestOutcome outcome, RequestOutcome outcome,
......
...@@ -250,6 +250,8 @@ class ChromePasswordProtectionService : public PasswordProtectionService { ...@@ -250,6 +250,8 @@ class ChromePasswordProtectionService : public PasswordProtectionService {
// |NOT_SIGNED_IN|. // |NOT_SIGNED_IN|.
LoginReputationClientRequest::PasswordReuseEvent::SyncAccountType LoginReputationClientRequest::PasswordReuseEvent::SyncAccountType
GetSyncAccountType() const override; GetSyncAccountType() const override;
safe_browsing::LoginReputationClientRequest::UrlDisplayExperiment
GetUrlDisplayExperiment() const override;
// Stores |verdict| in the cache based on its |trigger_type|, |url|, // Stores |verdict| in the cache based on its |trigger_type|, |url|,
// reused |password_type|, |verdict| and |receive_time|. // reused |password_type|, |verdict| and |receive_time|.
......
...@@ -174,6 +174,11 @@ class MockChromePasswordProtectionService ...@@ -174,6 +174,11 @@ class MockChromePasswordProtectionService
return account_info_; return account_info_;
} }
safe_browsing::LoginReputationClientRequest::UrlDisplayExperiment
GetUrlDisplayExperiment() const override {
return safe_browsing::LoginReputationClientRequest::UrlDisplayExperiment();
}
// Configures the results returned by IsExtendedReporting(), IsIncognito(), // Configures the results returned by IsExtendedReporting(), IsIncognito(),
// and IsHistorySyncEnabled(). // and IsHistorySyncEnabled().
void ConfigService(bool is_incognito, bool is_extended_reporting) { void ConfigService(bool is_incognito, bool is_extended_reporting) {
......
...@@ -27,11 +27,6 @@ const char kPreventElisionExtensionId[] = "jknemblkbdhdcpllfgbfekkdciegfboi"; ...@@ -27,11 +27,6 @@ const char kPreventElisionExtensionId[] = "jknemblkbdhdcpllfgbfekkdciegfboi";
} // namespace } // namespace
namespace safe_browsing { namespace safe_browsing {
// If true, a delayed warning will be shown when the user clicks on the page.
// If false, the warning won't be shown, but a metric will be recorded on the
// first click.
const base::FeatureParam<bool> kEnableMouseClicks{&kDelayedWarnings, "mouse",
/*default_value=*/false};
const char kDelayedWarningsHistogram[] = "SafeBrowsing.DelayedWarnings.Event"; const char kDelayedWarningsHistogram[] = "SafeBrowsing.DelayedWarnings.Event";
...@@ -297,7 +292,7 @@ bool SafeBrowsingUserInteractionObserver::HandleMouseEvent( ...@@ -297,7 +292,7 @@ bool SafeBrowsingUserInteractionObserver::HandleMouseEvent(
} }
// If warning isn't enabled for mouse clicks, still record the first time when // If warning isn't enabled for mouse clicks, still record the first time when
// the user clicks. // the user clicks.
if (!kEnableMouseClicks.Get()) { if (!kDelayedWarningsEnableMouseClicks.Get()) {
if (!mouse_click_with_no_warning_recorded_) { if (!mouse_click_with_no_warning_recorded_) {
RecordUMA(DelayedWarningEvent::kWarningNotTriggeredOnMouseClick); RecordUMA(DelayedWarningEvent::kWarningNotTriggeredOnMouseClick);
mouse_click_with_no_warning_recorded_ = true; mouse_click_with_no_warning_recorded_ = true;
......
...@@ -24,6 +24,10 @@ class MockPasswordProtectionService : public PasswordProtectionService { ...@@ -24,6 +24,10 @@ class MockPasswordProtectionService : public PasswordProtectionService {
MOCK_CONST_METHOD0(GetSyncAccountType, MOCK_CONST_METHOD0(GetSyncAccountType,
safe_browsing::LoginReputationClientRequest:: safe_browsing::LoginReputationClientRequest::
PasswordReuseEvent::SyncAccountType()); PasswordReuseEvent::SyncAccountType());
MOCK_CONST_METHOD0(
GetUrlDisplayExperiment,
safe_browsing::LoginReputationClientRequest::UrlDisplayExperiment());
MOCK_CONST_METHOD0(GetBrowserPolicyConnector, MOCK_CONST_METHOD0(GetBrowserPolicyConnector,
const policy::BrowserPolicyConnector*()); const policy::BrowserPolicyConnector*());
MOCK_CONST_METHOD0(GetCurrentContentAreaSize, gfx::Size()); MOCK_CONST_METHOD0(GetCurrentContentAreaSize, gfx::Size());
......
#include "base/containers/flat_set.h" // Copyright 2017 The Chromium Authors. All rights reserved. // Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
...@@ -132,6 +132,8 @@ PasswordProtectionRequest::PasswordProtectionRequest( ...@@ -132,6 +132,8 @@ PasswordProtectionRequest::PasswordProtectionRequest(
password_type_ != PasswordType::SAVED_PASSWORD || password_type_ != PasswordType::SAVED_PASSWORD ||
!matching_reused_credentials_.empty()); !matching_reused_credentials_.empty());
request_proto_->set_trigger_type(trigger_type_); request_proto_->set_trigger_type(trigger_type_);
*request_proto_->mutable_url_display_experiment() =
pps->GetUrlDisplayExperiment();
} }
PasswordProtectionRequest::~PasswordProtectionRequest() { PasswordProtectionRequest::~PasswordProtectionRequest() {
......
...@@ -423,6 +423,11 @@ class PasswordProtectionService : public history::HistoryServiceObserver { ...@@ -423,6 +423,11 @@ class PasswordProtectionService : public history::HistoryServiceObserver {
virtual LoginReputationClientRequest::PasswordReuseEvent::SyncAccountType virtual LoginReputationClientRequest::PasswordReuseEvent::SyncAccountType
GetSyncAccountType() const = 0; GetSyncAccountType() const = 0;
// Get information about Delayed Warnings and Omnibox URL display experiments.
// This information is sent in PhishGuard pings.
virtual LoginReputationClientRequest::UrlDisplayExperiment
GetUrlDisplayExperiment() const = 0;
// Returns the reason why a ping is not sent based on the |trigger_type|, // Returns the reason why a ping is not sent based on the |trigger_type|,
// |url| and |password_type|. Crash if |CanSendPing| is true. // |url| and |password_type|. Crash if |CanSendPing| is true.
virtual RequestOutcome GetPingNotSentReason( virtual RequestOutcome GetPingNotSentReason(
......
...@@ -1175,6 +1175,20 @@ base::Value SerializeDomFeatures(const DomFeatures& dom_features) { ...@@ -1175,6 +1175,20 @@ base::Value SerializeDomFeatures(const DomFeatures& dom_features) {
return std::move(dom_features_dict); return std::move(dom_features_dict);
} }
base::Value SerializeUrlDisplayExperiment(
const LoginReputationClientRequest::UrlDisplayExperiment& experiment) {
base::DictionaryValue d;
d.SetBoolean("delayed_warnings_enabled",
experiment.delayed_warnings_enabled());
d.SetBoolean("delayed_warnings_mouse_clicks_enabled",
experiment.delayed_warnings_mouse_clicks_enabled());
d.SetBoolean("reveal_on_hover", experiment.reveal_on_hover());
d.SetBoolean("hide_on_interaction", experiment.hide_on_interaction());
d.SetBoolean("elide_to_registrable_domain",
experiment.elide_to_registrable_domain());
return std::move(d);
}
std::string SerializePGPing(const LoginReputationClientRequest& request) { std::string SerializePGPing(const LoginReputationClientRequest& request) {
base::DictionaryValue request_dict; base::DictionaryValue request_dict;
...@@ -1225,6 +1239,12 @@ std::string SerializePGPing(const LoginReputationClientRequest& request) { ...@@ -1225,6 +1239,12 @@ std::string SerializePGPing(const LoginReputationClientRequest& request) {
SerializeDomFeatures(request.dom_features())); SerializeDomFeatures(request.dom_features()));
} }
if (request.has_url_display_experiment()) {
request_dict.SetKey(
"url_display_experiment",
SerializeUrlDisplayExperiment(request.url_display_experiment()));
}
std::string request_serialized; std::string request_serialized;
JSONStringValueSerializer serializer(&request_serialized); JSONStringValueSerializer serializer(&request_serialized);
serializer.set_pretty_print(true); serializer.set_pretty_print(true);
......
...@@ -43,6 +43,13 @@ const base::Feature kContentComplianceEnabled{ ...@@ -43,6 +43,13 @@ const base::Feature kContentComplianceEnabled{
const base::Feature kDelayedWarnings{"SafeBrowsingDelayedWarnings", const base::Feature kDelayedWarnings{"SafeBrowsingDelayedWarnings",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
// If true, a delayed warning will be shown when the user clicks on the page.
// If false, the warning won't be shown, but a metric will be recorded on the
// first click.
const base::FeatureParam<bool> kDelayedWarningsEnableMouseClicks{
&kDelayedWarnings, "mouse",
/*default_value=*/false};
const base::Feature kDownloadRequestWithToken{ const base::Feature kDownloadRequestWithToken{
"SafeBrowsingDownloadRequestWithToken", base::FEATURE_ENABLED_BY_DEFAULT}; "SafeBrowsingDownloadRequestWithToken", base::FEATURE_ENABLED_BY_DEFAULT};
......
...@@ -138,6 +138,9 @@ extern const base::Feature kUseNewDownloadWarnings; ...@@ -138,6 +138,9 @@ extern const base::Feature kUseNewDownloadWarnings;
// Controls whether the delayed warning experiment is enabled. // Controls whether the delayed warning experiment is enabled.
extern const base::Feature kDelayedWarnings; extern const base::Feature kDelayedWarnings;
// True if mouse clicks should undelay the warnings immediately when delayed
// warnings feature is enabled.
extern const base::FeatureParam<bool> kDelayedWarningsEnableMouseClicks;
base::ListValue GetFeatureStatusList(); base::ListValue GetFeatureStatusList();
......
...@@ -351,6 +351,23 @@ message LoginReputationClientRequest { ...@@ -351,6 +351,23 @@ message LoginReputationClientRequest {
// Whether the ping sent to Safe Browsing is a normal (full request) ping or a // Whether the ping sent to Safe Browsing is a normal (full request) ping or a
// sample ping for URLs on the allow-list. // sample ping for URLs on the allow-list.
optional ReportType report_type = 13; optional ReportType report_type = 13;
// Information about Simplified URL display experiments in Chrome.
// See go/phishguard-ping-spoof-detection
message UrlDisplayExperiment {
// True if SafeBrowsingDelayedWarnings is enabled.
optional bool delayed_warnings_enabled = 1;
// True if the "mouse" parameter of SafeBrowsingDelayedWarnings is enabled.
optional bool delayed_warnings_mouse_clicks_enabled = 2;
// True if RevealSteadyStateUrlPathQueryAndRefOnHover is enabled.
optional bool reveal_on_hover = 3;
// True if HideSteadyStateUrlPathQueryAndRefOnInteraction is enabled.
optional bool hide_on_interaction = 4;
// True if OmniboxUIExperimentElideToRegistrableDomain is enabled.
optional bool elide_to_registrable_domain = 5;
};
optional UrlDisplayExperiment url_display_experiment = 14;
} }
// The message is used for client response for login reputation requests. // The message is used for client response for login reputation requests.
......
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