RESOLVED FIXED 238402
Don't create directories on iOS if we are only using ephemeral storages
https://bugs.webkit.org/show_bug.cgi?id=238402
Summary Don't create directories on iOS if we are only using ephemeral storages
Alex Christensen
Reported 2022-03-25 19:02:24 PDT
Don't create directories on iOS if we are only using ephemeral storages
Attachments
Patch (20.70 KB, patch)
2022-03-25 19:05 PDT, Alex Christensen
no flags
Patch (33.09 KB, patch)
2022-03-28 10:52 PDT, Alex Christensen
cdumez: review+
ews-feeder: commit-queue-
Alex Christensen
Comment 1 2022-03-25 19:05:21 PDT
Chris Dumez
Comment 2 2022-03-26 13:55:13 PDT
Looks like iOS failures may be real?
Alex Christensen
Comment 3 2022-03-28 10:39:18 PDT
Comment on attachment 455813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=455813&action=review > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:334 > + if (!process.sessionID().isEphemeral()) { This was asserting because when we initialize the web process we don't always have a session ID yet. I'm moving this to WebProcess::setWebsiteDataStoreParameters
Alex Christensen
Comment 4 2022-03-28 10:52:28 PDT
Chris Dumez
Comment 5 2022-03-28 13:32:48 PDT
Comment on attachment 455929 [details] Patch r=me assuming the bots are happy.
Sihui Liu
Comment 6 2022-03-28 14:58:51 PDT
Comment on attachment 455929 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=455929&action=review > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:290 > + m_cookieStorageDirectory = resolveAndCreateReadWriteDirectoryForSandboxExtension(WebProcessPool::cookieStorageDirectory()); It's a bit weird that WebsiteDataStore asks WebProcessPool for directory. I If we think the directory is per WebsiteDataStore, maybe we should move cookieStorageDirectory() to WebsiteDataStore? If we don't think it's per WebsiteDataStore, maybe we should not send it in WebsiteDataStoreParameters? > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1905 > + if (String cookieStorageDirectory = WebProcessPool::cookieStorageDirectory(); !cookieStorageDirectory.isEmpty()) If we already call resolveDirectoriesIfNecessary() above, shouldn't we just check if (!m_cookieStorageDirectory.isEmpty())?
Alex Christensen
Comment 7 2022-03-29 12:30:27 PDT
This is indeed weird code that is quite old so I'm trying to minimize functional changes. It is taking another step in its transition from process pool to data store.
Alex Christensen
Comment 8 2022-03-29 13:57:13 PDT
Alex Christensen
Comment 9 2022-03-29 13:57:51 PDT
(by the way, I agree with Sihui's cleanup suggestions and think they should be done, but not right now)
Radar WebKit Bug Importer
Comment 10 2022-03-29 21:06:35 PDT
David Kilzer (:ddkilzer)
Comment 11 2022-03-29 21:07:15 PDT
Note You need to log in before you can comment on or make changes to this bug.