Bisect Info:
===========
Good build : 54.0.2820.0,  Revision Range - 409955 	
Bad build  : 54.0.2821.0,  Revision Range - 410228	

After executing the per-revision-bisect script, i got the following CL's between good and bad build versions
===========================================
https://chromium.googlesource.com/chromium/src/+log/10d41a7cb5416afdb434e98c3b411284c0ab0be8..37e24c4337dd6c5f8e41768d5d792edb52e52d60

Unable to find the culprit CL which caused this issue, requesting dev team to look into it and assign it to approproate owner.

Thank You...

I believe this is caused by change; "Enable subframe FrameNavigationEntries by default." but I'd argue we can actually close this because it is better for the user and really isn't a regression.

The situation is better for the user; if one tab crashes bring down multiple tabs isn't good.

Charlie can you confirm this is an intended side effect and if so close this issue as WontFix?

This isn't itself a bug-- there's already some cases that view source ends up in the same process and some that it ends up in a different process.  There's no real requirement either way, and arguably having it in a separate process is better (as dtapuska@ mentions in comment 2).

That said, there's something confusing going on here, and I'd like to understand it.  Normally, there's two cases:
1) Choosing View Source from a menu or using Ctrl+U will typically open in a cloned tab and will share the same process.
2) Navigating to view-source:[some URL] in a given tab would force a process swap, since View Source requires a new RenderFrameHost in a different mode.  Note: Because we can't have multiple SiteInstances with the same site URL in the same BrowsingInstance (and because the view-source: prefix is not included in the site URL), we have to do a BrowsingInstance swap to get the process swap to happen.

Let's look at case (1).  Using Ctrl+U on chromium.org stays in process, but this bug points out that using Ctrl+U on taobao.com uses a different process.  The difference appears to be related to the redirect (e.g., from taobao.com to world.taobao.com).

I'll look closer to find out why there's a difference.  Ideally, we'd get all of these cases to behave the same, presumably by always creating a different process.

Wow, this is subtle.  Looks like it's a combination of whether the page has iframes and whether there was a redirect causing the SiteInstance's site URL to not match the last committed URL.

iframes matter because SetPageState behavior differs based on whether there are any children.  If not, it just sets the main frame's PageState directly and returns.  If there are children, it will regenerate the whole frame tree of FrameNavigationEntries by parsing apart the new PageState.  Regenerating the frame tree of FNEs causes us to forget the SiteInstance.  (That's unfortunate, but it doesn't matter in the only other call to SetPageState, which is during session restore when we have no SiteInstance at all.)

Redirects matter because the cloned WebContents has a main frame RenderFrameHost with the same SiteInstance as the original tab.  Even if the page has iframes and thus we forget the SiteInstance in the cloned NavigationEntry, the WebContents will continue to use the RFH's SiteInstance if the site URL matches.  However, URLs that redirect from HTTP to HTTPS (or to a different site entirely) will leave us with a stale site URL.  The cloned WebContents will then do a cross-process navigation when told to go to the end URL.

For example:
http://csreis.github.io/tests/cross-site-iframe.html
This page has iframes but no redirects.  We clear the SiteInstance from the NavigationEntry but use the SiteInstance from the cloned RenderFrameHost, and thus stay in process.

http://world.taobao.com, which redirects to https://world.taobao.com
This page has iframes and redirects from HTTP to HTTPS, so it both loses the SiteInstance from the NavigationEntry and thinks it needs a cross-process navigation from the RFH's SiteInstance.  Thus, we end up in a new process.

https://gmail.com, which redirects to https://www.google.com/gmail/about/
That's the same example, but the domain changes instead of HTTP to HTTPS.  New process.

---

I'd say there's very little impact to users here, but this code really needs some cleanup.  We should avoid having different behavior based on whether there's iframes or redirects (for simplicity), and we should always create a new process (to be consistent with the navigation to view-source: case).  Triaging accordingly.

It might also make sense to always pass a single frame's PageState into ViewSource, rather than a single frame's PageState for the subframe case and a full page's PageState for the main frame case.

jshanbal@,
Unable to reproduce the issue on latest Canary-59.0.3068.1 .Could you please check & update the thread accordingly.
Thank you.

Issue 750485 points out another reason to consider changing the behavior of cloned tabs (from case 1 in comment 3), since middle clicking the back button stays in the process it was in before.  That's become more of an outlier since we fixed  issue 23815 , at which point most middle clicks go to a new process.  If we change this, we could more easily converge on having view source always go to a new process.

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

Commit e1b954d9... (from #c7 above) initially landed in the 64.0.3254.0 Canary.
In this version I've tried Ctrl-U on http://taobao.com/ and on https://www.chromium.org/ and in both cases the view-source tab ended up in a separate process.