Bug 162258 - CachedResourceRequest should store a SecurityOrigin
Summary: CachedResourceRequest should store a SecurityOrigin
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks: 162151
  Show dependency tree / graph
 
Reported: 2016-09-19 23:28 PDT by youenn fablet
Modified: 2016-09-22 01:59 PDT (History)
6 users (show)

See Also:

Attachments
Patch (16.48 KB, patch)
2016-09-20 00:27 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (16.51 KB, patch)
2016-09-20 01:10 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (16.49 KB, patch)
2016-09-20 01:56 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Rebasing after bug 162260 (15.07 KB, patch)
2016-09-21 07:36 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.