Summary: | Merge ServiceWorkerClientIdentifier into ServiceWorkerClientData | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | beidson, commit-queue, dbates, esprehn+autocc, ews-watchlist, kangil.han, webkit-bug-importer, youennf | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=180683 | ||||||||||
Attachments: |
|
Description
Chris Dumez
2017-12-11 13:49:33 PST
Created attachment 329030 [details]
Patch
Created attachment 329031 [details]
Patch
Comment on attachment 329031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=329031&action=review > Source/WebCore/dom/Document.cpp:7630 > + m_serviceWorkerConnection->unregisterServiceWorkerClient({ m_serviceWorkerConnection->serverConnectionIdentifier(), identifier() }); I wonder whether we should not have made SWClientConnection only take DocumentIdentifier and do the translation to ServiceWorkerClientIdentifier on its own. Maybe this is too late for that. If not we could stick with passing a DocumentIdentifier to unregisterServieWorkerClient. > Source/WebCore/workers/service/ServiceWorkerClientData.cpp:65 > + RELEASE_ASSERT(isDocument); // We do not support dedicated workers as clients yet. Isn't the downcast<Document> doing this release assert already? > Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:121 > + auto sourceClient = ServiceWorkerClient::getOrCreate(serviceWorkerGlobalScope, WTFMove(WTF::get<ServiceWorkerClientData>(sourceData))); Maybe use RefPtr<ServiceWorkerClient> here instead of auto so that there is not the need for it below for source =...? > Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h:93 > + void unregisterServiceWorkerClient(const WebCore::ServiceWorkerClientIdentifier&); Sometimes we are using const ServiceWorkerClientIdentifier&, sometimes ServiceWorkerClientIdentifier directly. Comment on attachment 329031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=329031&action=review >> Source/WebCore/workers/service/ServiceWorkerClientData.cpp:65 >> + RELEASE_ASSERT(isDocument); // We do not support dedicated workers as clients yet. > > Isn't the downcast<Document> doing this release assert already? Nah, only a ASSERT_WITH_SECURITY_IMPLICATION() (i.e. debug only) Created attachment 329042 [details]
Patch
Comment on attachment 329042 [details] Patch Clearing flags on attachment: 329042 Committed r225760: <https://trac.webkit.org/changeset/225760> All reviewed patches have been landed. Closing bug. |