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

Issue 608372 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 576261



Sign in to add a comment

PlzNavigate: support POST navigations

Project Member Reported by clamy@chromium.org, May 2 2016

Issue description

PlzNavigate should support POST navigations properly. This includes creating PageState with an appropriate body, and properly recording post ids.
 

Comment 1 by creis@chromium.org, May 16 2016

Cc: creis@chromium.org
For reference, here's @clamy's design doc, which describes the planned changes nicely:
https://docs.google.com/a/chromium.org/document/d/1BtrZjP-lbeza2OdRwr-68poSgGPE6YAfUftP5s1ugY8/edit?usp=sharing
Project Member

Comment 2 by bugdroid1@chromium.org, May 20 2016

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

commit 34754b494e8f9706aa48463dcd1f17026ad658a2
Author: clamy <clamy@chromium.org>
Date: Fri May 20 19:12:58 2016

PlzNavigate: store POST data in the FrameNavigationEntry

This allows to properly resend it during hitsory navigations when
browser-side-navigation is enabled.

A rational for the design can be found in the following doc: https://docs.google.com/a/chromium.org/document/d/1BtrZjP-lbeza2OdRwr-68poSgGPE6YAfUftP5s1ugY8/edit?usp=sharing

BUG= 575210 ,  608372 
TBR=nasko@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/browser/frame_host/frame_navigation_entry.cc
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/browser/frame_host/frame_navigation_entry.h
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/browser/frame_host/navigation_entry_impl.h
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/browser/frame_host/navigation_request.h
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/common/frame_messages.h
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/common/page_state_serialization.cc
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/common/page_state_serialization.h
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/common/resource_request_body.cc
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/common/resource_request_body.h
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/renderer/render_frame_impl.h
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/content/test/test_render_frame.cc
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter
[modify] https://crrev.com/34754b494e8f9706aa48463dcd1f17026ad658a2/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation

Comment 3 by clamy@chromium.org, Dec 6 2016

Labels: Proj-PlzNavigate-Blocking

Comment 4 by nasko@chromium.org, Dec 8 2016

There were a bunch of changes done to POST requests by lukasza@ since this bug was opened. Is there more work that is needed? What specifically is not working, given that content_browsertests run mostly clean with PlzNavigate?

Comment 5 by clamy@chromium.org, Dec 8 2016

We have a bunch of layout tests related to forms failing. I'd like to investigate them before closing this.
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 10 2017

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

commit 14d8238284dab1d8aaad166d5f1ba03cceaf793b
Author: clamy <clamy@chromium.org>
Date: Tue Jan 10 12:17:28 2017

PlzNavigate: don't send unexpected form urls to the browser

Layout tests sometimes attempt to load urls of the form about:blank?foo,
which are sent to the browser since they are not about:blank. However
the browser does not expect these urls, and they are converted to
about:blank when sanitizing the params received in the BeginNavigation
IPC. Don't send these urls to the browser when layout tests are active.

BUG= 608372 

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

[modify] https://crrev.com/14d8238284dab1d8aaad166d5f1ba03cceaf793b/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/14d8238284dab1d8aaad166d5f1ba03cceaf793b/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 19 2017

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

commit 4b3e2a7606da6f4d23fe6e83ece19aceaced85c5
Author: clamy <clamy@chromium.org>
Date: Thu Jan 19 12:14:07 2017

PlzNavigate: ensure POST submissions create navigation entries

This CL fixes an issue in PlzNavigate where a POST navigation that
resulted in a navigation to the current URL was wrongly categorized by
Blink as a reload and would not create a NavigationEntry.

BUG= 608372 

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

[modify] https://crrev.com/4b3e2a7606da6f4d23fe6e83ece19aceaced85c5/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
[modify] https://crrev.com/4b3e2a7606da6f4d23fe6e83ece19aceaced85c5/third_party/WebKit/Source/core/loader/FrameLoader.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 25 2017

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

commit 62c1bacb7df25e17142f3fd0a56c9a8965cde17d
Author: clamy <clamy@chromium.org>
Date: Wed Jan 25 13:30:59 2017

PlzNavigate: ensure POST navigations are treated as different document

This CL fixes an issue where the FrameLoader in Blink would attempt to
commit a POST navigation as same-site even though a different document
navigation should (and has started to) be performed.

BUG= 608372 

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

[modify] https://crrev.com/62c1bacb7df25e17142f3fd0a56c9a8965cde17d/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
[modify] https://crrev.com/62c1bacb7df25e17142f3fd0a56c9a8965cde17d/third_party/WebKit/Source/core/loader/FrameLoader.cpp

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 16 2017

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

commit 74feab800a38e1cc133c3406d19f966844881e42
Author: clamy <clamy@chromium.org>
Date: Thu Feb 16 17:47:47 2017

PlzNavigate: set the right cache policy on back POST navigations

We would repviously set the cache policy to net::LOAD_VALIDATE_CACHE in
that case. It should be net::LOAD_ONLY_FROM_CACHE |
net::LOAD_SKIP_CACHE_VALIDATION.

BUG= 608372 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/74feab800a38e1cc133c3406d19f966844881e42/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/74feab800a38e1cc133c3406d19f966844881e42/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation

Comment 10 by clamy@chromium.org, Apr 20 2017

Status: Fixed (was: Assigned)

Sign in to add a comment