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

Issue 660567 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

PlzNavigate: RedirectTest.ClientServerServer

Project Member Reported by ananta@chromium.org, Oct 28 2016

Issue description

The above browser_test fails with PlzNavigate on all platforms.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 2 2016

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

commit d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90
Author: ananta <ananta@chromium.org>
Date: Wed Nov 02 04:21:17 2016

Fix for the RedirectTest.ClientServerServer test with browser side navigation.

japhet: Please review blink changes.
jam/nasko: Please review everything

This test does the following:
Navigates to a meta tag refreshed page which is refreshed to server 1 via a HTTP
redirect followed by another redirect to server2 and then the final URL.

This works correctly in the regular case as individual redirect messages
are passed back to the renderer (blink) where the list is correctly maintained
and is passed back in the FrameHostMsg_DidCommitProvisionalLoad message with the
url of the referrer which initiated the client side redirect at the head of the list.

With browser side navigation we do pass the server redirect information back to the renderer
in the FrameMsg_CommitNavigation IPC message. When the navigation is processed the url
of the referrer which initated the client side redirect is added after the server side
redirect urls as blink just appends the document url (original referrer) to the redirect
list followed by the destination url. Basically the assumption is that blink is always
informed about server side redirects which is not the case with browser side navigation.

Changes in this patch are as below:
1. Add the document URL for client side redirects to the redirect list in the DocumentLoader ctor. The other changes
are to ensure that the client redirection flag makes it to the DocumentLoader ctor

   This ensures that the redirect list is in the correct order.

BUG= 660567 

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

[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/core/loader/DocumentLoader.h
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/core/loader/EmptyClients.cpp
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/core/loader/FrameLoaderClient.h
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/modules/webaudio/BaseAudioContextTest.cpp
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/web/FrameLoaderClientImpl.h
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/web/WebDataSourceImpl.cpp
[modify] https://crrev.com/d9062cc1ce0d2fda908c0f8d4e959c12d60d3d90/third_party/WebKit/Source/web/WebDataSourceImpl.h

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 8 2016

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

commit 7e07e49356ddd1dad690965d1f067e37d4647792
Author: ananta <ananta@chromium.org>
Date: Tue Nov 08 02:35:37 2016

PlzNavigate: Enable the RedirectTest.ClientServerServer test.

Enable this test as the issues leading to the test failure have been fixed.

BUG= 660567 
TBR=jam

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

[modify] https://crrev.com/7e07e49356ddd1dad690965d1f067e37d4647792/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter

Status: Fixed (was: Assigned)

Sign in to add a comment