Bug 162258

Summary: CachedResourceRequest should store a SecurityOrigin
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, cdumez, commit-queue, dbates, japhet, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 162151    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Rebasing after bug 162260 none

Description youenn fablet 2016-09-19 23:28:09 PDT
That way, it will be able to pass it to CachedResource at construction time.
Currently it is based on Origin header or Document origin.
In the first case, we lose some information like universal access.
Comment 1 youenn fablet 2016-09-20 00:27:54 PDT
Created attachment 289332 [details]
Patch
Comment 2 youenn fablet 2016-09-20 01:10:20 PDT
Created attachment 289337 [details]
Patch
Comment 3 youenn fablet 2016-09-20 01:56:18 PDT
Created attachment 289338 [details]
Patch
Comment 4 Sam Weinig 2016-09-20 08:37:57 PDT
Comment on attachment 289338 [details]
Patch

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

> Source/WebCore/loader/cache/CachedResourceRequest.h:81
> +    RefPtr<SecurityOrigin> m_origin;

In what scenarios can this be null (other than when we release it).  Can we require setting the origin in the constructor?  Also, should this really be a SecurityOrigin, or would a SecuirtyOriginData work (we need to establish clear rules on this, cc. andersca)?
Comment 5 youenn fablet 2016-09-20 08:52:12 PDT
(In reply to comment #4)
> Comment on attachment 289338 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289338&action=review
> 
> > Source/WebCore/loader/cache/CachedResourceRequest.h:81
> > +    RefPtr<SecurityOrigin> m_origin;
> 
> In what scenarios can this be null (other than when we release it).  Can we
> require setting the origin in the constructor?

It is null when fetching main resources during navigation.
We could require setting it within the constructor but most of the time it will be the context securityOrigin.

In the fetch spec, the origin is either an origin or set to the special "client" value, which we represents as a null RefPtr. In both spec and implementation, this is the default value.

> Also, should this really be
> a SecurityOrigin, or would a SecuirtyOriginData work (we need to establish
> clear rules on this, cc. andersca)?

Looking at SecurityOriginData, it seems it is a tuple (protocol, host, port)
As illustrated by the modified test, it is important to keep information such as m_universalAccess which AIUI SecurityOriginData is not aware.
Comment 6 youenn fablet 2016-09-21 07:36:11 PDT
Created attachment 289451 [details]
Rebasing after bug 162260
Comment 7 WebKit Commit Bot 2016-09-22 01:59:27 PDT
Comment on attachment 289451 [details]
Rebasing after bug 162260

Clearing flags on attachment: 289451

Committed r206255: <http://trac.webkit.org/changeset/206255>
Comment 8 WebKit Commit Bot 2016-09-22 01:59:32 PDT
All reviewed patches have been landed.  Closing bug.