Bug 234179 – Move SWServers from NetworkProcess to NetworkSession

• Component: Service Workers
• Status: RESOLVED FIXED

[Chris Dumez]

Move SWServers from NetworkProcess to NetworkSession, since they are per session.

[Chris Dumez]

Created attachment 446824 [details]
Patch

[Chris Dumez]

Created attachment 446909 [details]
Patch

[Chris Dumez]

ping review?

[Darin Adler]

Comment on attachment 446909 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:504
> +        auto& server = session->ensureSWServer();
> +        if (!server.isImportCompleted()) {

Scope it using the semicolon style?

    if (auto& server = session->ensureSWServer(); !server.isImportCompleted()) {

> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:509
> +                ASSERT(networkSession() && networkSession()->swServer() && networkSession()->swServer()->isImportCompleted());

I would write three separate assertions. Nicer to know which one failed.

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:1596
> +            // If we are deleting all of the data types that the resource load statistics store monitors
> +            // we do not need to re-grandfather old data.
> +            auto shouldGrandfather = ((monitoredTypesRaw & deletedTypesRaw) == monitoredTypesRaw) ? ShouldGrandfatherStatistics::No : ShouldGrandfatherStatistics::Yes;

The enum makes this harder to read. I hope there’s other code that makes it easier to read.

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:1664
> +        Vector<String> hosts;
> +        hosts.reserveInitialCapacity(originDatas.size());
> +        for (auto& origin : originDatas)
> +            hosts.uncheckedAppend(origin.host);

Should do this more directly with Vector::map?

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:1849
> +        Vector<String> registrableDomainsToDelete;
> +        registrableDomainsToDelete.reserveInitialCapacity(domainsToDeleteAllNonCookieWebsiteDataFor.size());
> +        for (auto& domain : domainsToDeleteAllNonCookieWebsiteDataFor)
> +            registrableDomainsToDelete.uncheckedAppend(domain.string());

Same question.

> Source/WebKit/NetworkProcess/NetworkSession.cpp:553
> +    UNUSED_PARAM(critical);

Could we omit the argument name instead of using UNUSED_PARAM?

[Chris Dumez]

Created attachment 447236 [details]
Patch

[EWS]

Committed r287084 (245279@main): <https://commits.webkit.org/245279@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447236 [details].

[Radar WebKit Bug Importer]

<rdar://problem/86530837>