Skip to content

[Site Isolation] WebKitTestRunner leaks isolated site records between tests and across runs - #71663

Open
szewai wants to merge 1 commit into
WebKit:mainfrom
szewai:eng/Site-Isolation-WebKitTestRunner-leaks-isolated-site-records-between-tests-and-across-runs
Open

[Site Isolation] WebKitTestRunner leaks isolated site records between tests and across runs#71663
szewai wants to merge 1 commit into
WebKit:mainfrom
szewai:eng/Site-Isolation-WebKitTestRunner-leaks-isolated-site-records-between-tests-and-across-runs

Conversation

@szewai

@szewai szewai commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

e59288a

[Site Isolation] WebKitTestRunner leaks isolated site records between tests and across runs
https://bugs.webkit.org/show_bug.cgi?id=321817
rdar://184954498

Reviewed by NOBODY (OOPS!).

configureWebsiteDataStoreTemporaryDirectories() redirects every website data directory WebKitTestRunner uses into its
temporary folder, but isolatedSitesDirectory was missing from that list, so it resolved to the real per-bundle path and
isolated site records are shared across runs. Records also survived between tests within a run, since
resetStateToConsistentValues() had nothing that cleared them.

Fixing these by redirecting the directory, and adding WebsiteDataType::IsolatedSiteRecord to the set
WKWebsiteDataStoreClearStorage() removes. Removal routes through IsolatedSiteStore::removeAllSites(), which removes site
records but does not clear import-from-ITP flag, so a reset does not cause a re-import.

testRunner.clearStorage() now clears these records too, which is what a test asking for a clean slate wants, and nothing
depends on them surviving it.

Drive by: give SiteIsolationSharedProcessEnabled defaultsOverridable: true and change its status from unstable to
internal, so it can be toggled through NSUserDefaults without a build change while it is being brought up. No
user-visible impact - the default value is unchanged, and GeneratePreferences.rb treats the two statuses identically.

No new tests: this is test infrastructure, and IsolatedSiteStore's persistence and removal behavior is already covered
by the TestWebKitAPI IsolatedSiteStore tests.

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreConfigurationRef.cpp:
(WKWebsiteDataStoreConfigurationCopyIsolatedSitesDirectory):
(WKWebsiteDataStoreConfigurationSetIsolatedSitesDirectory):
* Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreConfigurationRef.h:
* Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:
(WKWebsiteDataStoreClearStorage):
* Tools/WebKitTestRunner/TestController.cpp:
(WTR::TestController::configureWebsiteDataStoreTemporaryDirectories):

e59288a

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win ✅ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🧪 jsc-wpe
✅ 🧪 jsc-x86-64 ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk3-gcc
✅ 🛠 🧪 jsc-debug-arm64 ✅ 🛠 ios-safer-cpp ✅ 🧪 mac-AS-debug-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-intel-wk2 ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🛠 mac-safer-cpp ✅ 🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-site-isolation
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

… tests and across runs

https://bugs.webkit.org/show_bug.cgi?id=321817
rdar://184954498

Reviewed by NOBODY (OOPS!).

configureWebsiteDataStoreTemporaryDirectories() redirects every website data directory WebKitTestRunner uses into its
temporary folder, but isolatedSitesDirectory was missing from that list, so it resolved to the real per-bundle path and
isolated site records are shared across runs. Records also survived between tests within a run, since
resetStateToConsistentValues() had nothing that cleared them.

Fixing these by redirecting the directory, and adding WebsiteDataType::IsolatedSiteRecord to the set
WKWebsiteDataStoreClearStorage() removes. Removal routes through IsolatedSiteStore::removeAllSites(), which removes site
records but does not clear import-from-ITP flag, so a reset does not cause a re-import.

testRunner.clearStorage() now clears these records too, which is what a test asking for a clean slate wants, and nothing
depends on them surviving it.

Drive by: give SiteIsolationSharedProcessEnabled defaultsOverridable: true and change its status from unstable to
internal, so it can be toggled through NSUserDefaults without a build change while it is being brought up. No
user-visible impact - the default value is unchanged, and GeneratePreferences.rb treats the two statuses identically.

No new tests: this is test infrastructure, and IsolatedSiteStore's persistence and removal behavior is already covered
by the TestWebKitAPI IsolatedSiteStore tests.

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreConfigurationRef.cpp:
(WKWebsiteDataStoreConfigurationCopyIsolatedSitesDirectory):
(WKWebsiteDataStoreConfigurationSetIsolatedSitesDirectory):
* Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreConfigurationRef.h:
* Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:
(WKWebsiteDataStoreClearStorage):
* Tools/WebKitTestRunner/TestController.cpp:
(WTR::TestController::configureWebsiteDataStoreTemporaryDirectories):
@szewai szewai self-assigned this Aug 14, 2026
@szewai szewai added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Aug 14, 2026
@szewai
szewai marked this pull request as ready for review August 15, 2026 00:25
@szewai
szewai requested a review from rniwa August 15, 2026 00:25
@@ -1026,6 +1026,7 @@ void TestController::configureWebsiteDataStoreTemporaryDirectories(WKWebsiteData
WKWebsiteDataStoreConfigurationSetMediaKeysStorageDirectory(configuration, toWK(makeString(temporaryFolder, pathSeparator, "MediaKeys"_s, pathSeparator, randomNumber)).get());
WKWebsiteDataStoreConfigurationSetResourceLoadStatisticsDirectory(configuration, toWK(makeString(temporaryFolder, pathSeparator, "ResourceLoadStatistics"_s, pathSeparator, randomNumber)).get());
WKWebsiteDataStoreConfigurationSetServiceWorkerRegistrationDirectory(configuration, toWK(makeString(temporaryFolder, pathSeparator, "ServiceWorkers"_s, pathSeparator, randomNumber)).get());
WKWebsiteDataStoreConfigurationSetIsolatedSitesDirectory(configuration, toWK(makeString(temporaryFolder, pathSeparator, "IsolatedSites"_s, pathSeparator, randomNumber)).get());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want to delete this file afterwards?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Bugs Unclassified bugs are placed in this component until the correct component can be determined.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants