Bug 160404 - [Fetch API] Add support to ReferrerPolicy
Summary: [Fetch API] Add support to ReferrerPolicy
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: 151937
  Show dependency treegraph
 
Reported: 2016-08-01 07:48 PDT by youenn fablet
Modified: 2017-03-24 11:04 PDT (History)
3 users (show)

See Also:


Attachments
Patch (51.00 KB, patch)
2016-08-01 08:13 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (51.11 KB, patch)
2016-08-01 23:51 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (163.44 KB, patch)
2016-08-02 00:25 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2016-08-01 07:48:20 PDT
[Fetch API] Add support to ReferrerPolicy
Comment 1 youenn fablet 2016-08-01 08:13:42 PDT
Created attachment 285012 [details]
Patch
Comment 2 WebKit Commit Bot 2016-08-01 08:15:31 PDT
Attachment 285012 [details] did not pass style-queue:


ERROR: Source/WebCore/loader/ThreadableLoaderClientWrapper.h:98:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alex Christensen 2016-08-01 19:18:18 PDT
Comment on attachment 285012 [details]
Patch

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

> Source/WebCore/Modules/fetch/FetchRequest.cpp:298
> +    if (m_internalRequest.referrer != "no-referrer" && m_internalRequest.referrer != "client")

It seems like a case-sensitive check is correct in this case.  Could you add a test that tests No-Referrer and Client?
Could we use enum values instead of doing several string comparisons here?

> Source/WebCore/workers/WorkerScriptLoader.cpp:84
>      ASSERT(client);
> +    ASSERT(scriptExecutionContext);

We should come back later and make all these references instead of pointers at the callers.
Comment 4 youenn fablet 2016-08-01 23:49:19 PDT
Thanks for the review.

(In reply to comment #3)
> Comment on attachment 285012 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=285012&action=review
> 
> > Source/WebCore/Modules/fetch/FetchRequest.cpp:298
> > +    if (m_internalRequest.referrer != "no-referrer" && m_internalRequest.referrer != "client")
> 
> It seems like a case-sensitive check is correct in this case.  Could you add
> a test that tests No-Referrer and Client?
> Could we use enum values instead of doing several string comparisons here?

I will add a FIXME.
I do not like the current handling of referrer.
Although convenient, it is odd to pass it in the headers as it only serves to compute the actual value in CachedResource.
I would much prefer to pass it as a field in a structure like CachedResourceRequest for instance. The same applies to Origin.


> > Source/WebCore/workers/WorkerScriptLoader.cpp:84
> >      ASSERT(client);
> > +    ASSERT(scriptExecutionContext);
> 
> We should come back later and make all these references instead of pointers
> at the callers.

Right.
Comment 5 youenn fablet 2016-08-01 23:51:45 PDT
Created attachment 285078 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2016-08-01 23:54:34 PDT
Attachment 285078 [details] did not pass style-queue:


ERROR: Source/WebCore/loader/ThreadableLoaderClientWrapper.h:98:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 WebKit Commit Bot 2016-08-02 00:21:42 PDT
Comment on attachment 285078 [details]
Patch for landing

Clearing flags on attachment: 285078

Committed r204019: <http://trac.webkit.org/changeset/204019>
Comment 8 WebKit Commit Bot 2016-08-02 00:21:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 youenn fablet 2016-08-02 00:25:34 PDT
Reopening to attach new patch.
Comment 10 youenn fablet 2016-08-02 00:25:41 PDT
Created attachment 285080 [details]
Patch for landing