Create window in a new BrowsingInstance when opening a link in a new process.

Create window in a new BrowsingInstance when opening a link in a new process.
This allows the rel=noreferrer process trick to work even for same-site links.

BUG=69267
TEST=Same-site link to rel=noreferrer target=_blank link gets new process.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=125180

[Charlie Reis, 2012-02-06 20:38:54]

Hi Darin--
  This is still a draft, but I'm wondering if you think this approach is
plausible.  I'm not sure if we'll have quite enough information in
CreateNewWindow to kick off the navigation from there, but it looks pretty
close.

Thanks,
Charlie

[Charlie Reis, 2012-02-07 02:19:54]

Here's an updated draft that uses the correct WindowDisposition when opening the
new window.  Unfortunately, it requires a small change to the Chromium WebKit
port.

https://chromiumcodereview.appspot.com/9325082/diff/2001/content/renderer/ren...
File content/renderer/render_view_impl.cc (right):

https://chromiumcodereview.appspot.com/9325082/diff/2001/content/renderer/ren...
content/renderer/render_view_impl.cc:1391: new_window_navigation_policy_ =
policy;
I'm not thrilled about this, but I haven't found a better way to figure out the
correct disposition while we're in RenderViewImpl::createView.

This must be called from
FrameLoaderClientImpl::dispatchDecidePolicyForNewWindowAction, which is the same
place we're storing m_nextNavigationPolicy for later use.

[Charlie Reis, 2012-02-13 21:25:05]

Hey Darin-- now that you're back, would you have time to take a look at this? 
I'm curious if you think this is a reasonable approach.

Thanks,
Charlie

[darin (slow to review), 2012-02-14 00:15:56]

Is there a corresponding WebKit patch I can read?  I'm not sure I get
didSetNextNavigationPolicy.

https://chromiumcodereview.appspot.com/9325082/diff/2001/content/renderer/ren...
File content/renderer/render_view_impl.cc (right):

https://chromiumcodereview.appspot.com/9325082/diff/2001/content/renderer/ren...
content/renderer/render_view_impl.cc:1388: // TODO(creis): Should this be on
WebViewClient or WebFrameClient?
navigation related callbacks typically live on WebFrameClient

[Charlie Reis, 2012-02-14 02:18:58]

On 2012/02/14 00:15:56, darin wrote:
> Is there a corresponding WebKit patch I can read?  I'm not sure I get
> didSetNextNavigationPolicy.

I haven't put together a patch yet, but it's a one-liner in
FrameLoaderClientImpl::dispatchDecidePolicyForNewWindowAction.  Just add the
following line below the assignment to m_nextNavigationPolicy:
m_webFrame->client()->didSetNextnavigationPolicy(m_nextNavigationPolicy);

Here's a link to where it happens:
http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/src/FrameLoaderCl...

[Charlie Reis, 2012-03-01 23:31:50]

After our discussions, I've changed this to expect WebKit to pass the
WebNavigationPolicy into RenderViewImpl::createView.  That patch is here:
https://bugs.webkit.org/show_bug.cgi?id=80057

Note that the WebKit patch makes changes to the WebViewClient API by adding
another argument to createView.  Assuming these two CLs can't land at the same
time, perhaps I could land an initial Chrome CL that overloads
RenderViewImpl::createView?  That would avoid compile failures until this CL
lands.

[darin (slow to review), 2012-03-02 23:00:45]

Landing a chromium side patch in advance like that is a common technique.

On Thu, Mar 1, 2012 at 3:31 PM, <creis@chromium.org> wrote:

> After our discussions, I've changed this to expect WebKit to pass the
> WebNavigationPolicy into RenderViewImpl::createView.  That patch is here:
>
https://bugs.webkit.org/show_**bug.cgi?id=80057<https://bugs.webkit.org/show_...
>
> Note that the WebKit patch makes changes to the WebViewClient API by adding
> another argument to createView.  Assuming these two CLs can't land at the
> same
> time, perhaps I could land an initial Chrome CL that overloads
> RenderViewImpl::createView?  That would avoid compile failures until this
> CL
> lands.
>
>
https://chromiumcodereview.**appspot.com/9325082/<https://chromiumcodereview....
>

[Charlie Reis, 2012-03-02 23:59:12]

On 2012/03/02 23:00:45, darin wrote:
> Landing a chromium side patch in advance like that is a common technique.

Great.  I've posted the initial CL here:
https://chromiumcodereview.appspot.com/9583026/

This CL now has a test and is ready for review.  This approach currently puts
same-site rel=noreferrer links in a new process (as long as they're in a new
window).  I've verified that the same logic will work for any other cases that
opener_suppressed is true, so we could use it for things like rel=noscript and
window.open(url, "", "noscript") in the future.

Thanks!
Charlie

[Charlie Reis, 2012-03-06 17:52:20]

Hi Darin--
  The other patches are in place and the try jobs look happy.  Can you review
today?

Thanks,
Charlie

[darin (slow to review), 2012-03-06 17:54:34]

LGTM

https://chromiumcodereview.appspot.com/9325082/diff/16001/content/renderer/re...
File content/renderer/render_view_impl.cc (right):

https://chromiumcodereview.appspot.com/9325082/diff/16001/content/renderer/re...
content/renderer/render_view_impl.cc:1453: params.disposition =
looks like you don't need a line break here