DCHECK: Browser asking renderer to commit URLs it is not allowed to.

DCHECK: Browser asking renderer to commit URLs it is not allowed to.

This CL adds a DCHECK when the browser ask the renderer to commit an URL
and then kill the renderer when it does.

This CL was previously related with a problem when the renderer tries to
commit an error page to the chrome webstore inside an iframe. It turns
out that there was no more problems with PlzNavigate since the
ChromeWebStore error page uses a different process.

BUG=588314, 622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2697713005
Cr-Commit-Position: refs/heads/master@{#452808}
Committed: https://chromium.googlesource.com/chromium/src/+/4ba80de54a09324ae69bde773f194df0eec6e8b3

[arthursonzogni, 2017-02-15 16:11:10]

Description was changed from

==========
PlzNavigate: A more generic fix about error-pages/CWS interactions.

There is currently a problem with error pages, their url are the ones of the
blocked page. It is a problem when the renderer is killed whenever it tries to
commit this URL. For instance when the ChromeWebStore is loaded inside an
iframe.

This goal of this CL was to move a previous fix from the RendererFrameImpl to
the RendererFrameHostImpl. It now applies to every error code instead of only
BLOCKED_BY_RESPONSE. Moreover, it only targets the error pages that would have
caused the renderer to be killed.

It was harder to produce the same thing on non-PlzNavigate code, so the
previous fix was not removed for the moment.

The previous fix that is still there can be found in:
RenderFrameImpl::LoadNavigationErrorPage(....)

This CL was made to make progress on:
https://codereview.chromium.org/2655463006/

BUG=588314,622385
==========

to

==========
PlzNavigate: A more generic fix about error-pages/CWS interactions.

There is currently a problem with error pages, their url are the ones of the
blocked page. It is a problem when the renderer is killed whenever it tries to
commit this URL. For instance when the ChromeWebStore is loaded inside an
iframe.

This goal of this CL was to move a previous fix from the RendererFrameImpl to
the RendererFrameHostImpl. It now applies to every error code instead of only
BLOCKED_BY_RESPONSE. Moreover, it only targets the error pages that would have
caused the renderer to be killed.

It was harder to produce the same thing on non-PlzNavigate code, so the
previous fix was not removed for the moment.

The previous fix that is still there can be found in:
RenderFrameImpl::LoadNavigationErrorPage(....)

This CL was made to make progress on:
https://codereview.chromium.org/2655463006/

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[arthursonzogni, 2017-02-15 16:12:37]

Description was changed from

==========
PlzNavigate: A more generic fix about error-pages/CWS interactions.

There is currently a problem with error pages, their url are the ones of the
blocked page. It is a problem when the renderer is killed whenever it tries to
commit this URL. For instance when the ChromeWebStore is loaded inside an
iframe.

This goal of this CL was to move a previous fix from the RendererFrameImpl to
the RendererFrameHostImpl. It now applies to every error code instead of only
BLOCKED_BY_RESPONSE. Moreover, it only targets the error pages that would have
caused the renderer to be killed.

It was harder to produce the same thing on non-PlzNavigate code, so the
previous fix was not removed for the moment.

The previous fix that is still there can be found in:
RenderFrameImpl::LoadNavigationErrorPage(....)

This CL was made to make progress on:
https://codereview.chromium.org/2655463006/

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
PlzNavigate: A more generic fix about error-pages/CWS interactions.

There is currently a problem with error pages, their url are the ones of the
blocked page. It is a problem when the renderer is killed whenever it tries to
commit this URL. For instance when the ChromeWebStore is loaded inside an
iframe.

This goal of this CL was to move a previous fix from the RendererFrameImpl to
the RendererFrameHostImpl. It now applies to every error code instead of only
BLOCKED_BY_RESPONSE. Moreover, it only targets the error pages that would have
caused the renderer to be killed.

It was harder to produce the same thing on non-PlzNavigate code, so the
previous fix was not removed for the moment.

The previous fix that is still there can be found in:
RenderFrameImpl::LoadNavigationErrorPage(....)

This CL was made to make progress on:
https://codereview.chromium.org/2655463006/

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation,linux_chromium_browser_side_navigation_rel
==========

[arthursonzogni, 2017-02-15 16:12:43]

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

[commit-bot: I haz the power, 2017-02-15 16:13:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 16:47:29]

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

[commit-bot: I haz the power, 2017-02-15 16:47:31]

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...)

[arthursonzogni, 2017-02-15 17:19:33]

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

[commit-bot: I haz the power, 2017-02-15 17:20:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 21:26:22]

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

[commit-bot: I haz the power, 2017-02-15 21:26:24]

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

[arthursonzogni, 2017-02-16 08:52:25]

Description was changed from

==========
PlzNavigate: A more generic fix about error-pages/CWS interactions.

There is currently a problem with error pages, their url are the ones of the
blocked page. It is a problem when the renderer is killed whenever it tries to
commit this URL. For instance when the ChromeWebStore is loaded inside an
iframe.

This goal of this CL was to move a previous fix from the RendererFrameImpl to
the RendererFrameHostImpl. It now applies to every error code instead of only
BLOCKED_BY_RESPONSE. Moreover, it only targets the error pages that would have
caused the renderer to be killed.

It was harder to produce the same thing on non-PlzNavigate code, so the
previous fix was not removed for the moment.

The previous fix that is still there can be found in:
RenderFrameImpl::LoadNavigationErrorPage(....)

This CL was made to make progress on:
https://codereview.chromium.org/2655463006/

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation,linux_chromium_browser_side_navigation_rel
==========

to

==========
PlzNavigate: A more generic fix about error-pages/CWS interactions.

There is currently a problem with error pages, their urls are the ones of the
blocked page. It is a problem when the renderer is killed whenever it tries to
commit this URL. For instance when the ChromeWebStore is loaded inside an
iframe.

This goal of this CL was to move a previous fix from the RendererFrameImpl to
the RendererFrameHostImpl. It now applies to every error code instead of only
BLOCKED_BY_RESPONSE. Moreover, it only targets the error pages that would have
caused the renderer to be killed.

It was harder to produce the same thing on non-PlzNavigate code, so the
previous fix was not removed for the moment.

The previous fix that is still there can be found in:
RenderFrameImpl::LoadNavigationErrorPage(....)

This CL was made to make progress on:
https://codereview.chromium.org/2655463006/

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation,linux_chromium_browser_side_navigation_rel
==========

[arthursonzogni, 2017-02-16 08:55:37]

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

[arthursonzogni, 2017-02-16 08:55:38]

Hi Camille,

Please, could you take a look?

[clamy, 2017-02-16 13:45:59]

Thanks! A few comments below. Also, it seems quite a few tests are failing on
the PlzNavigate bot with this patch. Could you check?

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

https://codereview.chromium.org/2697713005/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:2768: // Don't ask the
renderer to commit an URL if the browser will decide to kill
nit: s/will decide to kill it if it does/will kill it when it does

https://codereview.chromium.org/2697713005/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:2774:
CommonNavigationParams new_common_params;
These CommonNavigationParams are badly initialized. Why not just rewrite the url
of the current CommonNavigationParams?

[arthursonzogni, 2017-02-17 09:30:47]

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

[commit-bot: I haz the power, 2017-02-17 09:30:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[arthursonzogni, 2017-02-17 09:33:20]

Thanks Camille.

I have the same failure with and without this patch, but I only have 2 tests
failing among this 25 layout test. I will run the trybot again to see if it
change.

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

https://codereview.chromium.org/2697713005/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:2774:
CommonNavigationParams new_common_params;
On 2017/02/16 13:45:59, clamy wrote:
> These CommonNavigationParams are badly initialized. Why not just rewrite the
url
> of the current CommonNavigationParams?

I wanted to be sure that any traces of the previous navigation attempt was
removed. But yes, using the default constructor of CommonNavigationParams is not
enough.
BTW, the common_params and request_params are const.
Please tell me if it is okay now.

[commit-bot: I haz the power, 2017-02-17 11:09:40]

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

[commit-bot: I haz the power, 2017-02-17 11:09:41]

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

[arthursonzogni, 2017-02-17 11:54:56]

Patchset #3 (id:40001) has been deleted

[arthursonzogni, 2017-02-17 12:01:02]

Description was changed from

==========
PlzNavigate: A more generic fix about error-pages/CWS interactions.

There is currently a problem with error pages, their urls are the ones of the
blocked page. It is a problem when the renderer is killed whenever it tries to
commit this URL. For instance when the ChromeWebStore is loaded inside an
iframe.

This goal of this CL was to move a previous fix from the RendererFrameImpl to
the RendererFrameHostImpl. It now applies to every error code instead of only
BLOCKED_BY_RESPONSE. Moreover, it only targets the error pages that would have
caused the renderer to be killed.

It was harder to produce the same thing on non-PlzNavigate code, so the
previous fix was not removed for the moment.

The previous fix that is still there can be found in:
RenderFrameImpl::LoadNavigationErrorPage(....)

This CL was made to make progress on:
https://codereview.chromium.org/2655463006/

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation,linux_chromium_browser_side_navigation_rel
==========

to

==========
PlzNavigate: A more generic fix about error-pages/CWS interactions.

There is currently a problem with error pages, their urls are the ones of the
blocked page. It is a problem when the renderer is killed whenever it tries to
commit this URL. For instance when the ChromeWebStore is loaded inside an
iframe.

This goal of this CL was to move a previous fix from the RendererFrameImpl to
the RendererFrameHostImpl. It now applies to every error code instead of only
BLOCKED_BY_RESPONSE. Moreover, it only targets the error pages that would have
caused the renderer to be killed.

It was harder to produce the same thing on non-PlzNavigate code, so the
previous fix was not removed for the moment.

The previous fix that is still there can be found in:
RenderFrameImpl::LoadNavigationErrorPage(....)

This CL was made to make progress on:
https://codereview.chromium.org/2655463006/

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.linux:linux_site_isolation,linux_chromium_browser_side_navigation_rel
==========

[arthursonzogni, 2017-02-17 12:01:10]

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

[commit-bot: I haz the power, 2017-02-17 12:01:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 12:01:33]

There were warnings when CQ was processing your CL:
 * CQ is not running the linux_site_isolation trybot, per your
CQ_INCLUDE_TRYBOTS flag request. That bot was already specified

[commit-bot: I haz the power, 2017-02-17 12:02:00]

There were warnings when CQ was processing your CL:
 * CQ is not running the linux_site_isolation trybot, per your
CQ_INCLUDE_TRYBOTS flag request. That bot was already specified

[commit-bot: I haz the power, 2017-02-17 12:31:49]

There were warnings when CQ was processing your CL:
 * CQ is not running the linux_site_isolation trybot, per your
CQ_INCLUDE_TRYBOTS flag request. That bot was already specified

[commit-bot: I haz the power, 2017-02-17 13:01:35]

There were warnings when CQ was processing your CL:
 * CQ is not running the linux_site_isolation trybot, per your
CQ_INCLUDE_TRYBOTS flag request. That bot was already specified

[commit-bot: I haz the power, 2017-02-17 13:32:14]

There were warnings when CQ was processing your CL:
 * CQ is not running the linux_site_isolation trybot, per your
CQ_INCLUDE_TRYBOTS flag request. That bot was already specified

[commit-bot: I haz the power, 2017-02-17 14:02:22]

There were warnings when CQ was processing your CL:
 * CQ is not running the linux_site_isolation trybot, per your
CQ_INCLUDE_TRYBOTS flag request. That bot was already specified

[commit-bot: I haz the power, 2017-02-17 14:30:25]

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

[commit-bot: I haz the power, 2017-02-17 14:30:26]

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

[clamy, 2017-02-21 15:32:25]

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

https://codereview.chromium.org/2697713005/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:2774:
CommonNavigationParams new_common_params;
On 2017/02/17 09:33:19, arthursonzogni wrote:
> On 2017/02/16 13:45:59, clamy wrote:
> > These CommonNavigationParams are badly initialized. Why not just rewrite the
> url
> > of the current CommonNavigationParams?
> 
> I wanted to be sure that any traces of the previous navigation attempt was
> removed. But yes, using the default constructor of CommonNavigationParams is
not
> enough.
> BTW, the common_params and request_params are const.
> Please tell me if it is okay now.

I see. Then we should move this code to NavigationRequest. I've just made a
patch that fold the NavigatorImpl code for failed navigations in
NavigationRequest. Could you move this resetting of the url to
NavigationRequest::OnRequestFailed as it is implemented in that patch? It should
be easier to update the url then.

[arthursonzogni, 2017-02-22 16:25:03]

Thanks Camille.
It works % these two things that might be unrelated.

* I found this bug with PlzNavigate:
https://bugs.chromium.org/p/chromium/issues/detail?id=695067

I have also found that when we tries to navigate to the CWS inside an iframe:
in ChromeContentBrowserClientExtensionsPart::CanCommitURL(...),
ProcessMap::Get(process_host->GetBrowserContext())->Contains(new_extension->id(),
process_host->GetID())
returns true with PlzNavigate and false without it.
I had to modify ChromeContentBrowserClientExtensionsPart::CanCommitURL (i.e.
making it returning false instead of true) to test my CL.

[clamy, 2017-02-22 16:40:16]

Thanks! Current version lgtm with one comment.

The bug looks weird, might not be PlzNavigate related?

For the second issue, are we switching process with PlzNavigate when we would
not without PlzNavigate? We have an issue when PlzNavigate isn't enabled where
we don't switch renderer processes on error pages even when we should.

https://codereview.chromium.org/2697713005/diff/80001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.h (right):

https://codereview.chromium.org/2697713005/diff/80001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.h:614: bool CanCommitURL(const
GURL& url);
You also need to move the function in the .cc file to match the header ordering.

[arthursonzogni, 2017-02-22 17:31:13]

> The bug looks weird, might not be PlzNavigate related?
I don't know, it is probably 50% / 50%
This bug happens only with PlzNavigate when the mouse enter and leave the error
page.
But this error also happens without PlzNavigate in a flaky way.

> For the second issue, are we switching process with PlzNavigate when we would
> not without PlzNavigate? We have an issue when PlzNavigate isn't enabled where
> we don't switch renderer processes on error pages even when we should.
It is probably that, at least it is what the line of code says. I don't know how
to check if we are switching process, I will investigate.

https://codereview.chromium.org/2697713005/diff/80001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.h (right):

https://codereview.chromium.org/2697713005/diff/80001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.h:614: bool CanCommitURL(const
GURL& url);
On 2017/02/22 16:40:16, clamy wrote:
> You also need to move the function in the .cc file to match the header
ordering.

Done.

[clamy, 2017-02-23 12:09:52]

On 2017/02/22 17:31:13, arthursonzogni wrote:
> > The bug looks weird, might not be PlzNavigate related?
> I don't know, it is probably 50% / 50%
> This bug happens only with PlzNavigate when the mouse enter and leave the
error
> page.
> But this error also happens without PlzNavigate in a flaky way.
> 
> > For the second issue, are we switching process with PlzNavigate when we
would
> > not without PlzNavigate? We have an issue when PlzNavigate isn't enabled
where
> > we don't switch renderer processes on error pages even when we should.
> It is probably that, at least it is what the line of code says. I don't know
how
> to check if we are switching process, I will investigate.

There are several ways: first you can check whether it is the speculative RFH
(in PlzNavigate) or the pending RFH (non PlzNavigate) that commits the
navigation. This will change the current RFH of the frame. You can also access
the RenderProcessHost from the RFH. The RPH has an id, different ids means we've
switched processes.

> 
>
https://codereview.chromium.org/2697713005/diff/80001/content/browser/frame_h...
> File content/browser/frame_host/render_frame_host_impl.h (right):
> 
>
https://codereview.chromium.org/2697713005/diff/80001/content/browser/frame_h...
> content/browser/frame_host/render_frame_host_impl.h:614: bool
CanCommitURL(const
> GURL& url);
> On 2017/02/22 16:40:16, clamy wrote:
> > You also need to move the function in the .cc file to match the header
> ordering.
> 
> Done.

[arthursonzogni, 2017-02-23 14:23:43]

On 2017/02/23 12:09:52, clamy wrote:
> On 2017/02/22 17:31:13, arthursonzogni wrote:
> > > The bug looks weird, might not be PlzNavigate related?
> > I don't know, it is probably 50% / 50%
> > This bug happens only with PlzNavigate when the mouse enter and leave the
> error
> > page.
> > But this error also happens without PlzNavigate in a flaky way.
> > 
> > > For the second issue, are we switching process with PlzNavigate when we
> would
> > > not without PlzNavigate? We have an issue when PlzNavigate isn't enabled
> where
> > > we don't switch renderer processes on error pages even when we should.
> > It is probably that, at least it is what the line of code says. I don't know
> how
> > to check if we are switching process, I will investigate.
> 
> There are several ways: first you can check whether it is the speculative RFH
> (in PlzNavigate) or the pending RFH (non PlzNavigate) that commits the
> navigation. This will change the current RFH of the frame. You can also access
> the RenderProcessHost from the RFH. The RPH has an id, different ids means
we've
> switched processes.
> 
> > 
> >
>
https://codereview.chromium.org/2697713005/diff/80001/content/browser/frame_h...
> > File content/browser/frame_host/render_frame_host_impl.h (right):
> > 
> >
>
https://codereview.chromium.org/2697713005/diff/80001/content/browser/frame_h...
> > content/browser/frame_host/render_frame_host_impl.h:614: bool
> CanCommitURL(const
> > GURL& url);
> > On 2017/02/22 16:40:16, clamy wrote:
> > > You also need to move the function in the .cc file to match the header
> > ordering.
> > 
> > Done.

I tested what you wanted to know. There is a process switch in both mode
PlzNavigate(on|off). The iframe is always in another process.

[arthursonzogni, 2017-02-23 14:51:08]

On 2017/02/23 14:23:43, arthursonzogni wrote:
> On 2017/02/23 12:09:52, clamy wrote:
> > On 2017/02/22 17:31:13, arthursonzogni wrote:
> > > > The bug looks weird, might not be PlzNavigate related?
> > > I don't know, it is probably 50% / 50%
> > > This bug happens only with PlzNavigate when the mouse enter and leave the
> > error
> > > page.
> > > But this error also happens without PlzNavigate in a flaky way.
> > > 
> > > > For the second issue, are we switching process with PlzNavigate when we
> > would
> > > > not without PlzNavigate? We have an issue when PlzNavigate isn't enabled
> > where
> > > > we don't switch renderer processes on error pages even when we should.
> > > It is probably that, at least it is what the line of code says. I don't
know
> > how
> > > to check if we are switching process, I will investigate.
> > 
> > There are several ways: first you can check whether it is the speculative
RFH
> > (in PlzNavigate) or the pending RFH (non PlzNavigate) that commits the
> > navigation. This will change the current RFH of the frame. You can also
access
> > the RenderProcessHost from the RFH. The RPH has an id, different ids means
> we've
> > switched processes.
> > 
> > > 
> > >
> >
>
https://codereview.chromium.org/2697713005/diff/80001/content/browser/frame_h...
> > > File content/browser/frame_host/render_frame_host_impl.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2697713005/diff/80001/content/browser/frame_h...
> > > content/browser/frame_host/render_frame_host_impl.h:614: bool
> > CanCommitURL(const
> > > GURL& url);
> > > On 2017/02/22 16:40:16, clamy wrote:
> > > > You also need to move the function in the .cc file to match the header
> > > ordering.
> > > 
> > > Done.
> 
> I tested what you wanted to know. There is a process switch in both mode
> PlzNavigate(on|off). The iframe is always in another process.

I also tested with an iframe that navigate with a redirect to the chrome
webstore.
With PlzNavigate => error_page(blank) in another process.
Without          => error_page(blank) in the same_process.

That is explaining the difference in CanCommitURL.

Do you think I can commit the patch?

[arthursonzogni, 2017-02-23 17:02:55]

Description was changed from

==========
PlzNavigate: A more generic fix about error-pages/CWS interactions.

There is currently a problem with error pages, their urls are the ones of the
blocked page. It is a problem when the renderer is killed whenever it tries to
commit this URL. For instance when the ChromeWebStore is loaded inside an
iframe.

This goal of this CL was to move a previous fix from the RendererFrameImpl to
the RendererFrameHostImpl. It now applies to every error code instead of only
BLOCKED_BY_RESPONSE. Moreover, it only targets the error pages that would have
caused the renderer to be killed.

It was harder to produce the same thing on non-PlzNavigate code, so the
previous fix was not removed for the moment.

The previous fix that is still there can be found in:
RenderFrameImpl::LoadNavigationErrorPage(....)

This CL was made to make progress on:
https://codereview.chromium.org/2655463006/

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.linux:linux_site_isolation,linux_chromium_browser_side_navigation_rel
==========

to

==========
DCHECK: Browser asking renderer to commit URLs it is not allowed to.

This CL adds a DCHECK when the browser ask the renderer to commit an URL
and then kill the renderer when it does.

This CL was previously related with a problem when the renderer tries to
commit an error page to the chrome webstore inside an iframe. It turns
out that there was no more problems with PlzNavigate since the
ChromeWebStore error page uses a different process.

BUG=588314,622385
==========

[arthursonzogni, 2017-02-23 17:04:02]

Description was changed from

==========
DCHECK: Browser asking renderer to commit URLs it is not allowed to.

This CL adds a DCHECK when the browser ask the renderer to commit an URL
and then kill the renderer when it does.

This CL was previously related with a problem when the renderer tries to
commit an error page to the chrome webstore inside an iframe. It turns
out that there was no more problems with PlzNavigate since the
ChromeWebStore error page uses a different process.

BUG=588314,622385
==========

to

==========
DCHECK: Browser asking renderer to commit URLs it is not allowed to.

This CL adds a DCHECK when the browser ask the renderer to commit an URL
and then kill the renderer when it does.

This CL was previously related with a problem when the renderer tries to
commit an error page to the chrome webstore inside an iframe. It turns
out that there was no more problems with PlzNavigate since the
ChromeWebStore error page uses a different process.

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[arthursonzogni, 2017-02-23 17:04:42]

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

[commit-bot: I haz the power, 2017-02-23 17:05:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[arthursonzogni, 2017-02-23 17:05:44]

So, there is no more problems with PlzNavigate when an error page to the
ChromeWebStore is displayed.

I turn the condition into a DCHECK.

[arthursonzogni, 2017-02-23 17:06:39]

Patchset #6 (id:120001) has been deleted

[arthursonzogni, 2017-02-23 17:07:07]

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

[commit-bot: I haz the power, 2017-02-23 17:07:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 17:28:44]

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

[commit-bot: I haz the power, 2017-02-23 17:28:44]

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

[arthursonzogni, 2017-02-24 10:50:13]

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

[commit-bot: I haz the power, 2017-02-24 10:50:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 10:59:57]

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

[commit-bot: I haz the power, 2017-02-24 10:59:58]

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

[arthursonzogni, 2017-02-24 11:47:11]

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

[arthursonzogni, 2017-02-24 11:47:22]

Patchset #7 (id:160001) has been deleted

[commit-bot: I haz the power, 2017-02-24 11:47:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 13:05:01]

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

[commit-bot: I haz the power, 2017-02-24 13:05:02]

Dry run: This issue passed the CQ dry run.

[arthursonzogni, 2017-02-24 13:35:54]

The CQ bit was checked by arthursonzogni@chromium.org

[arthursonzogni, 2017-02-24 13:35:55]

The patchset sent to the CQ was uploaded after l-g-t-m from clamy@chromium.org
Link to the patchset: https://codereview.chromium.org/2697713005/#ps180001
(title: "git cl try")

[commit-bot: I haz the power, 2017-02-24 13:36:09]

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

[commit-bot: I haz the power, 2017-02-24 13:39:41]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1487943354678710,
"parent_rev": "f051ea6a183af11218975773813a43a0753603c9", "commit_rev":
"4ba80de54a09324ae69bde773f194df0eec6e8b3"}

[commit-bot: I haz the power, 2017-02-24 13:40:15]

Description was changed from

==========
DCHECK: Browser asking renderer to commit URLs it is not allowed to.

This CL adds a DCHECK when the browser ask the renderer to commit an URL
and then kill the renderer when it does.

This CL was previously related with a problem when the renderer tries to
commit an error page to the chrome webstore inside an iframe. It turns
out that there was no more problems with PlzNavigate since the
ChromeWebStore error page uses a different process.

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
DCHECK: Browser asking renderer to commit URLs it is not allowed to.

This CL adds a DCHECK when the browser ask the renderer to commit an URL
and then kill the renderer when it does.

This CL was previously related with a problem when the renderer tries to
commit an error page to the chrome webstore inside an iframe. It turns
out that there was no more problems with PlzNavigate since the
ChromeWebStore error page uses a different process.

BUG=588314,622385
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2697713005
Cr-Commit-Position: refs/heads/master@{#452808}
Committed:
https://chromium.googlesource.com/chromium/src/+/4ba80de54a09324ae69bde773f19...
==========

[commit-bot: I haz the power, 2017-02-24 13:40:16]

Committed patchset #7 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/4ba80de54a09324ae69bde773f19...