Commit e993130c authored by Livvie Lin's avatar Livvie Lin Committed by Commit Bot

[iOS] Add additional metrics and tests for lookalikes

Bug: 1058898
Change-Id: I395d8e95992e2d741882f199fb80a5173db49d61
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2304310Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Commit-Queue: Livvie Lin <livvielin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790543}
parent 3bce16fc
...@@ -43,6 +43,9 @@ source_set("unit_tests") { ...@@ -43,6 +43,9 @@ source_set("unit_tests") {
":lookalikes", ":lookalikes",
"//base/test:test_support", "//base/test:test_support",
"//components/lookalikes/core", "//components/lookalikes/core",
"//components/security_interstitials/core",
"//components/ukm:test_support",
"//components/ukm/ios",
"//ios/web/public", "//ios/web/public",
"//ios/web/public/test", "//ios/web/public/test",
"//net", "//net",
......
include_rules = [ include_rules = [
"+components/lookalikes/core", "+components/lookalikes/core",
"+components/ukm/ios", "+components/ukm",
"+components/url_formatter", "+components/url_formatter",
"+ios/net", "+ios/net",
"+net/base", "+net/base",
......
...@@ -35,6 +35,10 @@ LookalikeUrlBlockingPage::LookalikeUrlBlockingPage( ...@@ -35,6 +35,10 @@ LookalikeUrlBlockingPage::LookalikeUrlBlockingPage(
source_id_(source_id), source_id_(source_id),
match_type_(match_type) { match_type_(match_type) {
DCHECK(web_state_); DCHECK(web_state_);
controller_->metrics_helper()->RecordUserDecision(
security_interstitials::MetricsHelper::SHOW);
controller_->metrics_helper()->RecordUserInteraction(
security_interstitials::MetricsHelper::TOTAL_VISITS);
// Creating an interstitial without showing it (e.g. from // Creating an interstitial without showing it (e.g. from
// chrome://interstitials) leaks memory, so don't create it here. // chrome://interstitials) leaks memory, so don't create it here.
......
...@@ -6,14 +6,19 @@ ...@@ -6,14 +6,19 @@
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#import "base/test/ios/wait_util.h" #import "base/test/ios/wait_util.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/values.h" #include "base/values.h"
#include "components/lookalikes/core/lookalike_url_util.h" #include "components/lookalikes/core/lookalike_url_util.h"
#include "components/security_interstitials/core/metrics_helper.h"
#include "components/ukm/ios/ukm_url_recorder.h"
#include "components/ukm/test_ukm_recorder.h"
#include "ios/components/security_interstitials/lookalikes/lookalike_url_controller_client.h" #include "ios/components/security_interstitials/lookalikes/lookalike_url_controller_client.h"
#include "ios/components/security_interstitials/lookalikes/lookalike_url_tab_allow_list.h" #include "ios/components/security_interstitials/lookalikes/lookalike_url_tab_allow_list.h"
#import "ios/web/public/navigation/navigation_item.h" #import "ios/web/public/navigation/navigation_item.h"
#import "ios/web/public/test/fakes/test_navigation_manager.h" #import "ios/web/public/test/fakes/test_navigation_manager.h"
#import "ios/web/public/test/fakes/test_web_state.h" #import "ios/web/public/test/fakes/test_web_state.h"
#include "ios/web/public/test/web_task_environment.h" #include "ios/web/public/test/web_task_environment.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_source_id.h" #include "services/metrics/public/cpp/ukm_source_id.h"
#include "testing/platform_test.h" #include "testing/platform_test.h"
...@@ -23,18 +28,28 @@ ...@@ -23,18 +28,28 @@
using security_interstitials::IOSSecurityInterstitialPage; using security_interstitials::IOSSecurityInterstitialPage;
using security_interstitials::SecurityInterstitialCommand; using security_interstitials::SecurityInterstitialCommand;
using security_interstitials::MetricsHelper;
using base::test::ios::WaitUntilConditionOrTimeout; using base::test::ios::WaitUntilConditionOrTimeout;
using base::test::ios::kSpinDelaySeconds; using base::test::ios::kSpinDelaySeconds;
namespace { namespace {
// Constants used for testing metrics.
const char kInterstitialDecisionMetric[] = "interstitial.lookalike.decision";
const char kInterstitialInteractionMetric[] =
"interstitial.lookalike.interaction";
const ukm::SourceId kTestSourceId = 1;
const LookalikeUrlMatchType kTestMatchType =
LookalikeUrlMatchType::kSkeletonMatchTop500;
using UkmEntry = ukm::builders::LookalikeUrl_NavigationSuggestion;
// Creates a LookalikeUrlBlockingPage with a given |safe_url|. // Creates a LookalikeUrlBlockingPage with a given |safe_url|.
std::unique_ptr<LookalikeUrlBlockingPage> CreateBlockingPage( std::unique_ptr<LookalikeUrlBlockingPage> CreateBlockingPage(
web::WebState* web_state, web::WebState* web_state,
const GURL& safe_url, const GURL& safe_url,
const GURL& request_url) { const GURL& request_url) {
return std::make_unique<LookalikeUrlBlockingPage>( return std::make_unique<LookalikeUrlBlockingPage>(
web_state, safe_url, request_url, ukm::kInvalidSourceId, web_state, safe_url, request_url, kTestSourceId, kTestMatchType,
LookalikeUrlMatchType::kSkeletonMatchTop500,
std::make_unique<LookalikeUrlControllerClient>(web_state, safe_url, std::make_unique<LookalikeUrlControllerClient>(web_state, safe_url,
request_url, "en-US")); request_url, "en-US"));
} }
...@@ -58,6 +73,7 @@ class LookalikeUrlBlockingPageTest : public PlatformTest { ...@@ -58,6 +73,7 @@ class LookalikeUrlBlockingPageTest : public PlatformTest {
web_state_.SetNavigationManager(std::move(navigation_manager)); web_state_.SetNavigationManager(std::move(navigation_manager));
LookalikeUrlTabAllowList::CreateForWebState(&web_state_); LookalikeUrlTabAllowList::CreateForWebState(&web_state_);
LookalikeUrlTabAllowList::FromWebState(&web_state_); LookalikeUrlTabAllowList::FromWebState(&web_state_);
ukm::InitializeSourceUrlRecorderForWebState(&web_state_);
} }
void SendCommand(SecurityInterstitialCommand command) { void SendCommand(SecurityInterstitialCommand command) {
...@@ -68,6 +84,18 @@ class LookalikeUrlBlockingPageTest : public PlatformTest { ...@@ -68,6 +84,18 @@ class LookalikeUrlBlockingPageTest : public PlatformTest {
/*sender_frame=*/nullptr); /*sender_frame=*/nullptr);
} }
// Checks that UKM recorded an event with the given metric name and value.
template <typename T>
void CheckUkm(const std::string& metric_name, T metric_value) {
auto navigation_entries =
test_ukm_recorder_.GetEntriesByName(UkmEntry::kEntryName);
ASSERT_EQ(1u, navigation_entries.size());
const ukm::mojom::UkmEntry* entry = navigation_entries[0];
ASSERT_TRUE(entry);
test_ukm_recorder_.ExpectEntryMetric(entry, metric_name,
static_cast<int>(metric_value));
}
protected: protected:
web::WebTaskEnvironment task_environment_{ web::WebTaskEnvironment task_environment_{
web::WebTaskEnvironment::IO_MAINLOOP}; web::WebTaskEnvironment::IO_MAINLOOP};
...@@ -75,11 +103,14 @@ class LookalikeUrlBlockingPageTest : public PlatformTest { ...@@ -75,11 +103,14 @@ class LookalikeUrlBlockingPageTest : public PlatformTest {
web::TestNavigationManager* navigation_manager_ = nullptr; web::TestNavigationManager* navigation_manager_ = nullptr;
GURL url_; GURL url_;
std::unique_ptr<IOSSecurityInterstitialPage> page_; std::unique_ptr<IOSSecurityInterstitialPage> page_;
base::HistogramTester histogram_tester_;
ukm::TestAutoSetUkmRecorder test_ukm_recorder_;
}; };
// Tests that the blocking page handles the proceed command by updating the // Tests that the blocking page handles the proceed command by updating the
// allow list and reloading the page. // allow list and reloading the page.
TEST_F(LookalikeUrlBlockingPageTest, HandleProceedCommand) { TEST_F(LookalikeUrlBlockingPageTest, HandleProceedCommand) {
test_ukm_recorder_.Purge();
GURL safe_url("https://www.safe.test"); GURL safe_url("https://www.safe.test");
page_ = CreateBlockingPage(&web_state_, safe_url, url_); page_ = CreateBlockingPage(&web_state_, safe_url, url_);
LookalikeUrlTabAllowList* allow_list = LookalikeUrlTabAllowList* allow_list =
...@@ -92,11 +123,24 @@ TEST_F(LookalikeUrlBlockingPageTest, HandleProceedCommand) { ...@@ -92,11 +123,24 @@ TEST_F(LookalikeUrlBlockingPageTest, HandleProceedCommand) {
EXPECT_TRUE(allow_list->IsDomainAllowed(url_.host())); EXPECT_TRUE(allow_list->IsDomainAllowed(url_.host()));
EXPECT_TRUE(navigation_manager_->ReloadWasCalled()); EXPECT_TRUE(navigation_manager_->ReloadWasCalled());
// Verify that metrics are recorded correctly.
histogram_tester_.ExpectTotalCount(kInterstitialDecisionMetric, 2);
histogram_tester_.ExpectBucketCount(kInterstitialDecisionMetric,
MetricsHelper::PROCEED, 1);
histogram_tester_.ExpectBucketCount(kInterstitialDecisionMetric,
MetricsHelper::SHOW, 1);
histogram_tester_.ExpectTotalCount(kInterstitialInteractionMetric, 1);
histogram_tester_.ExpectBucketCount(kInterstitialInteractionMetric,
MetricsHelper::TOTAL_VISITS, 1);
CheckUkm("MatchType", kTestMatchType);
CheckUkm("UserAction", LookalikeUrlBlockingPageUserAction::kClickThrough);
} }
// Tests that the blocking page handles the don't proceed command by navigating // Tests that the blocking page handles the don't proceed command by navigating
// to the suggested URL. // to the suggested URL.
TEST_F(LookalikeUrlBlockingPageTest, HandleDontProceedCommand) { TEST_F(LookalikeUrlBlockingPageTest, HandleDontProceedCommand) {
test_ukm_recorder_.Purge();
GURL safe_url("https://www.safe.test"); GURL safe_url("https://www.safe.test");
// Add a navigation for the committed interstitial page so that navigation to // Add a navigation for the committed interstitial page so that navigation to
// the safe URL can later be verified. // the safe URL can later be verified.
...@@ -107,12 +151,25 @@ TEST_F(LookalikeUrlBlockingPageTest, HandleDontProceedCommand) { ...@@ -107,12 +151,25 @@ TEST_F(LookalikeUrlBlockingPageTest, HandleDontProceedCommand) {
SendCommand(security_interstitials::CMD_DONT_PROCEED); SendCommand(security_interstitials::CMD_DONT_PROCEED);
EXPECT_EQ(web_state_.GetVisibleURL(), safe_url); EXPECT_EQ(web_state_.GetVisibleURL(), safe_url);
// Verify that metrics are recorded correctly.
histogram_tester_.ExpectTotalCount(kInterstitialDecisionMetric, 2);
histogram_tester_.ExpectBucketCount(kInterstitialDecisionMetric,
MetricsHelper::DONT_PROCEED, 1);
histogram_tester_.ExpectBucketCount(kInterstitialDecisionMetric,
MetricsHelper::SHOW, 1);
histogram_tester_.ExpectTotalCount(kInterstitialInteractionMetric, 1);
histogram_tester_.ExpectBucketCount(kInterstitialInteractionMetric,
MetricsHelper::TOTAL_VISITS, 1);
CheckUkm("MatchType", kTestMatchType);
CheckUkm("UserAction", LookalikeUrlBlockingPageUserAction::kAcceptSuggestion);
} }
// Tests that the blocking page handles the don't proceed command by going back // Tests that the blocking page handles the don't proceed command by going back
// if there is no safe NavigationItem to navigate to. // if there is no safe NavigationItem to navigate to.
TEST_F(LookalikeUrlBlockingPageTest, TEST_F(LookalikeUrlBlockingPageTest,
HandleDontProceedCommandWithoutSafeUrlGoBack) { HandleDontProceedCommandWithoutSafeUrlGoBack) {
test_ukm_recorder_.Purge();
// Insert a safe navigation so that the page can navigate back to safety, then // Insert a safe navigation so that the page can navigate back to safety, then
// add a navigation for the committed interstitial page. // add a navigation for the committed interstitial page.
GURL safe_url("https://www.safe.test"); GURL safe_url("https://www.safe.test");
...@@ -129,6 +186,18 @@ TEST_F(LookalikeUrlBlockingPageTest, ...@@ -129,6 +186,18 @@ TEST_F(LookalikeUrlBlockingPageTest,
// Verify that the NavigationManager has navigated back. // Verify that the NavigationManager has navigated back.
EXPECT_EQ(0, navigation_manager_->GetLastCommittedItemIndex()); EXPECT_EQ(0, navigation_manager_->GetLastCommittedItemIndex());
EXPECT_FALSE(navigation_manager_->CanGoBack()); EXPECT_FALSE(navigation_manager_->CanGoBack());
// Verify that metrics are recorded correctly.
histogram_tester_.ExpectTotalCount(kInterstitialDecisionMetric, 2);
histogram_tester_.ExpectBucketCount(kInterstitialDecisionMetric,
MetricsHelper::DONT_PROCEED, 1);
histogram_tester_.ExpectBucketCount(kInterstitialDecisionMetric,
MetricsHelper::SHOW, 1);
histogram_tester_.ExpectTotalCount(kInterstitialInteractionMetric, 1);
histogram_tester_.ExpectBucketCount(kInterstitialInteractionMetric,
MetricsHelper::TOTAL_VISITS, 1);
CheckUkm("MatchType", kTestMatchType);
CheckUkm("UserAction", LookalikeUrlBlockingPageUserAction::kAcceptSuggestion);
} }
// Tests that the blocking page handles the don't proceed command by closing the // Tests that the blocking page handles the don't proceed command by closing the
...@@ -136,6 +205,7 @@ TEST_F(LookalikeUrlBlockingPageTest, ...@@ -136,6 +205,7 @@ TEST_F(LookalikeUrlBlockingPageTest,
// back. // back.
TEST_F(LookalikeUrlBlockingPageTest, TEST_F(LookalikeUrlBlockingPageTest,
HandleDontProceedCommandWithoutSafeUrlClose) { HandleDontProceedCommandWithoutSafeUrlClose) {
test_ukm_recorder_.Purge();
page_ = CreateBlockingPage(&web_state_, GURL::EmptyGURL(), url_); page_ = CreateBlockingPage(&web_state_, GURL::EmptyGURL(), url_);
ASSERT_FALSE(navigation_manager_->CanGoBack()); ASSERT_FALSE(navigation_manager_->CanGoBack());
...@@ -148,4 +218,16 @@ TEST_F(LookalikeUrlBlockingPageTest, ...@@ -148,4 +218,16 @@ TEST_F(LookalikeUrlBlockingPageTest,
EXPECT_TRUE(WaitUntilConditionOrTimeout(kSpinDelaySeconds, ^{ EXPECT_TRUE(WaitUntilConditionOrTimeout(kSpinDelaySeconds, ^{
return web_state_.IsClosed(); return web_state_.IsClosed();
})); }));
// Verify that metrics are recorded correctly.
histogram_tester_.ExpectTotalCount(kInterstitialDecisionMetric, 2);
histogram_tester_.ExpectBucketCount(kInterstitialDecisionMetric,
MetricsHelper::DONT_PROCEED, 1);
histogram_tester_.ExpectBucketCount(kInterstitialDecisionMetric,
MetricsHelper::SHOW, 1);
histogram_tester_.ExpectTotalCount(kInterstitialInteractionMetric, 1);
histogram_tester_.ExpectBucketCount(kInterstitialInteractionMetric,
MetricsHelper::TOTAL_VISITS, 1);
CheckUkm("MatchType", kTestMatchType);
CheckUkm("UserAction", LookalikeUrlBlockingPageUserAction::kAcceptSuggestion);
} }
...@@ -101,6 +101,8 @@ void LookalikeUrlTabHelper::ShouldAllowResponse( ...@@ -101,6 +101,8 @@ void LookalikeUrlTabHelper::ShouldAllowResponse(
} }
DCHECK(!matched_domain.empty()); DCHECK(!matched_domain.empty());
RecordUMAFromMatchType(match_type);
if (ShouldBlockLookalikeUrlNavigation(match_type, navigated_domain)) { if (ShouldBlockLookalikeUrlNavigation(match_type, navigated_domain)) {
const std::string suggested_domain = GetETLDPlusOne(matched_domain); const std::string suggested_domain = GetETLDPlusOne(matched_domain);
DCHECK(!suggested_domain.empty()); DCHECK(!suggested_domain.empty());
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#import "ios/components/security_interstitials/lookalikes/lookalike_url_tab_helper.h" #import "ios/components/security_interstitials/lookalikes/lookalike_url_tab_helper.h"
#include "base/test/metrics/histogram_tester.h"
#include "ios/components/security_interstitials/lookalikes/lookalike_url_container.h" #include "ios/components/security_interstitials/lookalikes/lookalike_url_container.h"
#include "ios/components/security_interstitials/lookalikes/lookalike_url_tab_allow_list.h" #include "ios/components/security_interstitials/lookalikes/lookalike_url_tab_allow_list.h"
#import "ios/web/public/navigation/web_state_policy_decider.h" #import "ios/web/public/navigation/web_state_policy_decider.h"
...@@ -51,6 +52,8 @@ class LookalikeUrlTabHelperTest : public PlatformTest { ...@@ -51,6 +52,8 @@ class LookalikeUrlTabHelperTest : public PlatformTest {
LookalikeUrlTabAllowList* allow_list() { return allow_list_; } LookalikeUrlTabAllowList* allow_list() { return allow_list_; }
base::HistogramTester histogram_tester_;
private: private:
web::TestWebState web_state_; web::TestWebState web_state_;
LookalikeUrlTabAllowList* allow_list_; LookalikeUrlTabAllowList* allow_list_;
...@@ -59,12 +62,18 @@ class LookalikeUrlTabHelperTest : public PlatformTest { ...@@ -59,12 +62,18 @@ class LookalikeUrlTabHelperTest : public PlatformTest {
// Tests that ShouldAllowResponse properly blocks lookalike navigations and // Tests that ShouldAllowResponse properly blocks lookalike navigations and
// allows subframe navigations, non-HTTP/S navigations, and navigations // allows subframe navigations, non-HTTP/S navigations, and navigations
// to allowed domains. ShouldAllowRequest should always allow the navigation. // to allowed domains. ShouldAllowRequest should always allow the navigation.
// Also tests that UMA records correctly.
TEST_F(LookalikeUrlTabHelperTest, ShouldAllowResponse) { TEST_F(LookalikeUrlTabHelperTest, ShouldAllowResponse) {
GURL lookalike_url("https://xn--googl-fsa.com/"); GURL lookalike_url("https://xn--googl-fsa.com/");
// Lookalike IDNs should be blocked. // Lookalike IDNs should be blocked.
EXPECT_FALSE(ShouldAllowResponseUrl(lookalike_url, /*main_frame=*/true) EXPECT_FALSE(ShouldAllowResponseUrl(lookalike_url, /*main_frame=*/true)
.ShouldAllowNavigation()); .ShouldAllowNavigation());
histogram_tester_.ExpectUniqueSample(
lookalikes::kHistogramName,
static_cast<base::HistogramBase::Sample>(
NavigationSuggestionEvent::kMatchSkeletonTop500),
1);
// Non-main frame navigations should be allowed. // Non-main frame navigations should be allowed.
EXPECT_TRUE(ShouldAllowResponseUrl(lookalike_url, /*main_frame=*/false) EXPECT_TRUE(ShouldAllowResponseUrl(lookalike_url, /*main_frame=*/false)
...@@ -79,4 +88,6 @@ TEST_F(LookalikeUrlTabHelperTest, ShouldAllowResponse) { ...@@ -79,4 +88,6 @@ TEST_F(LookalikeUrlTabHelperTest, ShouldAllowResponse) {
allow_list()->AllowDomain("xn--googl-fsa.com"); allow_list()->AllowDomain("xn--googl-fsa.com");
EXPECT_TRUE(ShouldAllowResponseUrl(lookalike_url, /*main_frame=*/true) EXPECT_TRUE(ShouldAllowResponseUrl(lookalike_url, /*main_frame=*/true)
.ShouldAllowNavigation()); .ShouldAllowNavigation());
histogram_tester_.ExpectTotalCount(lookalikes::kHistogramName, 1);
} }
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