Fix cookie IPC race in browsing data test utility (#36125)
The upstream Chromium test BrowserContextDestructionVsCookieRemoval flakes because document.cookie (set via JavaScript in the renderer) uses a different Mojo pipe than GetAllCookies (queried from the browser via CookieManager), with no ordering guarantee between the two. Brave amplifies this from the upstream ~0.6 % rate because Ephemeral Storage forces every cookie operation through IPC (disabling Chromium's cookie cache optimization in CookieJar::IPCNeeded). Add a chromium_src override for browsing_data_test_util.cc that calls HasDataForType after SetDataForType. For cookies this forces a round-trip through RestrictedCookieManager → CookieStore, serialising with the prior SetCanonicalCookieAsync on the same CookieMonster task runner and guaranteeing the cookie is committed before any subsequent GetAllCookies query. The BrowserContextDestructionVsCookieRemoval test has a separate issue: BlockUntilCompletion() never returns after the incognito profile is destroyed during data removal. CookieIncognitoDeletion also hangs in incognito mode. Both remain disabled in the filter file. Resolves brave/brave-browser#54537
This commit is contained in:
@@ -0,0 +1,40 @@
|
||||
/* Copyright (c) 2026 The Brave Authors. All rights reserved.
|
||||
* This Source Code Form is subject to the terms of the Mozilla Public
|
||||
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
|
||||
* You can obtain one at https://mozilla.org/MPL/2.0/. */
|
||||
|
||||
// Brave disables Chromium's cookie IPC cache optimization (because Ephemeral
|
||||
// Storage can switch the cookie backend at runtime), which means
|
||||
// document.cookie always uses IPC. When a test sets a cookie via
|
||||
// document.cookie and immediately queries the backend cookie store through a
|
||||
// *different* Mojo pipe (CookieManager::GetAllCookies), there is no ordering
|
||||
// guarantee between the two pipes. Upstream Chromium sees this as a ~0.6 %
|
||||
// flake; Brave amplifies it because every cookie read/write goes through IPC.
|
||||
//
|
||||
// Fix: after ExecJs("set<Type>()"), verify the data is accessible via
|
||||
// EvalJs("has<Type>()"). For cookies this forces a round-trip through
|
||||
// RestrictedCookieManager → CookieStore, serialising with the prior
|
||||
// SetCanonicalCookieAsync on the same CookieMonster task runner and
|
||||
// guaranteeing the cookie is committed before any subsequent GetAllCookies.
|
||||
|
||||
#define SetDataForType SetDataForType_ChromiumImpl
|
||||
|
||||
#include <components/browsing_data/content/browsing_data_test_util.cc> // IWYU pragma: export
|
||||
|
||||
#undef SetDataForType
|
||||
|
||||
namespace browsing_data_test_util {
|
||||
|
||||
void SetDataForType(const std::string& type,
|
||||
content::WebContents* web_contents) {
|
||||
SetDataForType_ChromiumImpl(type, web_contents);
|
||||
// Round-trip through RestrictedCookieManager to synchronise the CookieStore.
|
||||
EXPECT_TRUE(HasDataForType(type, web_contents));
|
||||
}
|
||||
|
||||
void SetDataForType(const std::string& type,
|
||||
content::RenderFrameHost* render_frame_host) {
|
||||
SetDataForType_ChromiumImpl(type, render_frame_host);
|
||||
}
|
||||
|
||||
} // namespace browsing_data_test_util
|
||||
@@ -1944,10 +1944,12 @@
|
||||
# `kActionSendSharedTabGroupFeedback` ephemerally and triggers a CHECK.
|
||||
-VerticalTabViewDataSharingEnabledTest.LogsTabSwitchMetrics
|
||||
|
||||
# Chromium test flaky - disabled on Windows upstream (crbug.com/413259587). Race
|
||||
# condition between JavaScript cookie set via document.cookie and backend
|
||||
# SiteDataCountingHelper query. Brave has no modifications to this code.
|
||||
# Upstream flake rate 0.5% (LUCI Analysis, 30-day lookback); amplified by MSAN.
|
||||
# Chromium test: BrowsingDataRemover BrowserContext destruction hangs in Brave.
|
||||
# BlockUntilCompletion() never returns after incognito profile is destroyed
|
||||
# during data removal (crbug.com/413259587). Also hits a cookie IPC race
|
||||
# (GetSiteDataCount returns 0) that is fixed by the chromium_src override of
|
||||
# browsing_data_test_util.cc, but the hang remains. Upstream flake rate 0.6%
|
||||
# (LUCI Analysis, 30-day lookback).
|
||||
-All/BrowsingDataRemoverBrowserTestP.BrowserContextDestructionVsCookieRemoval/*
|
||||
-All/BrowsingDataRemoverBrowserTestP.CookieIncognitoDeletion/*
|
||||
|
||||
|
||||
Reference in New Issue
Block a user