Bug 223776 – Refactor NetworkSessionCocoa to prepare for per-WebPageProxy sessions

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brady Eidson]

Refactor NetworkSessionCocoa to prepare for per-WebPageProxy sessions

For upcoming work, different WKWebViews sharing one logical NetworkSession will need different NSURLSessions to do their networking.

To prepare for that, this patch will do a no-change-in-behavior refactor that accounts for the ability to "look up the set of NSURLSessions by WebPageProxyIdentifier"

[Brady Eidson]

Created attachment 424300 [details]
Patch for EWS

[Brady Eidson]

That makes sense, yay EWS

[Brady Eidson]

Created attachment 424308 [details]
Patch for EWS

[Brady Eidson]

Created attachment 424319 [details]
Patch for EWS

[Alex Christensen]

Comment on attachment 424319 [details]
Patch for EWS

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

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.h:159
> +    HashMap<WebPageProxyIdentifier, RefPtr<SessionSet>> m_perPageSessionSets;

I think the value could be made a Ref

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1298
> +    // FIXME: https://bugs.webkit.org/show_bug.cgi?id=177394

You may as well remove this comment and the 2 lines below.  We know why we can't do this, and we typically don't like having commented out code in WebKit.

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1307
> +    SessionSet* sessionSet = webPageProxyID ? m_perPageSessionSets.get(webPageProxyID) : nullptr;

If webPageProxyID can be 0 then it should probably be an Optional<WebPageProxyIdentifier>
If you're protecting against strange things happening to the HashTable, you should probably use decltype(m_perPageSessionSets.get)::isValidKey

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1319
> +    return sessionSetForPage(pageID).initializeEphemeralStatelessSessionIfNeeded(isNavigatingToAppBoundDomain, *this);

You call pageID webPageProxyID everywhere else in this patch.  Why not here?

[Brady Eidson]

(In reply to Alex Christensen from comment #5)
> Comment on attachment 424319 [details]
> Patch for EWS

All suggestions taken except:

> If webPageProxyID can be 0 then it should probably be an
> Optional<WebPageProxyIdentifier>
> If you're protecting against strange things happening to the HashTable, you
> should probably use decltype(m_perPageSessionSets.get)::isValidKey

In this patch it can be 0 (and in fact PingLoaders do not necessarily have a proper page context, so they are 0)

In the next patch there will be protection against zero.

ObjectIdentifier<> already has established patterns of "0 means no valid ID", I don't think we need to explore a new direction on that this time around.

[Brady Eidson]

Created attachment 424391 [details]
Patch for landing

[EWS]

Committed r275116: <https://commits.webkit.org/r275116>

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

[Radar WebKit Bug Importer]

<rdar://problem/75903385>