New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 636297 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Leaves the project on 2018/03/02
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Change WebURLLoaderClient::willFollowRedirect() to return a bool

Project Member Reported by tyoshino@chromium.org, Aug 10 2016

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Oct 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c42eabb6f25a2349f6c4d579f2ab4be55502302b

commit c42eabb6f25a2349f6c4d579f2ab4be55502302b
Author: tyoshino <tyoshino@chromium.org>
Date: Wed Oct 05 07:45:54 2016

Change WebURLLoaderClient::willFollowRedirect() API to return bool

WebURLLoaderImpl has been requiring clients to clear the newRequest
argument when they don't want the redirect to be followed. This is
not intuitive and required strange-looking code snippet to reset the
argument.

The method should just return a bool to indicate whether or not to
follow the redirect.

Changed ResourceLoader::willFollowRedirect() to return false when
m_fetcher->willFollowRedirect() returns false. As didFail() clears
m_loader, it should be safe and it's more natural that the case
returns false.

ResourceFetcher::willFollowRedirect() may change the URL of newRequest
and return true because FrameFetchContext::dispatchWillSendRequest()
calls prepareRequest() which calls
FrameLoaderClient::dispatchWillSendRequest() which calls
WebFrameClient::willSendRequest() whose implementations may change the
URL. RenderFrameImpl::willSendRequest() and
WebFrameTestClient::willSendRequest() are the implementations. This CL
leaves the interface unchanged though it should also be fixed.

R=japhet@chromium.org,mkwst@chromium.org,bbudge@chromium.org,jochen@chromium.org,dalecurtis@chromium.org
BUG= 636297 

Review-Url: https://codereview.chromium.org/2230173002
Cr-Commit-Position: refs/heads/master@{#423102}

[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/content/child/web_url_loader_impl.cc
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/content/child/web_url_loader_impl_unittest.cc
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/content/renderer/media/android/media_info_loader.cc
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/content/renderer/media/android/media_info_loader.h
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/content/renderer/pepper/pepper_url_loader_host.cc
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/content/renderer/pepper/pepper_url_loader_host.h
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/media/blink/resource_multibuffer_data_provider.cc
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/media/blink/resource_multibuffer_data_provider.h
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/fetch/RawResource.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/fetch/RawResource.h
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/fetch/RawResourceTest.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/fetch/Resource.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/fetch/Resource.h
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/fetch/ResourceFetcherTest.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/fetch/ResourceLoader.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/fetch/ResourceLoader.h
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/loader/DocumentLoader.h
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/loader/DocumentThreadableLoaderClient.h
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/loader/PingLoader.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/loader/TextTrackLoader.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/core/loader/TextTrackLoader.h
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/platform/testing/weburl_loader_mock.cc
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/web/AssociatedURLLoader.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/Source/web/AssociatedURLLoaderTest.cpp
[modify] https://crrev.com/c42eabb6f25a2349f6c4d579f2ab4be55502302b/third_party/WebKit/public/platform/WebURLLoaderClient.h

Status: Fixed (was: Started)
Created a new  bug 653047  for stopFetching() removal.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fbff05cfbc1af00ed8391f87ad49d23f632eddd6

commit fbff05cfbc1af00ed8391f87ad49d23f632eddd6
Author: tyoshino <tyoshino@chromium.org>
Date: Mon Oct 24 11:50:01 2016

Remove unnecessary setAllowStoredCredentials() call in PingLoader::willFollowRedirect()

This doesn't have any effect on sendBeacon's credentials handling.

This code was introduced in https://codereview.chromium.org/1016373002,
but even since at this point, AllowStoredCredentials is passed to the
BeaconLoader constructor.

Replacing with DCHECK.

R=mkwst@chromium.org
BUG= 636297 

Review-Url: https://codereview.chromium.org/2336273003
Cr-Commit-Position: refs/heads/master@{#427048}

[add] https://crrev.com/fbff05cfbc1af00ed8391f87ad49d23f632eddd6/third_party/WebKit/LayoutTests/http/tests/navigation/beacon-cross-origin-redirect-credentials.html
[modify] https://crrev.com/fbff05cfbc1af00ed8391f87ad49d23f632eddd6/third_party/WebKit/LayoutTests/http/tests/navigation/resources/save-beacon.php
[modify] https://crrev.com/fbff05cfbc1af00ed8391f87ad49d23f632eddd6/third_party/WebKit/Source/core/loader/PingLoader.cpp

Sign in to add a comment