Skip to content

GitLab

T
tangled
Commit c8351891 authored by Jonathan Mengedoht's avatar Jonathan Mengedoht Committed by Commit Bot
Browse files
Options

Add UKMs to WellKnownChangePasswordNavigationThrottle 

Bug: 927473
Change-Id: Ia9fcf42d6394755cf1042e0d51f4352185e8a972
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366593
Commit-Queue: Jonathan Mengedoht <mengedoht@google.com>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800324}
parent bff2440e
Hide whitespace changes
Inline Side-by-side
Showing with 95 additions and 5 deletions
chrome/browser/ui/passwords/well_known_change_password_navigation_throttle.cc
View file @ c8351891
......@@ -10,12 +10,15 @@
#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/ukm/content/source_url_recorder.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/page_navigator.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_user_data.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 "url/gurl.h"
#include "url/origin.h"
......@@ -79,7 +82,9 @@ WellKnownChangePasswordNavigationThrottle::
: NavigationThrottle(handle),
change_password_url_service_(
ChangePasswordUrlServiceFactory::GetForBrowserContext(
handle->GetWebContents()->GetBrowserContext())) {
handle->GetWebContents()->GetBrowserContext())),
source_id_(
ukm::GetSourceIdForWebContentsDocument(handle->GetWebContents())) {
change_password_url_service_->PrefetchURLs();
}
......@@ -125,13 +130,20 @@ const char* WellKnownChangePasswordNavigationThrottle::GetNameForLogging() {
void WellKnownChangePasswordNavigationThrottle::OnProcessingFinished(
bool is_supported) {
if (is_supported) {
RecordMetric(WellKnownChangePasswordResult::kUsedWellKnownChangePassword);
Resume();
return;
}
GURL url = navigation_handle()->GetURL();
GURL redirect_url = change_password_url_service_->GetChangePasswordUrl(url);
if (redirect_url.is_valid()) {
RecordMetric(WellKnownChangePasswordResult::kFallbackToOverrideUrl);
Redirect(redirect_url);
} else {
GURL url = navigation_handle()->GetURL();
GURL redirect_url = change_password_url_service_->GetChangePasswordUrl(url);
Redirect(redirect_url.is_valid() ? redirect_url : url.GetOrigin());
CancelDeferredNavigation(NavigationThrottle::CANCEL);
RecordMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
Redirect(url.GetOrigin());
}
CancelDeferredNavigation(NavigationThrottle::CANCEL);
}
void WellKnownChangePasswordNavigationThrottle::Redirect(const GURL& url) {
......@@ -152,3 +164,9 @@ void WellKnownChangePasswordNavigationThrottle::Redirect(const GURL& url) {
helper->GetWeakPtr(), std::move(params)));
}
void WellKnownChangePasswordNavigationThrottle::RecordMetric(
WellKnownChangePasswordResult result) {
ukm::builders::PasswordManager_WellKnownChangePasswordResult(source_id_)
.SetWellKnownChangePasswordResult(static_cast<int64_t>(result))
.Record(ukm::UkmRecorder::Get());
}
chrome/browser/ui/passwords/well_known_change_password_navigation_throttle.h
View file @ c8351891
......@@ -10,6 +10,7 @@
#include "content/public/browser/navigation_throttle.h"
#include "components/password_manager/core/browser/well_known_change_password_state.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
class GURL;
namespace content {
......@@ -21,6 +22,15 @@ namespace password_manager {
class ChangePasswordUrlService;
} // 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
// .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
......@@ -52,10 +62,13 @@ class WellKnownChangePasswordNavigationThrottle
void OnProcessingFinished(bool is_supported) override;
// Redirects to a given URL in the same tab.
void Redirect(const GURL& url);
// Records the given UKM metric.
void RecordMetric(WellKnownChangePasswordResult result);
password_manager::WellKnownChangePasswordState
well_known_change_password_state_{this};
password_manager::ChangePasswordUrlService* change_password_url_service_;
ukm::SourceId source_id_ = ukm::kInvalidSourceId;
};
#endif // CHROME_BROWSER_UI_PASSWORDS_WELL_KNOWN_CHANGE_PASSWORD_NAVIGATION_THROTTLE_H_
chrome/browser/ui/passwords/well_known_change_password_navigation_throttle_browsertest.cc
View file @ c8351891
......@@ -19,6 +19,7 @@
#include "components/password_manager/core/browser/change_password_url_service_impl.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/ukm/test_ukm_recorder.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/mock_navigation_handle.h"
#include "content/public/test/test_navigation_observer.h"
......@@ -28,6 +29,7 @@
#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_response.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
......@@ -86,6 +88,8 @@ class WellKnownChangePasswordNavigationThrottleBrowserTest
: public CertVerifierBrowserTest,
public testing::WithParamInterface<ResponseDelayParams> {
public:
using UkmBuilder =
ukm::builders::PasswordManager_WellKnownChangePasswordResult;
WellKnownChangePasswordNavigationThrottleBrowserTest() {
feature_list_.InitAndEnableFeature(
password_manager::features::kWellKnownChangePassword);
......@@ -104,8 +108,20 @@ class WellKnownChangePasswordNavigationThrottleBrowserTest
base::BindRepeating([](content::BrowserContext* context) {
return std::make_unique<TestChangePasswordUrlService>();
}));
test_recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
}
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));
}
ukm::TestAutoSetUkmRecorder* test_recorder() { return test_recorder_.get(); }
protected:
// Navigates to |kWellKnownChangePasswordPath| from the mock server. It waits
// until the navigation to |expected_path| happened.
......@@ -133,6 +149,7 @@ class WellKnownChangePasswordNavigationThrottleBrowserTest
std::unique_ptr<HttpResponse> HandleRequest(const HttpRequest& request);
base::test::ScopedFeatureList feature_list_;
std::unique_ptr<ukm::TestAutoSetUkmRecorder> test_recorder_;
};
std::unique_ptr<HttpResponse>
......@@ -179,6 +196,7 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
net::HTTP_NOT_FOUND, {}, response_delays.not_exist_delay};
TestNavigationThrottle(kWellKnownChangePasswordPath);
ExpectUkmMetric(WellKnownChangePasswordResult::kUsedWellKnownChangePassword);
}
IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
......@@ -193,6 +211,7 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
path_response_map_["/change-password"] = {net::HTTP_OK, {}, 0};
TestNavigationThrottle(/*expected_path=*/"/change-password");
ExpectUkmMetric(WellKnownChangePasswordResult::kUsedWellKnownChangePassword);
}
IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
......@@ -204,6 +223,7 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
net::HTTP_NOT_FOUND, {}, response_delays.not_exist_delay};
TestNavigationThrottle(/*expected_path=*/kWellKnownChangePasswordPath);
ExpectUkmMetric(WellKnownChangePasswordResult::kUsedWellKnownChangePassword);
}
IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
......@@ -221,6 +241,7 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
path_response_map_["/not-found"] = {net::HTTP_NOT_FOUND, {}, 0};
TestNavigationThrottle(/*expected_path=*/"/change-password");
ExpectUkmMetric(WellKnownChangePasswordResult::kUsedWellKnownChangePassword);
}
IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
......@@ -232,6 +253,7 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
net::HTTP_NOT_FOUND, {}, response_delays.not_exist_delay};
TestNavigationThrottle(/*expected_path=*/"/");
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
}
IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
......@@ -244,6 +266,7 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
net::HTTP_NOT_FOUND, {}, response_delays.not_exist_delay};
TestNavigationThrottle(/*expected_path=*/kMockChangePasswordPath);
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOverrideUrl);
}
// Single page applications often return 200 for all paths
......@@ -256,6 +279,7 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
net::HTTP_OK, {}, response_delays.not_exist_delay};
TestNavigationThrottle(/*expected_path=*/"/");
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
}
IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
......@@ -272,6 +296,7 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
path_response_map_["/not-found"] = {net::HTTP_NOT_FOUND, {}, 0};
TestNavigationThrottle(/*expected_path=*/"/");
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
}
IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
......@@ -300,6 +325,9 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
observer.Wait();
EXPECT_EQ(observer.last_navigation_url(), url);
// Expect no UKMs saved.
EXPECT_TRUE(
test_recorder()->GetEntriesByName(UkmBuilder::kEntryName).empty());
}
constexpr ResponseDelayParams kDelayParams[] = {{0, 1}, {1, 0}};
......
tools/metrics/histograms/enums.xml
View file @ c8351891
......@@ -74996,6 +74996,12 @@ Called by update_scheduler_enums.py.-->
<int value="5" label="User clicked the 'Sign in' button"/>
</enum>
<enum name="WellKnownChangePasswordResult">
<int value="0" label="Fallback to Origin"/>
<int value="1" label="Fallback to Url Override"/>
<int value="2" label="Well-Known supported"/>
</enum>
<enum name="WhitelistedDownloadType">
<int value="0" label="Does not match any whitelists"/>
<int value="1" label="URL whitelist"/>
tools/metrics/ukm/ukm.xml
View file @ c8351891
......@@ -9621,6 +9621,31 @@ be describing additional metrics about the same event.
</metric>
</event>
<event name="PasswordManager.WellKnownChangePasswordResult">
<owner>jdoerrie@chromium.org</owner>
<owner>vasilii@chromium.org</owner>
<summary>
Metrics related to the usage and support of .well-known/change-password. The
.well-known/change-password standard is defined to redirect to the sites
change password form. We collect whether a site supports the standart or
not. https://wicg.github.io/change-password-url/
</summary>
<metric name="WellKnownChangePasswordResult"
enum="WellKnownChangePasswordResult">
<summary>
Records if a site supports .well-known/change-password and if a url
override was used or not.
</summary>
<aggregation>
<history>
<statistics>
<enumeration/>
</statistics>
</history>
</aggregation>
</metric>
</event>
<event name="PaymentApp.CheckoutEvents">
<owner>sahel@chromium.org</owner>
<owner>web-payments-team@google.com</owner>
......
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