chromium/src
0
Fork 0
Code Activity

Revert "[BTM] Report browser storage access from BtmPageVisitObserver"

Browse Source 
This reverts commit 13ee40a73b.

Reason for revert: new tests are failing on Linux MSan
crbug.com/398308039

Original change's description:
> [BTM] Report browser storage access from BtmPageVisitObserver
>
> Design: go/btm-page-visit-observer
>
> Bug: 390201794
> Change-Id: Icb51881cdc1f3b849cc3cf03a53d08ef2abb5d20
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6278044
> Commit-Queue: Svend Larsen <svend@chromium.org>
> Reviewed-by: Ryan Tarpine <rtarpine@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1423402}

Bug: 390201794, 398308039
Change-Id: I41171ac1ef6b774eb076b2efc7ca083d90cf4ad9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6292332
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Muyao Xu <muyaoxu@google.com>
Reviewed-by: Muyao Xu <muyaoxu@google.com>
Owners-Override: Muyao Xu <muyaoxu@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1423475}
This commit is contained in:
Muyao Xu
2025-02-21 16:13:08 -08:00
committed by Chromium LUCI CQ
parent 18509b98d8
commit 7529d68d0c
6 changed files with 17 additions and 218 deletions

10
content/browser/dips/btm_page_visit_observer.cc

@ -148,16 +148,6 @@ void BtmPageVisitObserver::ReportVisit() {
pending_visits_.pop_front();
}
void BtmPageVisitObserver::NotifyStorageAccessed(
RenderFrameHost* render_frame_host,
blink::mojom::StorageTypeAccessed storage_type,
bool blocked) {
if (!render_frame_host->GetPage().IsPrimary() || blocked) {
return;
}
current_page_.had_qualifying_storage_access = true;
}
void BtmPageVisitObserver::OnCookiesAccessed(
RenderFrameHost* render_frame_host,
const CookieAccessDetails& details) {

3
content/browser/dips/btm_page_visit_observer.h

@ -70,9 +70,6 @@ class CONTENT_EXPORT BtmPageVisitObserver : public WebContentsObserver {
// WebContentsObserver overrides:
void DidStartNavigation(NavigationHandle* navigation_handle) override;
void DidFinishNavigation(NavigationHandle* navigation_handle) override;
void NotifyStorageAccessed(RenderFrameHost* render_frame_host,
blink::mojom::StorageTypeAccessed storage_type,
bool blocked) override;
void OnCookiesAccessed(RenderFrameHost* render_frame_host,
const CookieAccessDetails& details) override;
void OnCookiesAccessed(NavigationHandle* navigation_handle,

194
content/browser/dips/btm_page_visit_observer_browsertest.cc

@ -15,12 +15,9 @@
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/fenced_frame_test_util.h"
#include "content/public/test/prerender_test_util.h"
#include "content/shell/browser/shell.h"
#include "net/dns/mock_host_resolver.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/mojom/frame/frame.mojom-shared.h"
#include "url/gurl.h"
#if !BUILDFLAG(IS_ANDROID)
@ -39,8 +36,6 @@ using testing::IsEmpty;
namespace content {
namespace {
using blink::mojom::StorageTypeAccessed;
class BtmPageVisitObserverBrowserTest : public ContentBrowserTest {
public:
void SetUpOnMainThread() override {
@ -452,190 +447,11 @@ IN_PROC_BROWSER_TEST_F(BtmPageVisitObserverBrowserTest,
HasUrl(url3))));
}
class BtmPageVisitObserverSiteDataAccessBrowserTest
: public BtmPageVisitObserverBrowserTest,
public testing::WithParamInterface<StorageTypeAccessed> {
public:
BtmPageVisitObserverSiteDataAccessBrowserTest()
: prerender_test_helper_(
base::BindRepeating(&BtmPageVisitObserverSiteDataAccessBrowserTest::
GetActiveWebContents,
base::Unretained(this))) {}
void SetUpOnMainThread() override {
BtmPageVisitObserverBrowserTest::SetUpOnMainThread();
prerender_test_helper_.RegisterServerRequestMonitor(embedded_test_server());
}
WebContents* GetActiveWebContents() { return shell()->web_contents(); }
auto* fenced_frame_test_helper() { return &fenced_frame_test_helper_; }
auto* prerender_test_helper() { return &prerender_test_helper_; }
private:
test::FencedFrameTestHelper fenced_frame_test_helper_;
test::PrerenderTestHelper prerender_test_helper_;
};
IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverSiteDataAccessBrowserTest,
PrimaryMainFrameAccess) {
const GURL url1 =
embedded_https_test_server().GetURL("a.test", "/empty.html");
const GURL url2 =
embedded_https_test_server().GetURL("b.test", "/empty.html");
const GURL url3 =
embedded_https_test_server().GetURL("c.test", "/empty.html");
WebContents* web_contents = shell()->web_contents();
BtmPageVisitRecorder recorder(web_contents);
ASSERT_TRUE(NavigateToURL(web_contents, url1));
ASSERT_TRUE(AccessStorage(web_contents->GetPrimaryMainFrame(), GetParam()));
ASSERT_TRUE(NavigateToURL(web_contents, url2));
ASSERT_TRUE(NavigateToURL(web_contents, url3));
ASSERT_TRUE(recorder.WaitForSize(3));
EXPECT_THAT(
recorder.visits(),
ElementsAre(AllOf(PreviousPage(AllOf(HasUrl(GURL()),
HadQualifyingStorageAccess(false))),
HasUrl(url1)),
AllOf(PreviousPage(AllOf(HasUrl(url1),
HadQualifyingStorageAccess(true))),
HasUrl(url2)),
AllOf(PreviousPage(AllOf(HasUrl(url2),
HadQualifyingStorageAccess(false))),
HasUrl(url3))));
}
IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverSiteDataAccessBrowserTest,
IframeAccess) {
const GURL url1 = embedded_https_test_server().GetURL(
"a.test", "/page_with_blank_iframe.html");
const GURL url2 =
embedded_https_test_server().GetURL("b.test", "/empty.html");
const GURL url3 =
embedded_https_test_server().GetURL("c.test", "/empty.html");
WebContents* web_contents = shell()->web_contents();
BtmPageVisitRecorder recorder(web_contents);
ASSERT_TRUE(NavigateToURL(web_contents, url1));
RenderFrameHost* iframe = ChildFrameAt(web_contents, 0);
ASSERT_TRUE(AccessStorage(iframe, GetParam()));
ASSERT_TRUE(NavigateToURL(web_contents, url2));
ASSERT_TRUE(NavigateToURL(web_contents, url3));
ASSERT_TRUE(recorder.WaitForSize(3));
EXPECT_THAT(
recorder.visits(),
ElementsAre(AllOf(PreviousPage(AllOf(HasUrl(GURL()),
HadQualifyingStorageAccess(false))),
HasUrl(url1)),
AllOf(PreviousPage(AllOf(HasUrl(url1),
HadQualifyingStorageAccess(true))),
HasUrl(url2)),
AllOf(PreviousPage(AllOf(HasUrl(url2),
HadQualifyingStorageAccess(false))),
HasUrl(url3))));
}
IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverSiteDataAccessBrowserTest,
FencedFrameAccess) {
const GURL url1 =
embedded_https_test_server().GetURL("a.test", "/title1.html");
const GURL fenced_frame_url = embedded_https_test_server().GetURL(
"a.test", "/fenced_frames/title0.html");
const GURL url2 =
embedded_https_test_server().GetURL("b.test", "/empty.html");
WebContents* web_contents = shell()->web_contents();
BtmPageVisitRecorder recorder(web_contents);
ASSERT_TRUE(NavigateToURL(web_contents, url1));
std::unique_ptr<RenderFrameHostWrapper> fenced_frame =
std::make_unique<RenderFrameHostWrapper>(
fenced_frame_test_helper()->CreateFencedFrame(
web_contents->GetPrimaryMainFrame(), fenced_frame_url));
ASSERT_NE(fenced_frame, nullptr);
ASSERT_TRUE(AccessStorage(fenced_frame->get(), GetParam()));
ASSERT_TRUE(NavigateToURL(web_contents, url2));
ASSERT_TRUE(recorder.WaitForSize(2));
// Storage accesses in fenced frames should be ignored.
EXPECT_THAT(
recorder.visits(),
ElementsAre(AllOf(PreviousPage(AllOf(HasUrl(GURL()),
HadQualifyingStorageAccess(false))),
HasUrl(url1)),
AllOf(PreviousPage(AllOf(HasUrl(url1),
HadQualifyingStorageAccess(false))),
HasUrl(url2))));
}
IN_PROC_BROWSER_TEST_P(BtmPageVisitObserverSiteDataAccessBrowserTest,
PrerenderingAccess) {
// Prerendering pages do not have access to `StorageTypeAccessed::kFileSystem`
// until activation (AKA becoming the primary page, whose test case is already
// covered).
if (GetParam() == StorageTypeAccessed::kFileSystem) {
GTEST_SKIP();
}
const GURL url1 =
embedded_https_test_server().GetURL("a.test", "/title1.html");
const GURL prerendering_url =
embedded_https_test_server().GetURL("a.test", "/title2.html");
const GURL url2 =
embedded_https_test_server().GetURL("b.test", "/empty.html");
WebContents* web_contents = shell()->web_contents();
BtmPageVisitRecorder recorder(web_contents);
// Navigate to url1, and prerender a page that accesses storage.
ASSERT_TRUE(NavigateToURL(web_contents, url1));
const FrameTreeNodeId host_id =
prerender_test_helper()->AddPrerender(prerendering_url);
prerender_test_helper()->WaitForPrerenderLoadCompletion(prerendering_url);
test::PrerenderHostObserver observer(*web_contents, host_id);
ASSERT_FALSE(observer.was_activated());
RenderFrameHost* prerender_frame =
prerender_test_helper()->GetPrerenderedMainFrameHost(host_id);
ASSERT_NE(prerender_frame, nullptr);
ASSERT_TRUE(AccessStorage(prerender_frame, GetParam()));
prerender_test_helper()->CancelPrerenderedPage(host_id);
observer.WaitForDestroyed();
// End the page visit on url1.
ASSERT_TRUE(NavigateToURL(web_contents, url2));
ASSERT_TRUE(recorder.WaitForSize(2));
// Storage accesses in prerendered pages should be ignored.
EXPECT_THAT(
recorder.visits(),
ElementsAre(AllOf(PreviousPage(AllOf(HasUrl(GURL()),
HadQualifyingStorageAccess(false))),
HasUrl(url1)),
AllOf(PreviousPage(AllOf(HasUrl(url1),
HadQualifyingStorageAccess(false))),
HasUrl(url2))));
}
INSTANTIATE_TEST_SUITE_P(
All,
BtmPageVisitObserverSiteDataAccessBrowserTest,
::testing::Values(StorageTypeAccessed::kLocalStorage,
StorageTypeAccessed::kSessionStorage,
StorageTypeAccessed::kCacheStorage,
StorageTypeAccessed::kFileSystem,
StorageTypeAccessed::kIndexedDB),
[](const testing::TestParamInfo<
BtmPageVisitObserverSiteDataAccessBrowserTest::ParamType>& param_info) {
// We drop the first character of ToString(param) because it's just the
// constant-indicating 'k'.
return base::ToString(param_info.param).substr(1);
});
// WebAuthn tests do not work on Android because there is
// currently no way to install a virtual authenticator.
// TODO(crbug.com/40269763): Implement automated testing
// once the infrastructure permits it (Requires mocking
// the Android Platform Authenticator i.e. GMS Core).
// WebAuthn tests do not work on Android because there is currently no way to
// install a virtual authenticator.
// TODO(crbug.com/40269763): Implement automated testing once the infrastructure
// permits it (Requires mocking the Android Platform Authenticator i.e. GMS
// Core).
#if !BUILDFLAG(IS_ANDROID)
class BtmPageVisitObserverWebAuthnTest : public ContentBrowserTest {
public:

12
content/browser/dips/dips_bounce_detector_browsertest.cc

@ -458,6 +458,18 @@ class BtmBounceDetectorBrowserTest : public ContentBrowserTest {
}
}
[[nodiscard]] testing::AssertionResult AccessStorage(
RenderFrameHost* frame,
StorageTypeAccessed type) {
// We drop the first character of ToString(type) because it's just the
// constant-indicating 'k'.
return ExecJs(frame,
base::StringPrintf(kStorageAccessScript,
base::ToString(type).substr(1).c_str()),
EXECUTE_SCRIPT_NO_USER_GESTURE,
/*world_id=*/1);
}
auto* fenced_frame_test_helper() { return &fenced_frame_test_helper_; }
auto* prerender_test_helper() { return &prerender_test_helper_; }

12
content/browser/dips/dips_test_utils.cc

@ -48,18 +48,6 @@ base::expected<WebContents*, std::string> OpenInNewTab(
return tab_observer.window();
}
[[nodiscard]] testing::AssertionResult AccessStorage(
RenderFrameHost* frame,
blink::mojom::StorageTypeAccessed type) {
// We drop the first character of ToString(type) because it's just the
// constant-indicating 'k'.
return ExecJs(frame,
base::StringPrintf(kStorageAccessScript,
base::ToString(type).substr(1).c_str()),
EXECUTE_SCRIPT_NO_USER_GESTURE,
/*world_id=*/1);
}
void AccessCookieViaJSIn(WebContents* web_contents, RenderFrameHost* frame) {
FrameCookieAccessObserver observer(web_contents, frame,
CookieOperation::kChange);

4
content/browser/dips/dips_test_utils.h

@ -106,10 +106,6 @@ base::expected<WebContents*, std::string> OpenInNewTab(
WebContents* original_tab,
const GURL& url);
[[nodiscard]] testing::AssertionResult AccessStorage(
RenderFrameHost* frame,
blink::mojom::StorageTypeAccessed type);
// Helper function for performing client side cookie access via JS.
void AccessCookieViaJSIn(WebContents* web_contents, RenderFrameHost* frame);