PlzNavigate: fix test FrameNavigationEntry_SubframeHistoryFallback

PlzNavigate: fix test FrameNavigationEntry_SubframeHistoryFallback

This CL fixes
NavigationControllerBrowserTest.FrameNavigationEntry_SubframeHistoryFallback by
changing checks in NavigatorImpl::RequestOpenURL. This function assumed it
would be called on a subframe only when --site-per-process was enabled.
However, it's possible to call it when a history navigation to a subframe fails
with SiteIsolationPolicy::UseSubframeNavigationEntries enabled. Since
PlzNavigate uses subframe navigation entries, but not --site-per-process, we
would run into the checks.

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

Committed: https://crrev.com/ff990bb6350e5f081a98203e502ad3704eea78d1
Cr-Commit-Position: refs/heads/master@{#389453}

[clamy, 2016-04-15 12:36:10]

Description was changed from

==========
PlzNavigate: fix test FrameNavigationEntry_SubframeHistoryFallback

This CL fixes
NavigationControllerBrowserTest.FrameNavigationEntry_SubframeHistoryFallback by
changing checks in NavigatorImpl::RequestOpenURL. This function assumed it
would be called on a subframe only when --site-per-process was enabled.
However, it's possible to call it when a history navigation to a subframe fails
with SiteIsolationPolicy::UseSubframeNavigationEntries enabled. Since
PlzNavigate uses subframe navigation entries, but not --site-per-process, we
would run into the checks.

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

to

==========
PlzNavigate: fix test FrameNavigationEntry_SubframeHistoryFallback

This CL fixes
NavigationControllerBrowserTest.FrameNavigationEntry_SubframeHistoryFallback by
changing checks in NavigatorImpl::RequestOpenURL. This function assumed it
would be called on a subframe only when --site-per-process was enabled.
However, it's possible to call it when a history navigation to a subframe fails
with SiteIsolationPolicy::UseSubframeNavigationEntries enabled. Since
PlzNavigate uses subframe navigation entries, but not --site-per-process, we
would run into the checks.

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

[clamy, 2016-04-15 12:37:11]

Description was changed from

==========
PlzNavigate: fix test FrameNavigationEntry_SubframeHistoryFallback

This CL fixes
NavigationControllerBrowserTest.FrameNavigationEntry_SubframeHistoryFallback by
changing checks in NavigatorImpl::RequestOpenURL. This function assumed it
would be called on a subframe only when --site-per-process was enabled.
However, it's possible to call it when a history navigation to a subframe fails
with SiteIsolationPolicy::UseSubframeNavigationEntries enabled. Since
PlzNavigate uses subframe navigation entries, but not --site-per-process, we
would run into the checks.

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

to

==========
PlzNavigate: fix test FrameNavigationEntry_SubframeHistoryFallback

This CL fixes
NavigationControllerBrowserTest.FrameNavigationEntry_SubframeHistoryFallback by
changing checks in NavigatorImpl::RequestOpenURL. This function assumed it
would be called on a subframe only when --site-per-process was enabled.
However, it's possible to call it when a history navigation to a subframe fails
with SiteIsolationPolicy::UseSubframeNavigationEntries enabled. Since
PlzNavigate uses subframe navigation entries, but not --site-per-process, we
would run into the checks.

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

[clamy, 2016-04-15 12:37:11]

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

[clamy, 2016-04-15 12:37:27]

@creis: PTAL

[Charlie Reis, 2016-04-15 17:36:42]

[+site-isolation-reviews]

Yes, looks like I used the wrong policy check there.  Using FNEs does imply
going to NavigationController for subframes (and not using HistoryController in
the renderer process), even when OOPIFs are not enabled.

There's a linux_site_isolation failure, but I think that's probably unrelated
since that test also failed for Alex:
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_iso...
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_iso...

There are also failures on the linux_chromium_browser_side_navigation_rel.  Are
those real?

If the tests aren't regressing, this LGTM.

https://codereview.chromium.org/1888913003/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/1888913003/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigator_impl.cc:656:
SiteIsolationPolicy::UseSubframeNavigationEntries());
nit: This should just be UseSubframeNavigationEntries(), which will already
return true if AreCrossProcessFramesPossible() (since OOPIFs depend on FNEs).

https://codereview.chromium.org/1888913003/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigator_impl.cc:678:
SiteIsolationPolicy::UseSubframeNavigationEntries()) &&
Same.

[clamy, 2016-04-25 11:05:34]

Thanks! The failures on the linux-browser-side-navigation bot are due to an
issues we're having with the trybot where layout tests will always fail, even if
they work fine on the builder.

https://codereview.chromium.org/1888913003/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/1888913003/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigator_impl.cc:656:
SiteIsolationPolicy::UseSubframeNavigationEntries());
On 2016/04/15 17:36:42, Charlie Reis wrote:
> nit: This should just be UseSubframeNavigationEntries(), which will already
> return true if AreCrossProcessFramesPossible() (since OOPIFs depend on FNEs).

Removed this block due to code change in navigator_impl.

https://codereview.chromium.org/1888913003/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigator_impl.cc:678:
SiteIsolationPolicy::UseSubframeNavigationEntries()) &&
On 2016/04/15 17:36:42, Charlie Reis wrote:
> Same.

Done.

[clamy, 2016-04-25 11:05:41]

The CQ bit was checked by clamy@chromium.org

[clamy, 2016-04-25 11:05:42]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org
Link to the patchset: https://codereview.chromium.org/1888913003/#ps20001
(title: "Rebase + addressed comments")

[commit-bot: I haz the power, 2016-04-25 11:05:56]

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

[commit-bot: I haz the power, 2016-04-25 12:23:30]

Description was changed from

==========
PlzNavigate: fix test FrameNavigationEntry_SubframeHistoryFallback

This CL fixes
NavigationControllerBrowserTest.FrameNavigationEntry_SubframeHistoryFallback by
changing checks in NavigatorImpl::RequestOpenURL. This function assumed it
would be called on a subframe only when --site-per-process was enabled.
However, it's possible to call it when a history navigation to a subframe fails
with SiteIsolationPolicy::UseSubframeNavigationEntries enabled. Since
PlzNavigate uses subframe navigation entries, but not --site-per-process, we
would run into the checks.

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

to

==========
PlzNavigate: fix test FrameNavigationEntry_SubframeHistoryFallback

This CL fixes
NavigationControllerBrowserTest.FrameNavigationEntry_SubframeHistoryFallback by
changing checks in NavigatorImpl::RequestOpenURL. This function assumed it
would be called on a subframe only when --site-per-process was enabled.
However, it's possible to call it when a history navigation to a subframe fails
with SiteIsolationPolicy::UseSubframeNavigationEntries enabled. Since
PlzNavigate uses subframe navigation entries, but not --site-per-process, we
would run into the checks.

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

[commit-bot: I haz the power, 2016-04-25 12:23:31]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-25 12:24:49]

Description was changed from

==========
PlzNavigate: fix test FrameNavigationEntry_SubframeHistoryFallback

This CL fixes
NavigationControllerBrowserTest.FrameNavigationEntry_SubframeHistoryFallback by
changing checks in NavigatorImpl::RequestOpenURL. This function assumed it
would be called on a subframe only when --site-per-process was enabled.
However, it's possible to call it when a history navigation to a subframe fails
with SiteIsolationPolicy::UseSubframeNavigationEntries enabled. Since
PlzNavigate uses subframe navigation entries, but not --site-per-process, we
would run into the checks.

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

to

==========
PlzNavigate: fix test FrameNavigationEntry_SubframeHistoryFallback

This CL fixes
NavigationControllerBrowserTest.FrameNavigationEntry_SubframeHistoryFallback by
changing checks in NavigatorImpl::RequestOpenURL. This function assumed it
would be called on a subframe only when --site-per-process was enabled.
However, it's possible to call it when a history navigation to a subframe fails
with SiteIsolationPolicy::UseSubframeNavigationEntries enabled. Since
PlzNavigate uses subframe navigation entries, but not --site-per-process, we
would run into the checks.

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

Committed: https://crrev.com/ff990bb6350e5f081a98203e502ad3704eea78d1
Cr-Commit-Position: refs/heads/master@{#389453}
==========

[commit-bot: I haz the power, 2016-04-25 12:24:50]

Patchset 2 (id:??) landed as
https://crrev.com/ff990bb6350e5f081a98203e502ad3704eea78d1
Cr-Commit-Position: refs/heads/master@{#389453}