Commit 8f6bc5e3 authored by Jonathan Mengedoht's avatar Jonathan Mengedoht Committed by Commit Bot

Add UKMs to WellKnownChangePasswordTabHelper

Bug: 927473
Change-Id: Ib87706aeb8c6e100ba542a31e7374b2fe8c12210
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2372325
Commit-Queue: Jonathan Mengedoht <mengedoht@google.com>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801331}
parent 61a23e66
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "chrome/browser/password_manager/change_password_url_service_factory.h" #include "chrome/browser/password_manager/change_password_url_service_factory.h"
#include "components/password_manager/core/browser/change_password_url_service.h" #include "components/password_manager/core/browser/change_password_url_service.h"
#include "components/password_manager/core/browser/well_known_change_password_state.h" #include "components/password_manager/core/browser/well_known_change_password_state.h"
#include "components/password_manager/core/browser/well_known_change_password_util.h"
#include "components/password_manager/core/common/password_manager_features.h" #include "components/password_manager/core/common/password_manager_features.h"
#include "components/ukm/content/source_url_recorder.h" #include "components/ukm/content/source_url_recorder.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
...@@ -29,6 +28,7 @@ using content::NavigationHandle; ...@@ -29,6 +28,7 @@ using content::NavigationHandle;
using content::NavigationThrottle; using content::NavigationThrottle;
using content::WebContents; using content::WebContents;
using password_manager::IsWellKnownChangePasswordUrl; using password_manager::IsWellKnownChangePasswordUrl;
using password_manager::WellKnownChangePasswordResult;
using password_manager::WellKnownChangePasswordState; using password_manager::WellKnownChangePasswordState;
// Used to scope the posted navigation task to the lifetime of |web_contents|. // Used to scope the posted navigation task to the lifetime of |web_contents|.
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "content/public/browser/navigation_throttle.h" #include "content/public/browser/navigation_throttle.h"
#include "components/password_manager/core/browser/well_known_change_password_state.h" #include "components/password_manager/core/browser/well_known_change_password_state.h"
#include "components/password_manager/core/browser/well_known_change_password_util.h"
#include "services/metrics/public/cpp/ukm_source_id.h" #include "services/metrics/public/cpp/ukm_source_id.h"
class GURL; class GURL;
...@@ -22,15 +23,6 @@ namespace password_manager { ...@@ -22,15 +23,6 @@ namespace password_manager {
class ChangePasswordUrlService; class ChangePasswordUrlService;
} // namespace password_manager } // namespace password_manager
// Used to report UKMs about the support for .well-known/change-password.
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class WellKnownChangePasswordResult {
kFallbackToOriginUrl = 0,
kFallbackToOverrideUrl = 1,
kUsedWellKnownChangePassword = 2,
};
// This NavigationThrottle checks whether a site supports the // This NavigationThrottle checks whether a site supports the
// .well-known/change-password url. To check whether a site supports the // .well-known/change-password url. To check whether a site supports the
// change-password url, we also request a .well-known path that is defined to // change-password url, we also request a .well-known path that is defined to
...@@ -63,7 +55,7 @@ class WellKnownChangePasswordNavigationThrottle ...@@ -63,7 +55,7 @@ class WellKnownChangePasswordNavigationThrottle
// Redirects to a given URL in the same tab. // Redirects to a given URL in the same tab.
void Redirect(const GURL& url); void Redirect(const GURL& url);
// Records the given UKM metric. // Records the given UKM metric.
void RecordMetric(WellKnownChangePasswordResult result); void RecordMetric(password_manager::WellKnownChangePasswordResult result);
password_manager::WellKnownChangePasswordState password_manager::WellKnownChangePasswordState
well_known_change_password_state_{this}; well_known_change_password_state_{this};
......
...@@ -44,6 +44,7 @@ using net::test_server::HttpRequest; ...@@ -44,6 +44,7 @@ using net::test_server::HttpRequest;
using net::test_server::HttpResponse; using net::test_server::HttpResponse;
using password_manager::kWellKnownChangePasswordPath; using password_manager::kWellKnownChangePasswordPath;
using password_manager::kWellKnownNotExistingResourcePath; using password_manager::kWellKnownNotExistingResourcePath;
using password_manager::WellKnownChangePasswordResult;
constexpr char kMockChangePasswordPath[] = "/change-password-override"; constexpr char kMockChangePasswordPath[] = "/change-password-override";
......
...@@ -11,6 +11,15 @@ class GURL; ...@@ -11,6 +11,15 @@ class GURL;
namespace password_manager { namespace password_manager {
// Used to report UKMs about the support for .well-known/change-password.
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class WellKnownChangePasswordResult {
kFallbackToOriginUrl = 0,
kFallbackToOverrideUrl = 1,
kUsedWellKnownChangePassword = 2,
};
// Path for Well-Known change password url // Path for Well-Known change password url
// Spec: https://wicg.github.io/change-password-url/ // Spec: https://wicg.github.io/change-password-url/
extern const char kWellKnownChangePasswordPath[]; extern const char kWellKnownChangePasswordPath[];
......
...@@ -197,6 +197,7 @@ source_set("unit_tests") { ...@@ -197,6 +197,7 @@ source_set("unit_tests") {
"//components/prefs:test_support", "//components/prefs:test_support",
"//components/safe_browsing/core/common:safe_browsing_prefs", "//components/safe_browsing/core/common:safe_browsing_prefs",
"//components/security_state/ios", "//components/security_state/ios",
"//components/ukm:test_support",
"//google_apis", "//google_apis",
"//ios/chrome/browser/autofill", "//ios/chrome/browser/autofill",
"//ios/chrome/browser/browser_state:test_support", "//ios/chrome/browser/browser_state:test_support",
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "components/password_manager/core/browser/change_password_url_service.h" #include "components/password_manager/core/browser/change_password_url_service.h"
#include "components/password_manager/core/browser/well_known_change_password_state.h" #include "components/password_manager/core/browser/well_known_change_password_state.h"
#include "components/password_manager/core/browser/well_known_change_password_util.h"
#include "ios/web/public/navigation/web_state_policy_decider.h" #include "ios/web/public/navigation/web_state_policy_decider.h"
#include "ios/web/public/web_state_observer.h" #include "ios/web/public/web_state_observer.h"
#import "ios/web/public/web_state_user_data.h" #import "ios/web/public/web_state_user_data.h"
...@@ -67,6 +68,8 @@ class WellKnownChangePasswordTabHelper ...@@ -67,6 +68,8 @@ class WellKnownChangePasswordTabHelper
void OnProcessingFinished(bool is_supported) override; void OnProcessingFinished(bool is_supported) override;
// Redirects to a given URL in the same tab. // Redirects to a given URL in the same tab.
void Redirect(const GURL& url); void Redirect(const GURL& url);
// Records the given UKM metric.
void RecordMetric(WellKnownChangePasswordResult result);
web::WebState* web_state_ = nullptr; web::WebState* web_state_ = nullptr;
ProcessingState processing_state_ = kWaitingForFirstRequest; ProcessingState processing_state_ = kWaitingForFirstRequest;
......
...@@ -7,12 +7,14 @@ ...@@ -7,12 +7,14 @@
#import <Foundation/Foundation.h> #import <Foundation/Foundation.h>
#include "base/logging.h" #include "base/logging.h"
#include "components/password_manager/core/browser/well_known_change_password_util.h"
#include "components/password_manager/core/common/password_manager_features.h" #include "components/password_manager/core/common/password_manager_features.h"
#import "components/ukm/ios/ukm_url_recorder.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h" #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/passwords/ios_chrome_change_password_url_service_factory.h" #include "ios/chrome/browser/passwords/ios_chrome_change_password_url_service_factory.h"
#import "ios/web/public/navigation/navigation_context.h" #import "ios/web/public/navigation/navigation_context.h"
#import "net/base/mac/url_conversions.h" #import "net/base/mac/url_conversions.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
...@@ -124,12 +126,19 @@ void WellKnownChangePasswordTabHelper::OnProcessingFinished(bool is_supported) { ...@@ -124,12 +126,19 @@ void WellKnownChangePasswordTabHelper::OnProcessingFinished(bool is_supported) {
if (is_supported) { if (is_supported) {
std::move(response_policy_callback_) std::move(response_policy_callback_)
.Run(web::WebStatePolicyDecider::PolicyDecision::Allow()); .Run(web::WebStatePolicyDecider::PolicyDecision::Allow());
RecordMetric(WellKnownChangePasswordResult::kUsedWellKnownChangePassword);
} else { } else {
std::move(response_policy_callback_) std::move(response_policy_callback_)
.Run(web::WebStatePolicyDecider::PolicyDecision::Cancel()); .Run(web::WebStatePolicyDecider::PolicyDecision::Cancel());
GURL redirect_url = GURL redirect_url =
change_password_url_service_->GetChangePasswordUrl(request_url_); change_password_url_service_->GetChangePasswordUrl(request_url_);
Redirect(redirect_url.is_valid() ? redirect_url : request_url_.GetOrigin()); if (redirect_url.is_valid()) {
RecordMetric(WellKnownChangePasswordResult::kFallbackToOverrideUrl);
Redirect(redirect_url);
} else {
RecordMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
Redirect(request_url_.GetOrigin());
}
} }
} }
...@@ -143,4 +152,12 @@ void WellKnownChangePasswordTabHelper::Redirect(const GURL& url) { ...@@ -143,4 +152,12 @@ void WellKnownChangePasswordTabHelper::Redirect(const GURL& url) {
} }
} }
void WellKnownChangePasswordTabHelper::RecordMetric(
WellKnownChangePasswordResult result) {
ukm::SourceId source_id = ukm::GetSourceIdForWebStateDocument(web_state_);
ukm::builders::PasswordManager_WellKnownChangePasswordResult(source_id)
.SetWellKnownChangePasswordResult(static_cast<int64_t>(result))
.Record(ukm::UkmRecorder::Get());
}
WEB_STATE_USER_DATA_KEY_IMPL(WellKnownChangePasswordTabHelper) WEB_STATE_USER_DATA_KEY_IMPL(WellKnownChangePasswordTabHelper)
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "components/password_manager/core/browser/well_known_change_password_util.h" #include "components/password_manager/core/browser/well_known_change_password_util.h"
#include "components/password_manager/core/common/password_manager_features.h" #include "components/password_manager/core/common/password_manager_features.h"
#include "components/ukm/test_ukm_recorder.h"
#include "ios/chrome/browser/passwords/ios_chrome_change_password_url_service_factory.h" #include "ios/chrome/browser/passwords/ios_chrome_change_password_url_service_factory.h"
#import "ios/web/public/navigation/navigation_manager.h" #import "ios/web/public/navigation/navigation_manager.h"
#import "ios/web/public/test/fakes/test_web_client.h" #import "ios/web/public/test/fakes/test_web_client.h"
...@@ -24,6 +25,7 @@ ...@@ -24,6 +25,7 @@
#include "net/test/embedded_test_server/embedded_test_server.h" #include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h" #include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h" #include "net/test/embedded_test_server/http_response.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h" #include "services/network/test/test_url_loader_factory.h"
...@@ -41,6 +43,7 @@ using net::test_server::HttpRequest; ...@@ -41,6 +43,7 @@ using net::test_server::HttpRequest;
using net::test_server::HttpResponse; using net::test_server::HttpResponse;
using password_manager::kWellKnownChangePasswordPath; using password_manager::kWellKnownChangePasswordPath;
using password_manager::kWellKnownNotExistingResourcePath; using password_manager::kWellKnownNotExistingResourcePath;
using password_manager::WellKnownChangePasswordResult;
// ServerResponse describes how a server should respond to a given path. // ServerResponse describes how a server should respond to a given path.
struct ServerResponse { struct ServerResponse {
...@@ -77,6 +80,8 @@ class TestChangePasswordUrlService ...@@ -77,6 +80,8 @@ class TestChangePasswordUrlService
class WellKnownChangePasswordTabHelperTest : public web::TestWebClient, class WellKnownChangePasswordTabHelperTest : public web::TestWebClient,
public web::WebTestWithWebState { public web::WebTestWithWebState {
public: public:
using UkmBuilder =
ukm::builders::PasswordManager_WellKnownChangePasswordResult;
WellKnownChangePasswordTabHelperTest() { WellKnownChangePasswordTabHelperTest() {
feature_list_.InitAndEnableFeature( feature_list_.InitAndEnableFeature(
password_manager::features::kWellKnownChangePassword); password_manager::features::kWellKnownChangePassword);
...@@ -104,6 +109,7 @@ class WellKnownChangePasswordTabHelperTest : public web::TestWebClient, ...@@ -104,6 +109,7 @@ class WellKnownChangePasswordTabHelperTest : public web::TestWebClient,
SetSharedURLLoaderFactory( SetSharedURLLoaderFactory(
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>( base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&test_url_loader_factory_)); &test_url_loader_factory_));
test_recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
} }
// Sets a response for the |test_url_loader_factory_| with the |test_server_| // Sets a response for the |test_url_loader_factory_| with the |test_server_|
...@@ -113,6 +119,15 @@ class WellKnownChangePasswordTabHelperTest : public web::TestWebClient, ...@@ -113,6 +119,15 @@ class WellKnownChangePasswordTabHelperTest : public web::TestWebClient,
test_url_loader_factory_.AddResponse(test_server_->GetURL(path).spec(), "", test_url_loader_factory_.AddResponse(test_server_->GetURL(path).spec(), "",
status_code); status_code);
} }
void ExpectUkmMetric(WellKnownChangePasswordResult expected) {
auto entries = test_recorder_->GetEntriesByName(UkmBuilder::kEntryName);
// Expect one recorded metric.
ASSERT_EQ(1, static_cast<int>(entries.size()));
test_recorder_->ExpectEntryMetric(
entries[0], UkmBuilder::kWellKnownChangePasswordResultName,
static_cast<int64_t>(expected));
}
// Waits until the navigation is complete and waits for backgroundtasks to // Waits until the navigation is complete and waits for backgroundtasks to
// complete. Returns false when timed out. // complete. Returns false when timed out.
bool WaitUntilLoaded(); bool WaitUntilLoaded();
...@@ -124,6 +139,7 @@ class WellKnownChangePasswordTabHelperTest : public web::TestWebClient, ...@@ -124,6 +139,7 @@ class WellKnownChangePasswordTabHelperTest : public web::TestWebClient,
std::unique_ptr<EmbeddedTestServer> test_server_ = std::unique_ptr<EmbeddedTestServer> test_server_ =
std::make_unique<EmbeddedTestServer>(); std::make_unique<EmbeddedTestServer>();
TestChangePasswordUrlService* url_service_ = nullptr; TestChangePasswordUrlService* url_service_ = nullptr;
std::unique_ptr<ukm::TestAutoSetUkmRecorder> test_recorder_;
private: private:
// Returns a response for the given request. Uses |path_response_map_| to // Returns a response for the given request. Uses |path_response_map_| to
...@@ -185,6 +201,7 @@ TEST_F(WellKnownChangePasswordTabHelperTest, SupportForChangePassword) { ...@@ -185,6 +201,7 @@ TEST_F(WellKnownChangePasswordTabHelperTest, SupportForChangePassword) {
test_server_->GetURL(kWellKnownChangePasswordPath)); test_server_->GetURL(kWellKnownChangePasswordPath));
ASSERT_TRUE(WaitUntilLoaded()); ASSERT_TRUE(WaitUntilLoaded());
EXPECT_EQ(GetNavigatedUrl().path(), kWellKnownChangePasswordPath); EXPECT_EQ(GetNavigatedUrl().path(), kWellKnownChangePasswordPath);
ExpectUkmMetric(WellKnownChangePasswordResult::kUsedWellKnownChangePassword);
} }
TEST_F(WellKnownChangePasswordTabHelperTest, TEST_F(WellKnownChangePasswordTabHelperTest,
...@@ -200,6 +217,7 @@ TEST_F(WellKnownChangePasswordTabHelperTest, ...@@ -200,6 +217,7 @@ TEST_F(WellKnownChangePasswordTabHelperTest,
test_server_->GetURL(kWellKnownChangePasswordPath)); test_server_->GetURL(kWellKnownChangePasswordPath));
ASSERT_TRUE(WaitUntilLoaded()); ASSERT_TRUE(WaitUntilLoaded());
EXPECT_EQ(GetNavigatedUrl().path(), "/change-password"); EXPECT_EQ(GetNavigatedUrl().path(), "/change-password");
ExpectUkmMetric(WellKnownChangePasswordResult::kUsedWellKnownChangePassword);
} }
TEST_F(WellKnownChangePasswordTabHelperTest, TEST_F(WellKnownChangePasswordTabHelperTest,
...@@ -212,6 +230,7 @@ TEST_F(WellKnownChangePasswordTabHelperTest, ...@@ -212,6 +230,7 @@ TEST_F(WellKnownChangePasswordTabHelperTest,
test_server_->GetURL(kWellKnownChangePasswordPath)); test_server_->GetURL(kWellKnownChangePasswordPath));
ASSERT_TRUE(WaitUntilLoaded()); ASSERT_TRUE(WaitUntilLoaded());
EXPECT_EQ(GetNavigatedUrl().path(), "/"); EXPECT_EQ(GetNavigatedUrl().path(), "/");
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
} }
TEST_F(WellKnownChangePasswordTabHelperTest, NoSupportForChangePassword_Ok) { TEST_F(WellKnownChangePasswordTabHelperTest, NoSupportForChangePassword_Ok) {
...@@ -223,6 +242,7 @@ TEST_F(WellKnownChangePasswordTabHelperTest, NoSupportForChangePassword_Ok) { ...@@ -223,6 +242,7 @@ TEST_F(WellKnownChangePasswordTabHelperTest, NoSupportForChangePassword_Ok) {
test_server_->GetURL(kWellKnownChangePasswordPath)); test_server_->GetURL(kWellKnownChangePasswordPath));
ASSERT_TRUE(WaitUntilLoaded()); ASSERT_TRUE(WaitUntilLoaded());
EXPECT_EQ(GetNavigatedUrl().path(), "/"); EXPECT_EQ(GetNavigatedUrl().path(), "/");
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
} }
TEST_F(WellKnownChangePasswordTabHelperTest, TEST_F(WellKnownChangePasswordTabHelperTest,
...@@ -235,6 +255,7 @@ TEST_F(WellKnownChangePasswordTabHelperTest, ...@@ -235,6 +255,7 @@ TEST_F(WellKnownChangePasswordTabHelperTest,
test_server_->GetURL(kWellKnownChangePasswordPath)); test_server_->GetURL(kWellKnownChangePasswordPath));
ASSERT_TRUE(WaitUntilLoaded()); ASSERT_TRUE(WaitUntilLoaded());
EXPECT_EQ(GetNavigatedUrl().path(), "/"); EXPECT_EQ(GetNavigatedUrl().path(), "/");
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
} }
TEST_F(WellKnownChangePasswordTabHelperTest, TEST_F(WellKnownChangePasswordTabHelperTest,
...@@ -248,4 +269,5 @@ TEST_F(WellKnownChangePasswordTabHelperTest, ...@@ -248,4 +269,5 @@ TEST_F(WellKnownChangePasswordTabHelperTest,
test_server_->GetURL(kWellKnownChangePasswordPath)); test_server_->GetURL(kWellKnownChangePasswordPath));
ASSERT_TRUE(WaitUntilLoaded()); ASSERT_TRUE(WaitUntilLoaded());
EXPECT_EQ(GetNavigatedUrl().path(), kMockChangePasswordPath); EXPECT_EQ(GetNavigatedUrl().path(), kMockChangePasswordPath);
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOverrideUrl);
} }
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