Optimize WebsiteDataStoreConfiguration::copy
*** Bug 215011 has been marked as a duplicate of this bug. ***
Created attachment 405670 [details] Patch
<rdar://problem/64263406>
Committed r265134: <https://trac.webkit.org/changeset/265134> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405670 [details].
This actually wasn't quite enough! Need to apply the flag to the WebsiteDataStore constructor as well: WebsiteDataStore::WebsiteDataStore(Ref<WebsiteDataStoreConfiguration>&& configuration, PAL::SessionID sessionID) : m_sessionID(sessionID) , m_resolvedConfiguration(WTFMove(configuration)) , m_configuration((m_resolvedConfiguration)->copy()) <-------Here I'll followup in the morning
That calls WebsiteDataStoreConfiguration::copy() which benefits from this optimization.
Comment on attachment 405670 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405670&action=review > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreConfiguration.h:36 > +enum class WillCopyPathsFromExistingConfiguration : bool { No, Yes }; What's important here is not necessarily that the paths came from some other configuration, but rather that the paths are already fully resolved -- right? If so, perhaps a better enum would be PathsAreFullyResolved { No, Yes }. Global knowledge of the code and logical deduction can tell us that a path from an existing configuration is a fully resolved path, but it is best if reading code doesn't require global knowledge or logical deduction.
(In reply to Alex Christensen from comment #6) > That calls WebsiteDataStoreConfiguration::copy() which benefits from this > optimization. Yup I totally misread it late at night, we're good.
What's really important here is that we don't do disk read and write operations during WebsiteDataStoreConfiguration::copy. This was one solution, but I think a better one would be do std::call_once in each of the WebsiteDataStore::default*Directory in WebsiteDataStoreCocoa.mm and remove this strange looking code from WebsiteDataStoreConfiguration. That's not urgent, though, because this bug is fixed and we don't touch WebsiteDataStoreConfiguration much.