PlzNavigate: use RenderFrameHostImpl::IsRendererTransferNeededForNavigation

PlzNavigate: use RenderFrameHostImpl::IsRendererTransferNeededForNavigation

Following this CL, PlzNavigate will use
RenderFrameHostImpl::IsRendererTransferNeededForNavigation to determine if a
renderer swap should happen when SitePerProcess is enabled.

BUG=475027
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/61dfb23c129e0234c3613d7a2d627784db4f17ee
Cr-Commit-Position: refs/heads/master@{#377920}

[clamy, 2016-02-22 16:41:06]

Description was changed from

==========
PlzNavigate: use RenderFrameHostImpl::IsRendererTransferNeededForNavigation

Following this CL, PlzNavigate will use
RenderFrameHostImpl::IsRendererTransferNeededForNavigation to determine if a
renderer swap should happen when SitePerProcess is enabled.

BUG=475027
==========

to

==========
PlzNavigate: use RenderFrameHostImpl::IsRendererTransferNeededForNavigation

Following this CL, PlzNavigate will use
RenderFrameHostImpl::IsRendererTransferNeededForNavigation to determine if a
renderer swap should happen when SitePerProcess is enabled.

BUG=475027
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[clamy, 2016-02-22 16:56:25]

Description was changed from

==========
PlzNavigate: use RenderFrameHostImpl::IsRendererTransferNeededForNavigation

Following this CL, PlzNavigate will use
RenderFrameHostImpl::IsRendererTransferNeededForNavigation to determine if a
renderer swap should happen when SitePerProcess is enabled.

BUG=475027
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
PlzNavigate: use RenderFrameHostImpl::IsRendererTransferNeededForNavigation

Following this CL, PlzNavigate will use
RenderFrameHostImpl::IsRendererTransferNeededForNavigation to determine if a
renderer swap should happen when SitePerProcess is enabled.

BUG=475027
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[clamy, 2016-02-22 16:56:26]

clamy@chromium.org changed reviewers:
+ nasko@chromium.org

[clamy, 2016-02-22 16:56:34]

@nasko: PTAL

[nasko, 2016-02-25 20:04:45]

LGTM with nits.

https://codereview.chromium.org/1722623003/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_manager.cc (right):

https://codereview.chromium.org/1722623003/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager.cc:825:
ShouldMakeNetworkRequestForURL(request.common_params().url);
nit: Let's swap the order of these two calls. ShouldMakeNetworkRequestForURL is
much simpler and if it returns false, we don't have to evaluate
ShouldMakeNetworkRequestForURL. It also is a bit easier to read as we will only
ask for a transfer if it is known that we should be making a network request.

https://codereview.chromium.org/1722623003/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager.cc:841: if
(no_renderer_swap) {
nit: Empty line before the if statement.

https://codereview.chromium.org/1722623003/diff/20001/testing/buildbot/filter...
File
testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter
(right):

https://codereview.chromium.org/1722623003/diff/20001/testing/buildbot/filter...
testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter:4:
-CrossSiteTransferTest.ReplaceEntryCrossProcessTwice
Do we know why these two tests now fail?

[clamy, 2016-02-26 13:51:02]

Thanks!

https://codereview.chromium.org/1722623003/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_manager.cc (right):

https://codereview.chromium.org/1722623003/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager.cc:825:
ShouldMakeNetworkRequestForURL(request.common_params().url);
On 2016/02/25 20:04:44, nasko wrote:
> nit: Let's swap the order of these two calls. ShouldMakeNetworkRequestForURL
is
> much simpler and if it returns false, we don't have to evaluate
> ShouldMakeNetworkRequestForURL. It also is a bit easier to read as we will
only
> ask for a transfer if it is known that we should be making a network request.

Done. I also noted that we were preventing transfer when the RenderFrame wasn't
live. Added a condition for that.

https://codereview.chromium.org/1722623003/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager.cc:841: if
(no_renderer_swap) {
On 2016/02/25 20:04:44, nasko wrote:
> nit: Empty line before the if statement.

Done.

[clamy, 2016-02-26 13:51:16]

The CQ bit was checked by clamy@chromium.org

[clamy, 2016-02-26 13:51:16]

The patchset sent to the CQ was uploaded after l-g-t-m from nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/1722623003/#ps40001
(title: "Rebase + updates to tests")

[commit-bot: I haz the power, 2016-02-26 13:51:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1722623003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1722623003/40001

[clamy, 2016-02-26 13:52:43]

https://codereview.chromium.org/1722623003/diff/20001/testing/buildbot/filter...
File
testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter
(right):

https://codereview.chromium.org/1722623003/diff/20001/testing/buildbot/filter...
testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter:4:
-CrossSiteTransferTest.ReplaceEntryCrossProcessTwice
On 2016/02/25 20:04:44, nasko wrote:
> Do we know why these two tests now fail?

They used not to work, were fixed magically and then went back to failing. I'll
be looking at those next.

[commit-bot: I haz the power, 2016-02-26 16:41:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-26 16:41:30]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[clamy, 2016-02-26 16:45:08]

The CQ bit was checked by clamy@chromium.org

[commit-bot: I haz the power, 2016-02-26 16:45:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1722623003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1722623003/40001

[commit-bot: I haz the power, 2016-02-26 18:08:56]

Description was changed from

==========
PlzNavigate: use RenderFrameHostImpl::IsRendererTransferNeededForNavigation

Following this CL, PlzNavigate will use
RenderFrameHostImpl::IsRendererTransferNeededForNavigation to determine if a
renderer swap should happen when SitePerProcess is enabled.

BUG=475027
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
PlzNavigate: use RenderFrameHostImpl::IsRendererTransferNeededForNavigation

Following this CL, PlzNavigate will use
RenderFrameHostImpl::IsRendererTransferNeededForNavigation to determine if a
renderer swap should happen when SitePerProcess is enabled.

BUG=475027
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-02-26 18:08:57]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-26 18:10:14]

Description was changed from

==========
PlzNavigate: use RenderFrameHostImpl::IsRendererTransferNeededForNavigation

Following this CL, PlzNavigate will use
RenderFrameHostImpl::IsRendererTransferNeededForNavigation to determine if a
renderer swap should happen when SitePerProcess is enabled.

BUG=475027
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
PlzNavigate: use RenderFrameHostImpl::IsRendererTransferNeededForNavigation

Following this CL, PlzNavigate will use
RenderFrameHostImpl::IsRendererTransferNeededForNavigation to determine if a
renderer swap should happen when SitePerProcess is enabled.

BUG=475027
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/61dfb23c129e0234c3613d7a2d627784db4f17ee
Cr-Commit-Position: refs/heads/master@{#377920}
==========

[commit-bot: I haz the power, 2016-02-26 18:10:15]

Patchset 3 (id:??) landed as
https://crrev.com/61dfb23c129e0234c3613d7a2d627784db4f17ee
Cr-Commit-Position: refs/heads/master@{#377920}