Bug 201921

Summary: NetworkLoadParameters shouldn't store sessionID
Product: WebKit Reporter: Ben Nham <nham>
Component: WebKit2Assignee: Ben Nham <nham>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, cgarcia, commit-queue, ews-watchlist, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Ben Nham
Reported 2019-09-18 10:37:34 PDT
Now that there's a single session id per WebProcess, we don't need to store the session id in NetworkLoadParameters anymore. Clients can just directly ask NetworkConnectionToWebProcess for the WebProcess's session ID instead.
Attachments
Patch (27.90 KB, patch)
2019-09-18 11:50 PDT, Ben Nham 		no flags
DetailsFormatted DiffDiff
Patch (27.78 KB, patch)
2019-09-18 12:06 PDT, Ben Nham 		no flags
DetailsFormatted DiffDiff
Patch (27.74 KB, patch)
2019-09-18 12:33 PDT, Ben Nham 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ben Nham
Comment 1 2019-09-18 11:50:18 PDT
Created attachment 379055 [details] Patch
Chris Dumez
Comment 2 2019-09-18 11:57:49 PDT
Comment on attachment 379055 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379055&action=review > Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.cpp:-120 > - if (!decoder.decode(result.identifier)) Didn't you drop this one by mistake?
Ben Nham
Comment 3 2019-09-18 12:06:28 PDT
Created attachment 379057 [details] Patch
Ben Nham
Comment 4 2019-09-18 12:07:04 PDT
Yep, I deleted the extra line in the decoding function which broke website loading completely. Should be fixed in the newest patch.
Chris Dumez
Comment 5 2019-09-18 12:14:39 PDT
Comment on attachment 379057 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379057&action=review r=me with nits. > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:304 > + auto* networkSession = m_connection->networkProcess().networkSession(sessionID()); I believe this could be: auto* networkSession = m_connection->networkSession(); > Source/WebKit/NetworkProcess/PreconnectTask.h:43 > + explicit PreconnectTask(NetworkProcess&, PAL::SessionID, NetworkLoadParameters&&, CompletionHandler<void(const WebCore::ResourceError&)>&& completionHandler = { }); nit: we should probably drop the explicit.
Ben Nham
Comment 6 2019-09-18 12:33:32 PDT
Created attachment 379061 [details] Patch
Chris Dumez
Comment 7 2019-09-18 12:34:43 PDT
Let's wait for green bubbles before committing.
WebKit Commit Bot
Comment 8 2019-09-18 14:31:34 PDT
Comment on attachment 379061 [details] Patch Clearing flags on attachment: 379061 Committed r250053: <https://trac.webkit.org/changeset/250053>
WebKit Commit Bot
Comment 9 2019-09-18 14:31:35 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2019-09-18 14:32:15 PDT
<rdar://problem/55493060>
Note You need to log in before you can comment on or make changes to this bug.