Summary: | Resource Load Statistics: IPC to the WebsiteDataStore in the UI process from NetworkProcess::deleteWebsiteDataForRegistrableDomains() | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | John Wilander <wilander> | ||||||||||||||
Component: | WebKit Misc. | Assignee: | John Wilander <wilander> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | achristensen, beidson, bfulgham, commit-queue, ews-watchlist | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
John Wilander
2019-03-26 17:27:04 PDT
Created attachment 366029 [details]
Patch
Attachment 366029 [details] did not pass style-queue:
ERROR: Source/WebKit/Shared/WebsiteData/WebsiteData.h:42: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
Total errors found: 1 in 25 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to Build Bot from comment #3) > Attachment 366029 [details] did not pass style-queue: > > > ERROR: Source/WebKit/Shared/WebsiteData/WebsiteData.h:42: enum members > should use InterCaps with an initial capital letter or initial 'k' for > C-style enums. [readability/enum_casing] [4] > Total errors found: 1 in 25 files > > > If any of these errors are false positives, please file a bug against > check-webkit-style. I belive the style checker is wrong, complaining about the capitalization of: enum class WebsiteDataProcessType { Network, UI }; Non-Cocoa build failures seem to be about a new convenience function not being used. It’s currently only used in my code which is behind the Resource Load Statistics flag. The mac-debug test failures are unrelated. Created attachment 366088 [details]
Patch
Comment on attachment 366029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366029&action=review > Source/WebKit/NetworkProcess/NetworkProcess.cpp:1688 > + parentProcessConnection()->send(Messages::NetworkProcessProxy::DeleteWebsiteDataInUIProcessForRegistrableDomains(sessionID, dataTypesForUIProcess, fetchOptions, domainsToDeleteAllButCookiesFor, callbackID), 0); This seems like a prime candidate for sendWithAsyncReply. > Source/WebKit/UIProcess/WebsiteData/WebsiteDataRecord.cpp:117 > +bool WebsiteDataRecord::matches(const RegistrableDomain& domain) const This needs a WebCore:: or we need a using namespace WebCore inside the namespace WebKit. > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:2075 > + WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("IsStatisticsHasLocalStorage")); auto messageName = adoptWK(...) > Tools/WebKitTestRunner/TestInvocation.cpp:1435 > + WKRetainPtr<WKTypeRef> result(AdoptWK, WKBooleanCreate(hasLocalStorage)); auto result = adoptWK(...) Attachment 366088 [details] did not pass style-queue:
ERROR: Source/WebKit/Shared/WebsiteData/WebsiteData.h:42: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
Total errors found: 1 in 24 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Sorry, I saw your comments after preparing the latest patch. (In reply to Alex Christensen from comment #7) > Comment on attachment 366029 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=366029&action=review > > > Source/WebKit/NetworkProcess/NetworkProcess.cpp:1688 > > + parentProcessConnection()->send(Messages::NetworkProcessProxy::DeleteWebsiteDataInUIProcessForRegistrableDomains(sessionID, dataTypesForUIProcess, fetchOptions, domainsToDeleteAllButCookiesFor, callbackID), 0); > > This seems like a prime candidate for sendWithAsyncReply. Yes. Chris pointed this out in-person. Fixed in the latest patch. > > Source/WebKit/UIProcess/WebsiteData/WebsiteDataRecord.cpp:117 > > +bool WebsiteDataRecord::matches(const RegistrableDomain& domain) const > > This needs a WebCore:: or we need a using namespace WebCore inside the > namespace WebKit. Will fix. > > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:2075 > > + WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("IsStatisticsHasLocalStorage")); > > auto messageName = adoptWK(...) Will fix. > > Tools/WebKitTestRunner/TestInvocation.cpp:1435 > > + WKRetainPtr<WKTypeRef> result(AdoptWK, WKBooleanCreate(hasLocalStorage)); > > auto result = adoptWK(...) Will fix. Thanks! Created attachment 366091 [details]
Patch
Attachment 366091 [details] did not pass style-queue:
ERROR: Source/WebKit/Shared/WebsiteData/WebsiteData.h:42: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
Total errors found: 1 in 24 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Non-Cocoa build failures are probably because of a missing header include. Created attachment 366125 [details]
Patch
Attachment 366125 [details] did not pass style-queue:
ERROR: Source/WebKit/Shared/WebsiteData/WebsiteData.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
Total errors found: 1 in 24 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 366125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366125&action=review > Source/WebKit/Shared/WebsiteData/WebsiteData.cpp:98 > + case WebsiteDataType::MemoryCache: > + return WebsiteDataProcessType::UI; This is in the web process. > Source/WebKit/Shared/WebsiteData/WebsiteData.cpp:106 > + case WebsiteDataType::WebSQLDatabases: > + return WebsiteDataProcessType::UI; I'm not sure if this is correct. I thought this was in the Network process. Comment on attachment 366125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366125&action=review > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:627 > +void WebsiteDataStore::fetchDataForRegistrableDomains(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, const Vector<RegistrableDomain>& domains, CompletionHandler<void(Vector<WebsiteDataRecord>&&, HashSet<RegistrableDomain>&&)>&& completionHandler) Same needing WebCore:: or using namespace WebCore here. (In reply to Alex Christensen from comment #15) > Comment on attachment 366125 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=366125&action=review > > > Source/WebKit/Shared/WebsiteData/WebsiteData.cpp:98 > > + case WebsiteDataType::MemoryCache: > > + return WebsiteDataProcessType::UI; > > This is in the web process. Sure. My thinking was that the WebsiteDataStore in the UI process manages removal of this type but you're absolutely right. > > Source/WebKit/Shared/WebsiteData/WebsiteData.cpp:106 > > + case WebsiteDataType::WebSQLDatabases: > > + return WebsiteDataProcessType::UI; > > I'm not sure if this is correct. I thought this was in the Network process. Doesn't look like it. WebKit::NetworkProcess doesn't handle the type whereas WebKit::WebsiteDataStore does, like so: if (dataTypes.contains(WebsiteDataType::WebSQLDatabases) && isPersistent()) { callbackAggregator->addPendingCallback(); m_queue->dispatch([webSQLDatabaseDirectory = m_configuration->webSQLDatabaseDirectory().isolatedCopy(), callbackAggregator, modifiedSince] { WebCore::DatabaseTracker::trackerWithDatabasePath(webSQLDatabaseDirectory)->deleteDatabasesModifiedSince(modifiedSince); RunLoop::main().dispatch([callbackAggregator] { callbackAggregator->removePendingCallback(); }); }); } Created attachment 366198 [details]
Patch
Attachment 366198 [details] did not pass style-queue:
ERROR: Source/WebKit/Shared/WebsiteData/WebsiteData.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
Total errors found: 1 in 24 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 366206 [details]
Patch
Trying to fix the Wincairo build failure. :/ Attachment 366206 [details] did not pass style-queue:
ERROR: Source/WebKit/Shared/WebsiteData/WebsiteData.h:43: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4]
Total errors found: 1 in 24 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Filed https://bugs.webkit.org/show_bug.cgi?id=196374 on the style checker issue. Comment on attachment 366206 [details]
Patch
Thank you, Alex!
Comment on attachment 366206 [details] Patch Clearing flags on attachment: 366206 Committed r243632: <https://trac.webkit.org/changeset/243632> All reviewed patches have been landed. Closing bug. |