Attachment 338758 Details for Bug 184890 – Patch for landing

[patch] Patch for landing

bug-184890-20180425110024.patch (text/plain), 12.59 KB, created by youenn fablet on 2018-04-25 11:00:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-04-25 11:00:24 PDT

Size: 12.59 KB

patch

obsolete

>Subversion Revision: 230995
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 0b633e246e02d1e02da4206ef2590138ceb53ebd..74daa947c87cf2496ddcaeeb96ccc7338aafab03 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-04-25  Youenn Fablet  <youenn@apple.com>
>+
>+        Ensure DNT is set for redirections handled in NetworkProcess
>+        https://bugs.webkit.org/show_bug.cgi?id=184890
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        Compute whether DNT header should be set on requests based on:
>+        - request has a DNT header
>+        - session is ephemeral (aka private browsing mode)
>+        In both cases, we ensure a DNT header is added for any request triggered by a redirection.
>+
>+        Covered by http/wpt/fetch/dnt-header-after-redirection.html.
>+
>+        * NetworkProcess/NetworkLoadChecker.cpp:
>+        (WebKit::NetworkLoadChecker::check):
>+        (WebKit::NetworkLoadChecker::prepareRedirectedRequest):
>+        * NetworkProcess/NetworkLoadChecker.h:
>+        * NetworkProcess/NetworkResourceLoader.cpp:
>+        (WebKit::NetworkResourceLoader::willSendRedirectedRequest):
>+        (WebKit::NetworkResourceLoader::continueWillSendRequest):
>+        * NetworkProcess/PingLoad.cpp:
>+        (WebKit::PingLoad::willPerformHTTPRedirection):
>+        * WebProcess/Network/WebLoaderStrategy.cpp:
>+        (WebKit::WebLoaderStrategy::scheduleLoadFromNetworkProcess):
>+
> 2018-04-25  Dean Jackson  <dino@apple.com>
> 
>         Make a better flag for system preview, and disable it where necessary
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>index e6e14da4e138f03a2f2a1f2a2349393ba1f000ca..50efd7aabd6d6bf755c1ed0f15f8d04d9cec9982 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>@@ -34,6 +34,7 @@
> #include <WebCore/ContentSecurityPolicy.h>
> #include <WebCore/CrossOriginAccessControl.h>
> #include <WebCore/CrossOriginPreflightResultCache.h>
>+#include <WebCore/HTTPParsers.h>
> 
> #define RELEASE_LOG_IF_ALLOWED(fmt, ...) RELEASE_LOG_IF(m_sessionID.isAlwaysOnLoggingAllowed(), Network, "%p - NetworkLoadChecker::" fmt, this, ##__VA_ARGS__)
> 
>@@ -71,9 +72,21 @@ void NetworkLoadChecker::check(ResourceRequest&& request, ValidationHandler&& ha
>     ASSERT(!isChecking());
> 
>     m_firstRequestHeaders = request.httpHeaderFields();
>+    // FIXME: We should not get this information from the request but directly from some NetworkProcess setting.
>+    m_dntHeaderValue = m_firstRequestHeaders.get(HTTPHeaderName::DNT);
>+    if (m_dntHeaderValue.isNull() && m_sessionID.isEphemeral()) {
>+        m_dntHeaderValue = "1";
>+        request.setHTTPHeaderField(HTTPHeaderName::DNT, m_dntHeaderValue);
>+    }
>     checkRequest(WTFMove(request), WTFMove(handler));
> }
> 
>+void NetworkLoadChecker::prepareRedirectedRequest(ResourceRequest& request)
>+{
>+    if (!m_dntHeaderValue.isNull())
>+        request.setHTTPHeaderField(HTTPHeaderName::DNT, m_dntHeaderValue);
>+}
>+
> void NetworkLoadChecker::checkRedirection(WebCore::ResourceResponse& redirectResponse, ResourceRequest&& request, ValidationHandler&& handler)
> {
>     ASSERT(!isChecking());
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>index f4b907d0e655b567c3a327406d852c1690e25e77..504338cc4512ce1287e5c9874988f83b62c8da04 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
>@@ -52,6 +52,7 @@ public:
>     using ValidationHandler = CompletionHandler<void(RequestOrError&&)>;
>     void check(WebCore::ResourceRequest&&, ValidationHandler&&);
>     void checkRedirection(WebCore::ResourceResponse&, WebCore::ResourceRequest&&, ValidationHandler&&);
>+    void prepareRedirectedRequest(WebCore::ResourceRequest&);
> 
>     WebCore::ResourceError validateResponse(WebCore::ResourceResponse&);
> 
>@@ -109,6 +110,7 @@ private:
>     size_t m_redirectCount { 0 };
>     WebCore::URL m_previousURL;
>     WebCore::PreflightPolicy m_preflightPolicy;
>+    String m_dntHeaderValue;
> };
> 
> }
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>index 28ca19085a3aa24b07b37b2b5179801959d58ecb..8749af7721ab32fd9ca20fe294af80ad449a5e31 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -573,7 +573,10 @@ void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request,
>             if (storedCredentialsPolicy != m_networkLoadChecker->storedCredentialsPolicy()) {
>                 // We need to restart the load to update the session according the new credential policy.
>                 m_networkLoad->cancel();
>-                this->startNetworkLoad(WTFMove(result.value()), FirstLoad::No);
>+                auto request = WTFMove(result.value());
>+                m_networkLoadChecker->prepareRedirectedRequest(request);
>+
>+                this->startNetworkLoad(WTFMove(request), FirstLoad::No);
>                 return;
>             }
> 
>@@ -625,6 +628,9 @@ void NetworkResourceLoader::continueWillSendRequest(ResourceRequest&& newRequest
> {
>     RELEASE_LOG_IF_ALLOWED("continueWillSendRequest: (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ")", m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier);
> 
>+    if (m_networkLoadChecker)
>+        m_networkLoadChecker->prepareRedirectedRequest(newRequest);
>+
>     m_isAllowedToAskUserForCredentials = isAllowedToAskUserForCredentials;
> 
>     // If there is a match in the network cache, we need to reuse the original cache policy and partition.
>diff --git a/Source/WebKit/NetworkProcess/PingLoad.cpp b/Source/WebKit/NetworkProcess/PingLoad.cpp
>index d8ca72a6fe53758dafa28ab5de4109028638c905..4d0951d3306b06fb23322db061aafed1b582703e 100644
>--- a/Source/WebKit/NetworkProcess/PingLoad.cpp
>+++ b/Source/WebKit/NetworkProcess/PingLoad.cpp
>@@ -99,7 +99,10 @@ void PingLoad::willPerformHTTPRedirection(ResourceResponse&& redirectResponse, R
>             this->didFinish(result.error());
>             return;
>         }
>-        completionHandler(WTFMove(result.value()));
>+        auto request = WTFMove(result.value());
>+        m_networkLoadChecker->prepareRedirectedRequest(request);
>+
>+        completionHandler(WTFMove(request));
>     });
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index d1cee80eb07869be8d71a7ace048889634ff4bfe..1e191ab8c90f5576a2b5b9d4de7239c04c9b701d 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-04-25  Youenn Fablet  <youenn@apple.com>
>+
>+        Ensure DNT is set for redirections handled in NetworkProcess
>+        https://bugs.webkit.org/show_bug.cgi?id=184890
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        * http/wpt/fetch/dnt-header-after-redirection-expected.txt: Added.
>+        * http/wpt/fetch/dnt-header-after-redirection.html: Added.
>+        * http/wpt/fetch/resources/dnt-status.py: Added.
>+        * http/wpt/fetch/resources/redirect.py: Added.
>+        * platform/mac-wk1/TestExpectations:
>+        * platform/win/TestExpectations:
>+
> 2018-04-25  Youenn Fablet  <youenn@apple.com>
> 
>         Make DocumentThreadableLoader error logging more consistent
>diff --git a/LayoutTests/http/wpt/fetch/dnt-header-after-redirection-expected.txt b/LayoutTests/http/wpt/fetch/dnt-header-after-redirection-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..1ee6e382b4b5e6bc1ab44284c153cd983e975d52
>--- /dev/null
>+++ b/LayoutTests/http/wpt/fetch/dnt-header-after-redirection-expected.txt
>@@ -0,0 +1,6 @@
>+
>+
>+PASS video load with DNT 
>+PASS Sync XHR with DNT 
>+PASS Beacon with DNT 
>+
>diff --git a/LayoutTests/http/wpt/fetch/dnt-header-after-redirection.html b/LayoutTests/http/wpt/fetch/dnt-header-after-redirection.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..8bfaf5379b5e161bddecd9a2e5b4f2f96798cad3
>--- /dev/null
>+++ b/LayoutTests/http/wpt/fetch/dnt-header-after-redirection.html
>@@ -0,0 +1,66 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<title>DNT header after redirections</title>
>+<script src="/resources/testharness.js"></script>
>+<script src="/resources/testharnessreport.js"></script>
>+<script src="/common/utils.js"></script>
>+</head>
>+<body>
>+    <video id='video'></video>
>+    <script>
>+function waitFor(test, duration)
>+{
>+    return new Promise(resolve => {
>+        test.step_timeout(resolve, duration);
>+    });
>+}
>+
>+async function checkDNTHeader(test, token)
>+{
>+    let maxPollCount = 20;
>+    for (let cptr = 0; cptr < maxPollCount; cptr++) {
>+        await waitFor(test, 10);
>+        const response = await fetch("resources/dnt-status.py?read&token=" + token);
>+        if (await response.text() === "1")
>+            return true;
>+    }
>+    return false;
>+}
>+
>+if (window.testRunner)
>+    testRunner.setPrivateBrowsingEnabled(true);
>+else
>+    alert("Make sure to run this test in Private Browsing Mode");
>+
>+promise_test(async (test) => {
>+    const token = self.token();
>+    const finalURL = "dnt-status.py?store&token=" + token;
>+    video.src = "resources/redirect.py?location=" + encodeURIComponent(finalURL);
>+
>+    assert_true(await checkDNTHeader(test, token), "DNT header is there");
>+}, "video load with DNT");
>+
>+promise_test(async (test) => {
>+    const token = self.token();
>+    const finalURL = "dnt-status.py?store&token=" + token;
>+
>+    const xhr = new XMLHttpRequest();
>+    xhr.open("GET", "resources/redirect.py?location=" + encodeURIComponent(finalURL), false);
>+    xhr.send();
>+
>+    const response = await fetch("resources/dnt-status.py?read&token=" + token);
>+    assert_equals(await response.text(), "1", "DNT header");
>+}, "Sync XHR with DNT");
>+
>+promise_test(async (test) => {
>+    const token = self.token();
>+    const finalURL = "dnt-status.py?store&token=" + token;
>+
>+    navigator.sendBeacon("resources/redirect.py?location=" + encodeURIComponent(finalURL), "");
>+
>+    assert_true(await checkDNTHeader(test, token), "DNT header is there");
>+}, "Beacon with DNT");
>+    </script>
>+</body>
>+</html>
>diff --git a/LayoutTests/http/wpt/fetch/resources/dnt-status.py b/LayoutTests/http/wpt/fetch/resources/dnt-status.py
>new file mode 100644
>index 0000000000000000000000000000000000000000..2979d5ed6d6fa50b262adaf1c66569bf3e5f38d8
>--- /dev/null
>+++ b/LayoutTests/http/wpt/fetch/resources/dnt-status.py
>@@ -0,0 +1,10 @@
>+def main(request, response):
>+    headers = [("Content-Type", "text/plain"),
>+               ("Cache-Control", "no-cache"),
>+               ("Pragma", "no-cache")]
>+
>+    if "store" in request.GET:
>+        request.server.stash.put(request.GET['token'], request.headers.get("DNT", "-1"))
>+        return 200, headers, ""
>+
>+    return 200, headers, str(request.server.stash.take(request.GET['token']))
>diff --git a/LayoutTests/http/wpt/fetch/resources/redirect.py b/LayoutTests/http/wpt/fetch/resources/redirect.py
>new file mode 100644
>index 0000000000000000000000000000000000000000..4006c9d41d9bdcbede2bc23109301a1b73e7be10
>--- /dev/null
>+++ b/LayoutTests/http/wpt/fetch/resources/redirect.py
>@@ -0,0 +1,5 @@
>+def main(request, response):
>+    headers = [("Cache-Control", "no-cache"),
>+               ("Pragma", "no-cache"),
>+               ("Location", request.GET['location'])]
>+    return 302, headers, ""
>diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
>index c0e6da91aa36d005860b8cf2a8cd7d7e99d44280..7a198c002c681117c389f6f95ea164b655b19655 100644
>--- a/LayoutTests/platform/mac-wk1/TestExpectations
>+++ b/LayoutTests/platform/mac-wk1/TestExpectations
>@@ -453,6 +453,9 @@ webkit.org/b/182571 media/modern-media-controls/tracks-support/tracks-support-sh
> 
> webkit.org/b/173432 [ Debug ] imported/w3c/web-platform-tests/fetch/nosniff/importscripts.html [ Pass Crash ]
> 
>+# Requires WK2 loading support
>+http/wpt/fetch/dnt-header-after-redirection.html [ Skip ]
>+
> # requires wk2 speculative tiling
> fast/images/low-memory-decode.html [ Skip ]
> 
>diff --git a/LayoutTests/platform/win/TestExpectations b/LayoutTests/platform/win/TestExpectations
>index d5a9406b01983be3572a5613d6cc10ab177b202e..d65c4fbe6de5bc164da8c699c9e075d2d340ebd8 100644
>--- a/LayoutTests/platform/win/TestExpectations
>+++ b/LayoutTests/platform/win/TestExpectations
>@@ -3712,6 +3712,9 @@ js/dom/builtin-getter-name.html [ Failure ]
> fast/text/variations/font-selection-font-weight.html [ ImageOnlyFailure ]
> fast/forms/file/entries-api/webkitdirectory-open-panel.html [ Skip ]
> 
>+# Requires WK2 loading support
>+http/wpt/fetch/dnt-header-after-redirection.html [ Skip ]
>+
> # html/syntax web platform tests are failing on Windows.
> webkit.org/b/162415 imported/w3c/web-platform-tests/html/syntax [ Skip ]
>

Actions: View | Formatted Diff | Diff

Attachments on bug 184890: 338592 | 338596 | 338598 | 338601 | 338619 | 338634 | 338637 | 338758