Swaps renderer processes on links with rel=noreferrer and target=_blank....

Swaps renderer processes on links with rel=noreferrer and target=_blank.

We now detect these types of navigations in RenderView and allow the browser process to handle them.  This allows us to swap process if the navigation is cross-site.  Requires exposing suppressOpenerInNewFrame in WebFrame to accurately detect these links.

BUG=24447
TEST=RenderViewHostManagerTest.SwapProcessOnRelNoreferrerWithTargetBlank

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=29783

[Charlie Reis, 2009-10-16 19:56:16]

Here's a first cut.  The test will fail until Nate's change lands.

The test case is a bit long, but I thought it would be more efficient than three
tests each starting their own HTTP servers.

[darin (slow to review), 2009-10-19 19:40:28]

no need for GYP changes when adding a .html test file.

http://codereview.chromium.org/284015/diff/1/4
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/284015/diff/1/4#newcode1885
Line 1885: GURL(frame->url()) == GURL() &&
will the frame's url really be empty?  i would have still expected about:blank.

[Charlie Reis, 2009-10-19 19:55:47]

http://codereview.chromium.org/284015/diff/1/4
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/284015/diff/1/4#newcode1885
Line 1885: GURL(frame->url()) == GURL() &&
On 2009/10/19 19:40:29, darin wrote:
> will the frame's url really be empty?  i would have still expected
about:blank.

That's what I'm seeing on target=_blank links.  I was surprised that it was
different from the Gmail case, as well, since we do see about:blank there.

[Charlie Reis, 2009-10-19 20:51:00]

New changelist uploaded-- I only added an #include in the test to get it to
compile after I sync'd.  (Also alphabetized the includes, since they were out of
order.)

After sync'ing, I verified that the url is empty (i.e., GURL()) in this case,
and not about:blank.

[darin (slow to review), 2009-10-19 21:33:04]

LGTM

http://codereview.chromium.org/284015/diff/6001/5004
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/284015/diff/6001/5004#newcode1923
Line 1923: GURL(frame->url()) == GURL() &&
nit: can you just do frame->url().isEmpty()?

http://codereview.chromium.org/284015/diff/6001/5004#newcode1928
Line 1928: request.httpHeaderField(WebString::fromUTF8("Referer")).isNull() &&
we really should add a httpReferrer() getter on WebURLRequest to help people not
mistype Referer ;-)

(something for another CL)

[Charlie Reis, 2009-10-20 06:43:54]

Needed to add another condition to the heuristic to make sure this RenderView
was created by another Frame.  Also cleaned up some whitespace and fixed the
nits.

Current revision passes the windows try bot.

http://codereview.chromium.org/284015/diff/6001/5004
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/284015/diff/6001/5004#newcode1923
Line 1923: GURL(frame->url()) == GURL() &&
On 2009/10/19 21:33:04, darin wrote:
> nit: can you just do frame->url().isEmpty()?

Sure-- fixed in the lastest changelist.

http://codereview.chromium.org/284015/diff/6001/5004#newcode1928
Line 1928: request.httpHeaderField(WebString::fromUTF8("Referer")).isNull() &&
On 2009/10/19 21:33:04, darin wrote:
> we really should add a httpReferrer() getter on WebURLRequest to help people
not
> mistype Referer ;-)
> 
> (something for another CL)

Good idea-- I added a TODO.

[darin (slow to review), 2009-10-20 07:00:16]

http://codereview.chromium.org/284015/diff/7002/8005
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/284015/diff/7002/8005#newcode1914
Line 1914: bool userg = frame->isProcessingUserGesture();
leftover debugging cruft?

[Charlie Reis, 2009-10-20 15:14:16]

http://codereview.chromium.org/284015/diff/7002/8005
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/284015/diff/7002/8005#newcode1914
Line 1914: bool userg = frame->isProcessingUserGesture();
On 2009/10/20 07:00:16, darin wrote:
> leftover debugging cruft?

D'oh.  Yep-- I've removed this and the line below.

[Charlie Reis, 2009-10-20 18:45:06]

Here's the updated changelist, which depends on Nate's WebKit patch to
FrameLoader.

I left in the previous checks we were doing (in addition to the new more
accurate check).  I could probably edit out a few of them if you want, but many
of them are still important since the flag stays true for the lifetime of the
RenderView.

I haven't sent it past the try bots yet.  I'll wait for the next deps roll.

[darin (slow to review), 2009-10-20 19:32:48]

http://codereview.chromium.org/284015/diff/16001/16006
File webkit/api/public/WebFrame.h (right):

http://codereview.chromium.org/284015/diff/16001/16006#newcode308
Line 308: virtual bool suppressOpenerInNewFrame() const = 0;
this reads like a command or an action.  i think it should read more like an
attribute.  perhaps like these:

suppressesOpenerInNewFrame
willSuppressOpenerInNewFrame

[Charlie Reis, 2009-10-20 20:21:00]

http://codereview.chromium.org/284015/diff/16001/16006
File webkit/api/public/WebFrame.h (right):

http://codereview.chromium.org/284015/diff/16001/16006#newcode308
Line 308: virtual bool suppressOpenerInNewFrame() const = 0;
On 2009/10/20 19:32:48, darin wrote:
> this reads like a command or an action.  i think it should read more like an
> attribute.  perhaps like these:
> 
> suppressesOpenerInNewFrame
> willSuppressOpenerInNewFrame

I renamed it to isSuppressingOpenerInNewFrame, since it's a temporary state on
the frame that's creating this frame.

[Charlie Reis, 2009-10-20 21:18:42]

Renamed to willSuppressOpenerInNewFrame.

[Charlie Reis, 2009-10-21 18:54:50]

Is this good to go?

Thanks,
Charlie

[darin (slow to review), 2009-10-22 07:00:42]

LGTM

http://codereview.chromium.org/284015/diff/6002/5009
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/284015/diff/6002/5009#newcode1323
Line 1323: view->opener_suppressed_ = creator->willSuppressOpenerInNewFrame();
nit: I recommend capturing this value to a local variable up where we call
isProcessingUserGesture.  the code between there and here could have side
effects, so it'd probably be better to avoid potential problems (however
unlikely they may be).

[Charlie Reis, 2009-10-22 16:58:24]

http://codereview.chromium.org/284015/diff/6002/5009
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/284015/diff/6002/5009#newcode1323
Line 1323: view->opener_suppressed_ = creator->willSuppressOpenerInNewFrame();
On 2009/10/22 07:00:42, darin wrote:
> nit: I recommend capturing this value to a local variable up where we call
> isProcessingUserGesture.  the code between there and here could have side
> effects, so it'd probably be better to avoid potential problems (however
> unlikely they may be).

Fixed.  Thanks-- I'll get it landed!