From 9971db0e0ea70fcea11bc4961fe37043dfac32e6 Mon Sep 17 00:00:00 2001 From: Netzenbot Date: Thu, 7 May 2026 11:57:37 -0400 Subject: [PATCH] Fix cookie IPC race in browsing data test utility (#36125) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../content/browsing_data_test_util.cc | 40 +++++++++++++++++++ test/filters/browser_tests.filter | 10 +++-- 2 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 chromium_src/components/browsing_data/content/browsing_data_test_util.cc diff --git a/chromium_src/components/browsing_data/content/browsing_data_test_util.cc b/chromium_src/components/browsing_data/content/browsing_data_test_util.cc new file mode 100644 index 00000000000..5f9d658bfdd --- /dev/null +++ b/chromium_src/components/browsing_data/content/browsing_data_test_util.cc @@ -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()"), verify the data is accessible via +// EvalJs("has()"). 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 // 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 diff --git a/test/filters/browser_tests.filter b/test/filters/browser_tests.filter index eb676dfe5e9..2472c78a603 100644 --- a/test/filters/browser_tests.filter +++ b/test/filters/browser_tests.filter @@ -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/*