As the first step, I think that we can stop unnecessarily using frame's PageState - it should be sufficient to just conjure a fresh state via content::PageState::CreateFromURL.

This step will be a desirable cleanup in itself, but will also make it easier to write a browsertest for the view-frame-source scenario (without the clean-up it is difficult to get the PageState for the right frame).

FWIW, I have a WIP CL doing this @ https://chromium-review.googlesource.com/703738


After this we can see how to fix the bug.  One way that might work is to call entry_[i]->set_site_instance(nullptr) for all entries copied by NavigationControllerImpl::InsertEntriesFrom.  We still need to check 1) if other things should be also cleared and 2) if the clearing might be undesirable for some direct or indirect callers of NavigationControllerImpl::InsertEntriesFrom.

It seems that the fix in NavigationControllerImpl::InsertEntriesFrom would also affect the action of "Duplicating" a tab.  And it would probably also help with issue 750485 (middle-clicking back button).

Oops, guess there was some collision here. I've been poking at this in https://chromium-review.googlesource.com/c/chromium/src/+/695913. The problem has been with view-source on post data.

Unfortunately, going through OpenURLParams isn't good enough to smuggle in the parameters we need. My next attempt was going to be forging a NavigationEntry directly, with the post data and post ID cloned from the original FNE. If you'd like to take over that patch, I think it might be a good starting point for fixing this (and some other bugs as well).

I am working on adopting Daniel's https://chromium-review.googlesource.com/c/chromium/src/+/695913 and making it ready to land, but I still have a few things to do before I can push it out for the next round of reviews.  I should have it ready early Monday.


It seems to me that  https://crbug.com/523  has regressed before we started to look into this area. 
 When I look at sources of a document retrieved by POST, then the same document is retrieved via GET (tested locally by running embedded_test_server and going through form_that_posts_to_echoall.html;  tested with 63.0.3223.8 (Official Build) dev (64-bit)).  I've tested both "View page source" and "View frame source".

OTOH, maybe this doesn't matter that much - after the CL POST works fine (and I am also adding a regression test for this scenario).

The issue Daniel was seeing on http://www.dobrev.com/games/Post-Demo.html when trying the OpenURL approach was related to not propagating Referer header.  I think I've fixed this issue in patchset 10 in https://chromium-review.googlesource.com/#/c/chromium/src/+/695913

Unfortunately, I found another problem - going through OpenURL hits the network and previous behavior was to load from the cache.  I've added test verification for this and I am trying to figure out a fix - I think that I'll try to replicate the old behavior (e.g. stamping PageState), rather than going through OpenURL (obviously stamping PageState of the *right* frame).

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

commit e1b954d9a6cf66a123ddf719860aa4e5b5cd2888
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Mon Oct 30 21:28:06 2017

Move view-source handling to the //content layer.

This CL moves most of handling of view-source into the //content layer,
exposing it through a single public API:
  void RenderFrameHost::ViewSource()

This move helps to
- Avoid passing PageState to the //chrome layer (e.g. via
  content::CustomContextMenuContext).
- Create browser tests that can simulate triggerring a view-source
  from the code (e.g. without having to simulate mouse clicks).
- Ensure that the right navigation entries are used - preventing
  incorrect reusing of main frame's site instance for showing
  a subframe's view-source ( https://crbug.com/770946 )

This CL adds regression tests for  https://crbug.com/523  that ensure that
view-source for HTTP POST works fine in case of main frame and subframe.
Works fine = no new network requests are issued (verified by a response
nonce added to the /echoall default handler in the embedded http test
server).

Bug: 770487,  770946 ,  774691 ,  699493 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ic0afeed898b4f0900f3f8ad47a903f83f2c589d3
Reviewed-on: https://chromium-review.googlesource.com/695913
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512625}
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/chrome_navigation_browsertest.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/tab_contents/view_source_browsertest.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/ui/browser.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/ui/browser.h
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/ui/browser_command_controller.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/ui/browser_commands.h
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/browser/ui/views/page_info/page_info_bubble_view_browsertest.cc
[add] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/chrome/test/data/form_that_posts_to_echoall.html
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/browser/frame_host/render_frame_host_delegate.h
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/common/frame_messages.h
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/public/browser/render_frame_host.h
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/public/browser/web_contents.h
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/public/browser/web_contents_delegate.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/public/browser/web_contents_delegate.h
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/public/common/context_menu_params.h
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/content/renderer/context_menu_params_builder.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/net/test/embedded_test_server/default_handlers.cc
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/third_party/WebKit/Source/core/page/ContextMenuClient.cpp
[modify] https://crrev.com/e1b954d9a6cf66a123ddf719860aa4e5b5cd2888/third_party/WebKit/public/web/WebContextMenuData.h