Change WebURLLoaderClient::willFollowRedirect() API to return bool

third_party/WebKit/Source/core/fetch/RawResource.cpp

Index: third_party/WebKit/Source/core/fetch/RawResource.cpp
diff --git a/third_party/WebKit/Source/core/fetch/RawResource.cpp b/third_party/WebKit/Source/core/fetch/RawResource.cpp
index 4aec26459642a659fa5082df00c08e86779320d9..0605f7a2bca05e8ef86bdb911e09e2edb11e7fa6 100644
--- a/third_party/WebKit/Source/core/fetch/RawResource.cpp
+++ b/third_party/WebKit/Source/core/fetch/RawResource.cpp
@@ -122,14 +122,22 @@ void RawResource::didAddClient(ResourceClient* c)
     Resource::didAddClient(client);
 }
 
-void RawResource::willFollowRedirect(ResourceRequest& newRequest, const ResourceResponse& redirectResponse)
+bool RawResource::willFollowRedirect(ResourceRequest& newRequest, const ResourceResponse& redirectResponse)
 {
-    Resource::willFollowRedirect(newRequest, redirectResponse);
+    bool follow = Resource::willFollowRedirect(newRequest, redirectResponse);
+    if (!follow)
+        newRequest = ResourceRequest();

[Nate Chapin, 2016/09/06 16:46:14]

Why is blanking the ResourceRequest needed?

[tyoshino (SeeGerritForStatus), 2016/09/07 10:52:28]

Ah, yeah. It's unnecessary. I just tried to make things consistent. Changed to
DCHECK() on the result of Resource::willFollowRedirect().

 
     ASSERT(!redirectResponse.isNull());
     ResourceClientWalker<RawResourceClient> w(clients());
-    while (RawResourceClient* c = w.next())
-        c->redirectReceived(this, newRequest, redirectResponse);
+    while (RawResourceClient* c = w.next()) {
+        if (!c->redirectReceived(this, newRequest, redirectResponse)) {
+            follow = false;
+            newRequest = ResourceRequest();

[tyoshino (SeeGerritForStatus), 2016/09/07 10:52:28]

This is to keep the clients see the same (possibly already nullified) request as
before.

This logic should be fixed later to consult all the clients without changing
newRequest and return false only when none of the clients said "proceed".

[Nate Chapin, 2016/09/07 17:05:45]

How much trouble would it be to change the behavior in this patch?

[tyoshino (SeeGerritForStatus), 2016/09/14 14:53:47]

We need some clean
- one is https://codereview.chromium.org/2336273003/.
- another is ... DocumentThreadableLoader is used by AssociatedURLLoader, and
therefore making redirectReceived()'s newRequest argument const needs making
DocumentThreadableLoaderClient::willFollowRedirect()'s newRequest argument
const, and it needs making WebURLLoaderClient::willFollowRedirect() const.
CrossOriginAccessControl::handleRedirect() still mutates newRequest. So, we need
to change this first.

As I have to these non-trivial work, I preferred to do them later.

[Nate Chapin, 2016/09/14 22:25:27]

OK.

+        }
+    }
+
+    return follow;
 }
 
 void RawResource::willNotFollowRedirect()