0

[BTM] Report destination page UKM source ID from BtmPageVisitObserver

BtmNavigationFlowDetector needs the UKM source ID for some of the UKM
events it emits.

Bug: 390201794
Change-Id: I14503265ee73fc26c2b773879fc530e6292ec113
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6377908
Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Reviewed-by: Andrew Liu <liu@chromium.org>
Commit-Queue: Svend Larsen <svend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1437749}
This commit is contained in:
Svend Larsen
2025-03-25 14:33:13 -07:00
committed by Chromium LUCI CQ
parent bedb97fa83
commit f9aab1453b
6 changed files with 180 additions and 106 deletions

@ -20,9 +20,16 @@ BtmNavigationInfo::BtmNavigationInfo(NavigationHandle& navigation_handle)
: was_user_initiated(!navigation_handle.IsRendererInitiated() ||
navigation_handle.HasUserGesture()),
was_renderer_initiated(navigation_handle.IsRendererInitiated()),
page_transition(navigation_handle.GetPageTransition()) {}
page_transition(navigation_handle.GetPageTransition()),
destination({navigation_handle.GetURL(),
navigation_handle.GetNextPageUkmSourceId()}) {
CHECK(navigation_handle.HasCommitted());
}
BtmNavigationInfo::BtmNavigationInfo(const BtmNavigationInfo&) = default;
BtmNavigationInfo& BtmNavigationInfo::operator=(const BtmNavigationInfo&) =
default;
BtmNavigationInfo::BtmNavigationInfo(BtmNavigationInfo&&) = default;
BtmNavigationInfo& BtmNavigationInfo::operator=(BtmNavigationInfo&&) = default;
BtmNavigationInfo::~BtmNavigationInfo() = default;
BtmPageVisitObserver::BtmPageVisitObserver(WebContents* web_contents,
@ -69,6 +76,7 @@ class NavigationState
// Returns the navigation info paired with the cookie access of the final
// (i.e. committed) URL of the navigation.
// Precondition: `navigation_handle.HasCommitted()` must be `true`.
std::pair<BtmNavigationInfo, BtmDataAccessType> CreateNavigationInfo(
NavigationHandle& navigation_handle) {
BtmNavigationInfo navigation(navigation_handle);
@ -168,8 +176,7 @@ void BtmPageVisitObserver::DidFinishNavigation(
state->CreateNavigationInfo(*navigation_handle);
// Don't report the visit right away; put it in the pending queue and wait a
// bit to see if we receive any late cookie notifications.
pending_visits_.emplace_back(std::move(current_page_), std::move(navigation),
navigation_handle->GetURL());
pending_visits_.emplace_back(std::move(current_page_), std::move(navigation));
base::SequencedTaskRunner::GetCurrentDefault()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&BtmPageVisitObserver::ReportVisit,
@ -186,7 +193,7 @@ void BtmPageVisitObserver::DidFinishNavigation(
void BtmPageVisitObserver::ReportVisit() {
CHECK(!pending_visits_.empty());
VisitTuple& visit = pending_visits_.front();
callback_.Run(visit.prev_page, visit.navigation, visit.url);
callback_.Run(visit.prev_page, visit.navigation);
pending_visits_.pop_front();
}

@ -15,6 +15,7 @@
#include "base/time/default_clock.h"
#include "base/time/time.h"
#include "content/common/content_export.h"
#include "content/public/browser/btm_redirect_info.h"
#include "content/public/browser/web_contents_observer.h"
#include "url/gurl.h"
@ -39,27 +40,31 @@ struct CONTENT_EXPORT BtmServerRedirectInfo {
};
struct CONTENT_EXPORT BtmNavigationInfo {
// Precondition: `navigation_handle.HasCommitted()` must be `true`.
explicit BtmNavigationInfo(NavigationHandle& navigation_handle);
BtmNavigationInfo(const BtmNavigationInfo&);
BtmNavigationInfo& operator=(const BtmNavigationInfo&);
BtmNavigationInfo(BtmNavigationInfo&&);
BtmNavigationInfo& operator=(BtmNavigationInfo&&);
~BtmNavigationInfo();
std::vector<BtmServerRedirectInfo> server_redirects;
bool was_user_initiated;
bool was_renderer_initiated;
ui::PageTransition page_transition;
// The page where the navigation ultimately committed.
UrlAndSourceId destination;
};
class CONTENT_EXPORT BtmPageVisitObserver : public WebContentsObserver {
public:
using VisitCallback = base::RepeatingCallback<
void(const BtmPageVisitInfo&, const BtmNavigationInfo&, const GURL&)>;
using VisitCallback = base::RepeatingCallback<void(const BtmPageVisitInfo&,
const BtmNavigationInfo&)>;
// The three arguments to `VisitCallback`.
// The arguments to `VisitCallback`.
struct VisitTuple {
BtmPageVisitInfo prev_page;
BtmNavigationInfo navigation;
GURL url;
};
BtmPageVisitObserver(WebContents* web_contents,

@ -84,14 +84,19 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, SmokeTest) {
ASSERT_TRUE(NavigateToURL(web_contents, url3));
ASSERT_TRUE(recorder.WaitForSize(3));
EXPECT_EQ(recorder.visits()[0].prev_page.url, GURL());
EXPECT_EQ(recorder.visits()[0].url, url1);
EXPECT_THAT(recorder.visits()[0].prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_THAT(recorder.visits()[0].navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_EQ(recorder.visits()[1].prev_page.url, url1);
EXPECT_EQ(recorder.visits()[1].url, url2);
EXPECT_THAT(recorder.visits()[1].prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_THAT(recorder.visits()[1].navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_EQ(recorder.visits()[2].prev_page.url, url2);
EXPECT_EQ(recorder.visits()[2].url, url3);
EXPECT_THAT(recorder.visits()[2].prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_THAT(recorder.visits()[2].navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -116,12 +121,14 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, CreatedWhileOnPage) {
// Should be the URL we were on when observation started, instead of blank.
EXPECT_THAT(first_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_EQ(first_visit.url, url2);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitRecorder::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_EQ(second_visit.url, url3);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 2u);
}
@ -149,7 +156,8 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest,
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_THAT(first_visit.navigation.server_redirects, IsEmpty());
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
@ -157,7 +165,8 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest,
// Same-document navigations shouldn't be reported as server redirects.
EXPECT_THAT(first_visit.navigation.server_redirects, IsEmpty());
// Same-document navigations shouldn't be counted as page visits.
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 2u);
}
@ -183,13 +192,15 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, Redirects) {
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_THAT(first_visit.navigation.server_redirects, IsEmpty());
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_EQ(second_visit.navigation.server_redirects.size(), 2u);
EXPECT_EQ(second_visit.url, url3);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
const BtmServerRedirectInfo& first_server_redirect =
second_visit.navigation.server_redirects[0];
@ -248,7 +259,8 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, RedirectToSelf) {
ASSERT_TRUE(recorder.WaitForSize(3));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_EQ(second_visit.navigation.server_redirects.size(), 1u);
const BtmServerRedirectInfo& first_server_redirect =
@ -266,7 +278,8 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, RedirectToSelf) {
// The non-navigation cookie write is correctly attributed to the previous
// page.
EXPECT_TRUE(third_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -293,7 +306,8 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, IframeRedirect) {
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_THAT(first_visit.navigation.server_redirects, IsEmpty());
EXPECT_EQ(first_visit.url, top_level_url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(top_level_url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
@ -301,7 +315,8 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, IframeRedirect) {
// Only redirects in top-level navigations should be reported; redirects in
// iframes should be ignored.
EXPECT_THAT(second_visit.navigation.server_redirects, IsEmpty());
EXPECT_EQ(second_visit.url, top_level_url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(top_level_url2, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 2u);
}
@ -325,20 +340,23 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, DocumentCookie) {
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_FALSE(second_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
// url2 accessed cookies; no other page did.
EXPECT_TRUE(third_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -359,13 +377,15 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, UserActivation) {
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.received_user_activation);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_TRUE(second_visit.prev_page.received_user_activation);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 2u);
}
@ -392,21 +412,24 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, NavigationInitiation) {
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.navigation.was_renderer_initiated);
EXPECT_TRUE(first_visit.navigation.was_user_initiated);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_TRUE(second_visit.navigation.was_renderer_initiated);
EXPECT_TRUE(second_visit.navigation.was_user_initiated);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_TRUE(third_visit.navigation.was_renderer_initiated);
EXPECT_FALSE(third_visit.navigation.was_user_initiated);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -439,19 +462,22 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, VisitDuration) {
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_EQ(first_visit.prev_page.visit_duration,
time_elapsed_before_first_page_visit);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_EQ(second_visit.prev_page.visit_duration, visit_duration1);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_EQ(third_visit.prev_page.visit_duration, visit_duration2);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -483,21 +509,24 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, PageTransition) {
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_THAT(first_visit.navigation.page_transition,
CoreTypeIs(ui::PageTransition::PAGE_TRANSITION_TYPED));
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_THAT(second_visit.navigation.page_transition,
CoreTypeIs(transition_type2));
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_THAT(third_visit.navigation.page_transition,
CoreTypeIs(transition_type3));
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -523,7 +552,8 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest,
BtmPageVisitRecorder::VisitTuple first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
BtmPageVisitRecorder::VisitTuple second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
@ -531,7 +561,8 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest,
// Navigational cookie writes are active storage accesses and should be
// reported.
EXPECT_TRUE(second_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
BtmPageVisitRecorder::VisitTuple third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
@ -539,7 +570,8 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest,
// Navigational cookie reads are passive storage accesses and shouldn't be
// reported.
EXPECT_FALSE(third_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -570,19 +602,22 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, SubresourceCookie) {
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_TRUE(second_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_FALSE(third_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -629,28 +664,32 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest,
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
// 1P cookie accesses in iframes should be reported.
EXPECT_TRUE(second_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
// Non-CHIPS 3P cookie accesses in iframes should be ignored.
EXPECT_FALSE(third_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& fourth_visit = recorder.visits()[3];
EXPECT_THAT(fourth_visit.prev_page,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
// CHIPS 3P cookie accesses in iframes should be reported.
EXPECT_TRUE(fourth_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(fourth_visit.url, url4);
EXPECT_THAT(fourth_visit.navigation.destination,
HasUrlAndMatchingSourceId(url4, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 4u);
}
@ -720,28 +759,32 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest, IframeDocumentCookie) {
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
// Iframe 1P cookie accesses should be reported.
EXPECT_TRUE(second_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
// CHIPS 3PC accesses should be reported.
EXPECT_TRUE(third_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& fourth_visit = recorder.visits()[3];
EXPECT_THAT(fourth_visit.prev_page,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
// Non-CHIPS 3PC accesses should be ignored.
EXPECT_FALSE(fourth_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(fourth_visit.url, url4);
EXPECT_THAT(fourth_visit.navigation.destination,
HasUrlAndMatchingSourceId(url4, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 4u);
}
@ -773,19 +816,22 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest,
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_TRUE(second_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_FALSE(third_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -819,19 +865,22 @@ IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverClientRedirectBrowserTest,
// The client redirector shouldn't be reported as a server redirector.
EXPECT_THAT(first_visit.navigation.server_redirects, IsEmpty());
// The client redirector should be reported as a page visit instead.
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_THAT(second_visit.navigation.server_redirects, IsEmpty());
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_THAT(third_visit.navigation.server_redirects, IsEmpty());
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -866,7 +915,8 @@ IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverClientRedirectBrowserTest,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
EXPECT_FALSE(first_visit.navigation.server_redirects[0].did_write_cookies);
// Client redirectors should be reported as page visits.
EXPECT_EQ(first_visit.url, url2);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
@ -877,7 +927,8 @@ IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverClientRedirectBrowserTest,
EXPECT_THAT(second_visit.navigation.server_redirects[0],
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_TRUE(second_visit.navigation.server_redirects[0].did_write_cookies);
EXPECT_EQ(second_visit.url, url4);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url4, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 2u);
}
@ -937,7 +988,8 @@ IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverSiteDataAccessBrowserTest,
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
@ -945,13 +997,15 @@ IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverSiteDataAccessBrowserTest,
// Storage accesses by the primary main frame should be attributed to that
// page's visit.
EXPECT_TRUE(second_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_FALSE(third_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -977,20 +1031,23 @@ IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverSiteDataAccessBrowserTest,
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
// Storage accesses by iframes should be attributed to the top-level frame.
EXPECT_TRUE(second_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_FALSE(third_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}
@ -1019,14 +1076,16 @@ IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverSiteDataAccessBrowserTest,
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
// Storage accesses in fenced frames should be ignored.
EXPECT_FALSE(second_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 2u);
}
@ -1069,14 +1128,16 @@ IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverSiteDataAccessBrowserTest,
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
// Storage accesses in prerendered pages should be ignored.
EXPECT_FALSE(second_visit.prev_page.had_qualifying_storage_access);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 2u);
}
@ -1216,20 +1277,23 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverWebAuthnTest, SuccessfulWAA) {
const BtmPageVisitObserver::VisitTuple& first_visit = recorder.visits()[0];
EXPECT_THAT(first_visit.prev_page, HasUrlAndSourceIdForBlankPage());
EXPECT_FALSE(first_visit.prev_page.had_successful_web_authn_assertion);
EXPECT_EQ(first_visit.url, url1);
EXPECT_THAT(first_visit.navigation.destination,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& second_visit = recorder.visits()[1];
EXPECT_THAT(second_visit.prev_page,
HasUrlAndMatchingSourceId(url1, &ukm_recorder()));
// Successful WAAs should be reported.
EXPECT_TRUE(second_visit.prev_page.had_successful_web_authn_assertion);
EXPECT_EQ(second_visit.url, url2);
EXPECT_THAT(second_visit.navigation.destination,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
const BtmPageVisitObserver::VisitTuple& third_visit = recorder.visits()[2];
EXPECT_THAT(third_visit.prev_page,
HasUrlAndMatchingSourceId(url2, &ukm_recorder()));
EXPECT_FALSE(third_visit.prev_page.had_successful_web_authn_assertion);
EXPECT_EQ(third_visit.url, url3);
EXPECT_THAT(third_visit.navigation.destination,
HasUrlAndMatchingSourceId(url3, &ukm_recorder()));
EXPECT_EQ(recorder.visits().size(), 3u);
}

@ -46,8 +46,7 @@ std::ostream& operator<<(std::ostream& out, const BtmNavigationInfo& nav) {
std::ostream& operator<<(std::ostream& out,
const BtmPageVisitObserver::VisitTuple& visit) {
return out << "VisitTuple{prev_page=" << visit.prev_page
<< ", navigation=" << visit.navigation << ", url=" << visit.url
<< "}";
<< ", navigation=" << visit.navigation << "}";
}
BtmPageVisitRecorder::BtmPageVisitRecorder(WebContents* web_contents,
@ -76,9 +75,8 @@ BtmPageVisitRecorder::~BtmPageVisitRecorder() = default;
}
void BtmPageVisitRecorder::OnVisit(const BtmPageVisitInfo& prev_page,
const BtmNavigationInfo& navigation,
const GURL& url) {
visits_.emplace_back(prev_page, navigation, url);
const BtmNavigationInfo& navigation) {
visits_.emplace_back(prev_page, navigation);
if (wait_state_.has_value() && wait_state_->wanted_count <= visits_.size()) {
wait_state_->run_loop.Quit();

@ -50,8 +50,7 @@ class BtmPageVisitRecorder {
// Called by `observer_` on each page visit; appends to `visits_`.
void OnVisit(const BtmPageVisitInfo& prev_page,
const BtmNavigationInfo& navigation,
const GURL& url);
const BtmNavigationInfo& navigation);
std::vector<VisitTuple> visits_;
// Populated only during a call to `WaitForSize()`. Mostly a `RunLoop` wrapped
@ -60,8 +59,8 @@ class BtmPageVisitRecorder {
BtmPageVisitObserver observer_;
};
// Matches the `url` property of `VisitTuple`, `BtmPageVisitInfo`, or
// `BtmServerRedirectInfo`.
// Matches the `url` property of `BtmPageVisitInfo`,
// `BtmNavigationInfo::destination`, or `BtmServerRedirectInfo`.
MATCHER_P(HasUrl,
matcher,
"has url that " + testing::DescribeMatcher<GURL>(matcher, negation)) {

@ -52,14 +52,13 @@ TEST_F(BtmPageVisitObserverTest, PreviousPage) {
tester->NavigateAndCommit(url1);
ASSERT_TRUE(recorder.WaitForSize(1));
ASSERT_THAT(recorder.visits(),
ElementsAre(AllOf(PreviousPage(HasUrl(GURL())), HasUrl(url1))));
EXPECT_EQ(recorder.visits()[0].prev_page.url, GURL());
EXPECT_EQ(recorder.visits()[0].navigation.destination.url, url1);
tester->NavigateAndCommit(url2);
ASSERT_TRUE(recorder.WaitForSize(2));
ASSERT_THAT(recorder.visits(),
ElementsAre(AllOf(PreviousPage(HasUrl(GURL())), HasUrl(url1)),
AllOf(PreviousPage(HasUrl(url1)), HasUrl(url2))));
EXPECT_EQ(recorder.visits()[1].prev_page.url, url1);
EXPECT_EQ(recorder.visits()[1].navigation.destination.url, url2);
}
TEST_F(BtmPageVisitObserverTest, ServerRedirects) {
@ -78,12 +77,13 @@ TEST_F(BtmPageVisitObserverTest, ServerRedirects) {
ASSERT_TRUE(recorder.WaitForSize(2));
// Two navigations are observed, the second with a redirect.
ASSERT_THAT(
recorder.visits(),
ElementsAre(AllOf(Navigation(ServerRedirects(IsEmpty())), HasUrl(url1)),
AllOf(PreviousPage(HasUrl(url1)),
Navigation(ServerRedirects(ElementsAre(HasUrl(url2)))),
HasUrl(url3))));
const BtmNavigationInfo& navigation1 = recorder.visits()[0].navigation;
EXPECT_EQ(navigation1.server_redirects.size(), 0u);
EXPECT_EQ(navigation1.destination.url, url1);
const BtmNavigationInfo& navigation2 = recorder.visits()[1].navigation;
ASSERT_EQ(navigation2.server_redirects.size(), 1u);
EXPECT_EQ(navigation2.server_redirects[0].url, url2);
EXPECT_EQ(navigation2.destination.url, url3);
}
TEST_F(BtmPageVisitObserverTest, IgnoreUncommitted) {
@ -103,9 +103,9 @@ TEST_F(BtmPageVisitObserverTest, IgnoreUncommitted) {
ASSERT_TRUE(recorder.WaitForSize(2));
// Only url1 and url3 navigations are observed.
ASSERT_THAT(recorder.visits(),
ElementsAre(HasUrl(url1),
AllOf(PreviousPage(HasUrl(url1)), HasUrl(url3))));
EXPECT_EQ(recorder.visits()[0].navigation.destination.url, url1);
EXPECT_EQ(recorder.visits()[1].prev_page.url, url1);
EXPECT_EQ(recorder.visits()[1].navigation.destination.url, url3);
}
TEST_F(BtmPageVisitObserverTest, IgnoreSubframes) {
@ -126,9 +126,9 @@ TEST_F(BtmPageVisitObserverTest, IgnoreSubframes) {
ASSERT_TRUE(recorder.WaitForSize(2));
// Only url1 and url3 navigations are observed.
ASSERT_THAT(recorder.visits(),
ElementsAre(HasUrl(url1),
AllOf(PreviousPage(HasUrl(url1)), HasUrl(url3))));
EXPECT_EQ(recorder.visits()[0].navigation.destination.url, url1);
EXPECT_EQ(recorder.visits()[1].prev_page.url, url1);
EXPECT_EQ(recorder.visits()[1].navigation.destination.url, url3);
}
// Same-document navigations are ignored.
@ -149,9 +149,9 @@ TEST_F(BtmPageVisitObserverTest, IgnoreSameDocument) {
ASSERT_TRUE(recorder.WaitForSize(2));
// Only the url1a and url2 navigations are observed.
ASSERT_THAT(recorder.visits(),
ElementsAre(HasUrl(url1a),
AllOf(PreviousPage(HasUrl(url1a)), HasUrl(url2))));
EXPECT_EQ(recorder.visits()[0].navigation.destination.url, url1a);
EXPECT_EQ(recorder.visits()[1].prev_page.url, url1a);
EXPECT_EQ(recorder.visits()[1].navigation.destination.url, url2);
}
TEST_F(BtmPageVisitObserverTest, FlushPendingVisitsAtDestruction) {
@ -160,9 +160,10 @@ TEST_F(BtmPageVisitObserverTest, FlushPendingVisitsAtDestruction) {
{
BtmPageVisitObserver observer(
web_contents(),
base::BindLambdaForTesting([&counter](const BtmPageVisitInfo&,
const BtmNavigationInfo&,
const GURL&) { ++counter; }));
base::BindLambdaForTesting(
[&counter](const BtmPageVisitInfo&, const BtmNavigationInfo&) {
++counter;
}));
NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(),
GURL("http://a.test/"));
NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(),
@ -173,7 +174,7 @@ TEST_F(BtmPageVisitObserverTest, FlushPendingVisitsAtDestruction) {
// If observer's pending visits isn't flushed, `counter` will (typically)
// still be 0. But if they are, all three visits will be recorded.
ASSERT_EQ(counter, 3);
EXPECT_EQ(counter, 3);
}
} // namespace content