RESOLVED FIXED 160404
[Fetch API] Add support to ReferrerPolicy 
https://bugs.webkit.org/show_bug.cgi?id=160404
Summary [Fetch API] Add support to ReferrerPolicy
youenn fablet
Reported 2016-08-01 07:48:20 PDT
[Fetch API] Add support to ReferrerPolicy
Attachments
Patch (51.00 KB, patch)
2016-08-01 08:13 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (51.11 KB, patch)
2016-08-01 23:51 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (163.44 KB, patch)
2016-08-02 00:25 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
youenn fablet
Comment 1 2016-08-01 08:13:42 PDT
Created attachment 285012 [details] Patch
WebKit Commit Bot
Comment 2 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.
Alex Christensen
Comment 3 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.
youenn fablet
Comment 4 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.
youenn fablet
Comment 5 2016-08-01 23:51:45 PDT
Created attachment 285078 [details] Patch for landing
WebKit Commit Bot
Comment 6 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.
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2016-08-02 00:21:47 PDT
All reviewed patches have been landed. Closing bug.
youenn fablet
Comment 9 2016-08-02 00:25:34 PDT
Reopening to attach new patch.
youenn fablet
Comment 10 2016-08-02 00:25:41 PDT
Created attachment 285080 [details] Patch for landing
Note You need to log in before you can comment on or make changes to this bug.