Bug 180886 - Implement getting ServiceWorker registrations for the WKWebsiteDataStore API
Summary: Implement getting ServiceWorker registrations for the WKWebsiteDataStore API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-15 14:55 PST by Brady Eidson
Modified: 2017-12-16 17:17 PST (History)
3 users (show)

See Also:


Attachments
Patch (9.02 KB, patch)
2017-12-15 15:28 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (9.10 KB, patch)
2017-12-15 15:53 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (9.11 KB, patch)
2017-12-15 15:58 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (8.71 KB, patch)
2017-12-15 16:15 PST, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2017-12-15 14:55:23 PST
Implement getting ServiceWorker registrations for the WKWebsiteDataStore API
Comment 1 Brady Eidson 2017-12-15 15:28:35 PST
Created attachment 329525 [details]
Patch
Comment 2 Brady Eidson 2017-12-15 15:53:06 PST
Created attachment 329531 [details]
Patch
Comment 3 Chris Dumez 2017-12-15 15:57:41 PST
Comment on attachment 329525 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329525&action=review

r=me with comments. Also please fix GTK/WPE build.

> Source/WebCore/workers/service/server/SWServer.cpp:796
> +    while (!m_getOriginsWithRegistrationsCallbacks.isEmpty())

Why bother we the container mutations? How about we make m_getOriginsWithRegistrationsCallbacks a Vector and use:
auto getOriginsWithRegistrationsCallbacks = WTFMove(m_getOriginsWithRegistrationsCallbacks);
for (auto& callback : getOriginsWithRegistrationsCallbacks)
    callback(originsWithRegistrations);

Seems like this would be more efficient.

> Source/WebCore/workers/service/server/SWServer.h:238
> +    Deque<WTF::Function<void(const HashSet<SecurityOriginData>&)>> m_getOriginsWithRegistrationsCallbacks;

Seems like this could be a Vector.

> Source/WebKit/StorageProcess/StorageProcess.cpp:273
> +        for (auto& origin : *originsWithServiceWorkers)

Why isn't this in the swServerForSession(sessionID).getOriginsWithRegistrations() lambda? Similarly to what is done for IndexedDB? Then we do not need originsWithServiceWorkers / rawOrigins either.
Comment 4 Brady Eidson 2017-12-15 15:58:39 PST
Created attachment 329532 [details]
Patch
Comment 5 Brady Eidson 2017-12-15 16:15:32 PST
Created attachment 329535 [details]
Patch
Comment 6 WebKit Commit Bot 2017-12-16 10:54:09 PST
Comment on attachment 329535 [details]
Patch

Clearing flags on attachment: 329535

Committed r226003: <https://trac.webkit.org/changeset/226003>
Comment 7 WebKit Commit Bot 2017-12-16 10:54:10 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2017-12-16 10:56:41 PST
<rdar://problem/36091505>
Comment 9 Brady Eidson 2017-12-16 17:17:14 PST
The code landed in this patch begins to be tested over in https://bugs.webkit.org/show_bug.cgi?id=180911 \o/