PlzNavigate: blocked browser-initiated navigations should transfer processes sometimes

PlzNavigate: don't reuse current_frame_host() for error pages
if the navigation is browser-initiated. The "stay in current
process to prevent privilege escalation" strategy is only valid
when the navigation was initiated by that process.

(As an aside, it is worth pointing out that current_frame_host is
not necessarily the initiator process.)

This change prevents a CheckWebUIRendererDoesNotDisplayNormalURL browser
crash in the scenario where the current page is chrome://settings, and
the user types in an URL that happens to be blocked by a
NavigationThrottle. This scenario starts being possible once
ExtensionNavigationThrottle starts doing more aggressive blocking of
top-level navigations.

BUG=661324
TEST=ToolbarModelTest.ShouldDisplayURL
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2884123002
Cr-Commit-Position: refs/heads/master@{#472303}
Committed: https://chromium.googlesource.com/chromium/src/+/1c2f3f0b21b2e0eefe0e76e5f519a30970933202

[ncarter (slow), 2017-05-15 23:40:32]

Description was changed from

==========
PlzNavigate: don't reuse current_frame_host() for error pages
if the navigation is browser-initiated. The "stay in current
process to prevent privilege escalation" strategy is only valid
when the navigation was initiated by that process.

(As an aside, it is worth pointing out that current_frame_host is
not necessarily the initiator process.)

This change prevents a CheckWebUIRendererDoesNotDisplayNormalURL browser
crash in the scenario where the current page is chrome://settings, and
the user types in an URL that happens to be blocked by a
NavigationThrottle. This scenario starts being possible once
ExtensionNavigationThrottle starts doing more aggressive blocking of
top-level navigations.

BUG=661324
==========

to

==========
PlzNavigate: don't reuse current_frame_host() for error pages
if the navigation is browser-initiated. The "stay in current
process to prevent privilege escalation" strategy is only valid
when the navigation was initiated by that process.

(As an aside, it is worth pointing out that current_frame_host is
not necessarily the initiator process.)

This change prevents a CheckWebUIRendererDoesNotDisplayNormalURL browser
crash in the scenario where the current page is chrome://settings, and
the user types in an URL that happens to be blocked by a
NavigationThrottle. This scenario starts being possible once
ExtensionNavigationThrottle starts doing more aggressive blocking of
top-level navigations.

BUG=661324
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[ncarter (slow), 2017-05-15 23:40:43]

The CQ bit was checked by nick@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-15 23:41:26]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ncarter (slow), 2017-05-15 23:42:43]

Description was changed from

==========
PlzNavigate: don't reuse current_frame_host() for error pages
if the navigation is browser-initiated. The "stay in current
process to prevent privilege escalation" strategy is only valid
when the navigation was initiated by that process.

(As an aside, it is worth pointing out that current_frame_host is
not necessarily the initiator process.)

This change prevents a CheckWebUIRendererDoesNotDisplayNormalURL browser
crash in the scenario where the current page is chrome://settings, and
the user types in an URL that happens to be blocked by a
NavigationThrottle. This scenario starts being possible once
ExtensionNavigationThrottle starts doing more aggressive blocking of
top-level navigations.

BUG=661324
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
PlzNavigate: don't reuse current_frame_host() for error pages
if the navigation is browser-initiated. The "stay in current
process to prevent privilege escalation" strategy is only valid
when the navigation was initiated by that process.

(As an aside, it is worth pointing out that current_frame_host is
not necessarily the initiator process.)

This change prevents a CheckWebUIRendererDoesNotDisplayNormalURL browser
crash in the scenario where the current page is chrome://settings, and
the user types in an URL that happens to be blocked by a
NavigationThrottle. This scenario starts being possible once
ExtensionNavigationThrottle starts doing more aggressive blocking of
top-level navigations.

BUG=661324
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[ncarter (slow), 2017-05-15 23:49:56]

nick@chromium.org changed reviewers:
+ creis@chromium.org, nasko@chromium.org

[ncarter (slow), 2017-05-15 23:49:56]

nasko or charlie, please review

[Charlie Reis, 2017-05-16 00:04:27]

[+alexmos to CC]

Makes sense-- in a browser-initiated case, the current page wasn't trying to
make the request.  Seems reasonable to create a process for the destination URL
in that case, even though it's going to be blocked.

Just to leave a breadcrumb here, can you mention which CL is introducing the
test for this?

Otherwise, LGTM, but it looks like the
PlzNavigateNavigationHandleImplBrowserTest.ErrorPageBlockedNavigation test is
failing.

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

https://codereview.chromium.org/2884123002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_request.cc:641: //   document, to avoid
creating a process for the destination.
Let's add something like:
This does not include browser-initiated blocked navigations, in which case the
current document wasn't trying to make the request.

[commit-bot: I haz the power, 2017-05-16 00:21:49]

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

[commit-bot: I haz the power, 2017-05-16 00:21:50]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[ncarter (slow), 2017-05-16 20:06:59]

PTAL. I fixed the test and rewrote the comment.

[ncarter (slow), 2017-05-16 20:07:08]

The CQ bit was checked by nick@chromium.org

[ncarter (slow), 2017-05-16 20:07:08]

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/2884123002/#ps60001
(title: "Phrasing.")

[commit-bot: I haz the power, 2017-05-16 20:08:49]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Reis, 2017-05-16 20:41:08]

Thanks!  LGTM.

[commit-bot: I haz the power, 2017-05-16 21:53:36]

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

[commit-bot: I haz the power, 2017-05-16 21:53:37]

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

[ncarter (slow), 2017-05-16 22:06:07]

The CQ bit was checked by nick@chromium.org

[commit-bot: I haz the power, 2017-05-16 22:07:30]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ncarter (slow), 2017-05-16 22:27:28]

Description was changed from

==========
PlzNavigate: don't reuse current_frame_host() for error pages
if the navigation is browser-initiated. The "stay in current
process to prevent privilege escalation" strategy is only valid
when the navigation was initiated by that process.

(As an aside, it is worth pointing out that current_frame_host is
not necessarily the initiator process.)

This change prevents a CheckWebUIRendererDoesNotDisplayNormalURL browser
crash in the scenario where the current page is chrome://settings, and
the user types in an URL that happens to be blocked by a
NavigationThrottle. This scenario starts being possible once
ExtensionNavigationThrottle starts doing more aggressive blocking of
top-level navigations.

BUG=661324
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
PlzNavigate: don't reuse current_frame_host() for error pages
if the navigation is browser-initiated. The "stay in current
process to prevent privilege escalation" strategy is only valid
when the navigation was initiated by that process.

(As an aside, it is worth pointing out that current_frame_host is
not necessarily the initiator process.)

This change prevents a CheckWebUIRendererDoesNotDisplayNormalURL browser
crash in the scenario where the current page is chrome://settings, and
the user types in an URL that happens to be blocked by a
NavigationThrottle. This scenario starts being possible once
ExtensionNavigationThrottle starts doing more aggressive blocking of
top-level navigations.

BUG=661324
TEST=ToolbarModelTest.ShouldDisplayURL
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2017-05-17 03:35:23]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494972367440200,
"parent_rev": "a6fc2f20dd92dbf5ea9b73ae877708efd0f432d4", "commit_rev":
"1c2f3f0b21b2e0eefe0e76e5f519a30970933202"}

[commit-bot: I haz the power, 2017-05-17 03:35:32]

Description was changed from

==========
PlzNavigate: don't reuse current_frame_host() for error pages
if the navigation is browser-initiated. The "stay in current
process to prevent privilege escalation" strategy is only valid
when the navigation was initiated by that process.

(As an aside, it is worth pointing out that current_frame_host is
not necessarily the initiator process.)

This change prevents a CheckWebUIRendererDoesNotDisplayNormalURL browser
crash in the scenario where the current page is chrome://settings, and
the user types in an URL that happens to be blocked by a
NavigationThrottle. This scenario starts being possible once
ExtensionNavigationThrottle starts doing more aggressive blocking of
top-level navigations.

BUG=661324
TEST=ToolbarModelTest.ShouldDisplayURL
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
PlzNavigate: don't reuse current_frame_host() for error pages
if the navigation is browser-initiated. The "stay in current
process to prevent privilege escalation" strategy is only valid
when the navigation was initiated by that process.

(As an aside, it is worth pointing out that current_frame_host is
not necessarily the initiator process.)

This change prevents a CheckWebUIRendererDoesNotDisplayNormalURL browser
crash in the scenario where the current page is chrome://settings, and
the user types in an URL that happens to be blocked by a
NavigationThrottle. This scenario starts being possible once
ExtensionNavigationThrottle starts doing more aggressive blocking of
top-level navigations.

BUG=661324
TEST=ToolbarModelTest.ShouldDisplayURL
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2884123002
Cr-Commit-Position: refs/heads/master@{#472303}
Committed:
https://chromium.googlesource.com/chromium/src/+/1c2f3f0b21b2e0eefe0e76e5f519...
==========

[commit-bot: I haz the power, 2017-05-17 03:35:34]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/1c2f3f0b21b2e0eefe0e76e5f519...