Bug 226910

Summary: Partition CrossOriginPreflightResultCache by SessionID
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, darin, ews-watchlist, japhet, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

Description Alex Christensen 2021-06-10 20:13:40 PDT
Partition CrossOriginPreflightResultCache by SessionID
Comment 1 Alex Christensen 2021-06-10 20:14:32 PDT
Created attachment 431172 [details]
Patch
Comment 2 Alex Christensen 2021-06-10 21:30:38 PDT
Created attachment 431179 [details]
Patch
Comment 3 youenn fablet 2021-06-10 23:43:17 PDT
Comment on attachment 431179 [details]
Patch


Can we get an API test for this one?
Something like:
- load a page with a sessionID, trigger a preflight and verify server has received a preflight.
- load another page in same pool but with a different sessionID, trigger the same prefllight and verify again the server has received the preflight.

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

> Source/WebCore/loader/CrossOriginAccessControl.cpp:260
> +Expected<void, String> validatePreflightResponse(const PAL::SessionID& sessionID, const ResourceRequest& request, const ResourceResponse& response, StoredCredentialsPolicy storedCredentialsPolicy, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)

s/const PAL::SessionID&/PAL::SessionID

> Source/WebCore/loader/CrossOriginAccessControl.h:85
> +WEBCORE_EXPORT Expected<void, String> validatePreflightResponse(const PAL::SessionID&, const ResourceRequest&, const ResourceResponse&, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);

Ditto here and below I guess.
Comment 4 Alex Christensen 2021-06-11 09:16:29 PDT
That's a good idea for a test.  Will do.
const PAL::SessionID& is used so we can only forward declare SessionID.
Comment 5 Alex Christensen 2021-06-11 10:55:46 PDT
Created attachment 431210 [details]
Patch
Comment 6 EWS 2021-06-11 17:06:28 PDT
Committed r278800 (238757@main): <https://commits.webkit.org/238757@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 431210 [details].
Comment 7 Radar WebKit Bug Importer 2021-06-11 17:07:22 PDT
<rdar://problem/79223523>
Comment 8 Darin Adler 2021-06-13 12:21:45 PDT
Comment on attachment 431210 [details]
Patch

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

> Source/WebCore/loader/CrossOriginAccessControl.h:85
> +WEBCORE_EXPORT Expected<void, String> validatePreflightResponse(const PAL::SessionID&, const ResourceRequest&, const ResourceResponse&, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);

Since a SessionID is just a single 64-bit integer, I suggest we pass it by value, not const&.
Comment 9 Alex Christensen 2021-06-14 12:38:15 PDT
(In reply to Darin Adler from comment #8)
> Comment on attachment 431210 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=431210&action=review
> 
> > Source/WebCore/loader/CrossOriginAccessControl.h:85
> > +WEBCORE_EXPORT Expected<void, String> validatePreflightResponse(const PAL::SessionID&, const ResourceRequest&, const ResourceResponse&, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
> 
> Since a SessionID is just a single 64-bit integer, I suggest we pass it by
> value, not const&.

I'm doing this in bug 226983