Simplify decidePolicyForNavigation + remove POST-only-in-http(s) DCHECK.

Simplify decidePolicyForNavigation + remove POST-only-in-http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/96db896a6696944f84bb31f1baea9b4bb88deb91
Cr-Commit-Position: refs/heads/master@{#407033}

[Łukasz Anforowicz, 2016-07-20 21:55:35]

Description was changed from

==========
Simplify decidePolicyForNavigation + remove POST only in http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
==========

to

==========
Simplify decidePolicyForNavigation + remove POST only in http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Łukasz Anforowicz, 2016-07-20 22:08:35]

Description was changed from

==========
Simplify decidePolicyForNavigation + remove POST only in http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Simplify decidePolicyForNavigation + remove POST-only-in-http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Łukasz Anforowicz, 2016-07-20 22:08:47]

Description was changed from

==========
Simplify decidePolicyForNavigation + remove POST-only-in-http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Simplify decidePolicyForNavigation + remove POST-only-in-http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Łukasz Anforowicz, 2016-07-20 22:23:03]

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

[Łukasz Anforowicz, 2016-07-20 22:23:04]

Charlie, can you take a look please?

https://codereview.chromium.org/2166113002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl.cc (left):

https://codereview.chromium.org/2166113002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl.cc:675: }
I think simply removing the DCHECK is the best option - it doesn't seem right to
try to enforce POST-only-in-http(s) condition only in NavigationConteller, when
other parts of the code apparently are already capable of handling POST to
non-http(s) URIs (we already know about tests that exercise this for file: and
chrome-extension://... URIs).

For completeness, below I try to list other (rejected) alternatives:
- Keep the check above, but replace NOTREACHED with code that changes the method
to GET and drops the POST body.  From test behavior it seems that this is what
//net code must be doing for file: URIs already, so not sure if there is a point
in duplicating this logic.
- Keep the assert here (and maybe even add this assert in other places) after
tweaking the callers to guarantee this condition.  This also seems to duplicate
the fallback-to-GET logic that apparently is already present in //net.

Also just for the record - before putting this CL out for review, I tried to
author a test that would trigger POST to chrome-extension://... URI. 
Unfortunately I failed - despite various attempts of setting
browser_handles_all_top_level_requests from
ExtensionWebRequestApiTest.PostData2, I couldn't make this setting stick into
the right renderer process.

https://codereview.chromium.org/2166113002/diff/1/content/renderer/render_fra...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2166113002/diff/1/content/renderer/render_fra...
content/renderer/render_frame_impl.cc:4991: should_fork =
!old_url.SchemeIs(url::kFileScheme);
The changes above [should] just replicate the changes that have been suggested
and implemented in patchset #2 of
https://codereview.chromium.org/2153573002/#msg4.

[Łukasz Anforowicz, 2016-07-20 22:23:57]

+tsepez@ to CC [the author of https://crrev.com/2153573002 that this CL is based
on]

[Charlie Reis, 2016-07-20 22:50:38]

Thanks for following up!  LGTM with one sanity check below.

https://codereview.chromium.org/2166113002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl.cc (left):

https://codereview.chromium.org/2166113002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl.cc:675: }
On 2016/07/20 22:23:04, Łukasz Anforowicz wrote:
> I think simply removing the DCHECK is the best option - it doesn't seem right
to
> try to enforce POST-only-in-http(s) condition only in NavigationConteller,
when
> other parts of the code apparently are already capable of handling POST to
> non-http(s) URIs (we already know about tests that exercise this for file: and
> chrome-extension://... URIs).
> 
> For completeness, below I try to list other (rejected) alternatives:
> - Keep the check above, but replace NOTREACHED with code that changes the
method
> to GET and drops the POST body.  From test behavior it seems that this is what
> //net code must be doing for file: URIs already, so not sure if there is a
point
> in duplicating this logic.
> - Keep the assert here (and maybe even add this assert in other places) after
> tweaking the callers to guarantee this condition.  This also seems to
duplicate
> the fallback-to-GET logic that apparently is already present in //net.
> 
> Also just for the record - before putting this CL out for review, I tried to
> author a test that would trigger POST to chrome-extension://... URI. 
> Unfortunately I failed - despite various attempts of setting
> browser_handles_all_top_level_requests from
> ExtensionWebRequestApiTest.PostData2, I couldn't make this setting stick into
> the right renderer process.

Thanks.  I think I agree, if the net stack is just going to convert it anyway. 
(Otherwise it's kind of meaningless to do a POST to a file URL.)

Do you know if we end up storing the POST data in the file:// NavigationEntry? 
Hopefully not.  (Should be easy to check on a file URL that submits to a new
window, or in the
fast/events/popup-allowed-from-gesture-initiated-form-submit.html test.)

[Łukasz Anforowicz, 2016-07-21 00:16:50]

On 2016/07/20 22:50:38, Charlie Reis wrote:
> Thanks for following up!  LGTM with one sanity check below.
> 
>
https://codereview.chromium.org/2166113002/diff/1/content/browser/frame_host/...
> File content/browser/frame_host/navigation_controller_impl.cc (left):
> 
>
https://codereview.chromium.org/2166113002/diff/1/content/browser/frame_host/...
> content/browser/frame_host/navigation_controller_impl.cc:675: }
> On 2016/07/20 22:23:04, Łukasz Anforowicz wrote:
> > I think simply removing the DCHECK is the best option - it doesn't seem
right
> to
> > try to enforce POST-only-in-http(s) condition only in NavigationConteller,
> when
> > other parts of the code apparently are already capable of handling POST to
> > non-http(s) URIs (we already know about tests that exercise this for file:
and
> > chrome-extension://... URIs).
> > 
> > For completeness, below I try to list other (rejected) alternatives:
> > - Keep the check above, but replace NOTREACHED with code that changes the
> method
> > to GET and drops the POST body.  From test behavior it seems that this is
what
> > //net code must be doing for file: URIs already, so not sure if there is a
> point
> > in duplicating this logic.
> > - Keep the assert here (and maybe even add this assert in other places)
after
> > tweaking the callers to guarantee this condition.  This also seems to
> duplicate
> > the fallback-to-GET logic that apparently is already present in //net.
> > 
> > Also just for the record - before putting this CL out for review, I tried to
> > author a test that would trigger POST to chrome-extension://... URI. 
> > Unfortunately I failed - despite various attempts of setting
> > browser_handles_all_top_level_requests from
> > ExtensionWebRequestApiTest.PostData2, I couldn't make this setting stick
into
> > the right renderer process.
> 
> Thanks.  I think I agree, if the net stack is just going to convert it anyway.

> (Otherwise it's kind of meaningless to do a POST to a file URL.)

"the net stack is just going to convert it anyway" is sort of tautologically
true - by "convert" I mean the final effect - just loading the file: URI and
ignoring the POST data (at least for the resource request;  I am not quite sure
if it gets stored in the session history - see below).  At least this is what I
see when manually trying out
fast/events/popup-allowed-from-gesture-initiated-form-submit.html - the target
of the form appears (and obviously the POST data couldn't have been POSTed
anywhere in case of a file: URI).

> Do you know if we end up storing the POST data in the file:// NavigationEntry?

> Hopefully not.  (Should be easy to check on a file URL that submits to a new
> window, or in the
> fast/events/popup-allowed-from-gesture-initiated-form-submit.html test.)

I am not sure what you mean here / how to check this.

NavigationEntryImpl::SetPostData is getting called with non-empty POST data
before the actual navigation happens:

NavigationEntryImpl::SetPostData;
url=file:///usr/local/google/home/lukasza/src/chromium4/src/third_party/WebKit/LayoutTests/fast/events/resources/popup-allowed-from-gesture-initiated-form-submit-target.html;
data=1; stack=#0 0x0000004c1ef1 __interceptor_backtrace
#1 0x7f253f1bacf3 base::debug::StackTrace::StackTrace()
#2 0x7f2540ff4d7a content::NavigationEntryImpl::SetPostData()
#3 0x7f2540fde3df content::NavigationControllerImpl::LoadURLWithParams()
#4 0x000000d47a2d content::Shell::OpenURLFromTab()
#5 0x7f254198d913 content::WebContentsImpl::RequestOpenURL()
#6 0x7f2541017579 content::NavigatorImpl::RequestOpenURL()
#7 0x7f25410426aa content::RenderFrameHostImpl::OpenURL()
#8 0x7f254102b242 content::RenderFrameHostImpl::OnOpenURL()

It gets reset once the page commits:

NavigationEntryImpl::SetPostData;
url=file:///usr/local/google/home/lukasza/src/chromium4/src/third_party/WebKit/LayoutTests/fast/events/resources/popup-allowed-from-gesture-initiated-form-submit-target.html;
data=0; stack=#0 0x0000004c1ef1 __interceptor_backtrace
#1 0x7f253f1bacf3 base::debug::StackTrace::StackTrace()
#2 0x7f2540ff4d7a content::NavigationEntryImpl::SetPostData()
#3 0x7f2540ff6f56 content::NavigationEntryImpl::ResetForCommit()
#4 0x7f2540fdfb16 content::NavigationControllerImpl::RendererDidNavigate()
#5 0x7f2541015877 content::NavigatorImpl::DidNavigate()
#6 0x7f2541029efe content::RenderFrameHostImpl::OnDidCommitProvisionalLoad()

OTOH, I see that once the page commits (i.e. in almost the same callstack as
above), the POST data is present inside PageState (I've added some ad-hoc code
to explode the page state in RendererDidNavigate and log it to stderr):

[10867:10867:0720/171309:203259260102:ERROR:navigation_controller_impl.cc(888)]
NavigationControllerImpl::RendererDidNavigate;
url=file:///usr/local/google/home/lukasza/src/chromium4/src/third_party/WebKit/LayoutTests/fast/events/resources/popup-allowed-from-gesture-initiated-form-submit-target.html;
post_data=1

[Charlie Reis, 2016-07-21 23:25:21]

Thanks, still LGTM.

https://codereview.chromium.org/2166113002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl.cc (left):

https://codereview.chromium.org/2166113002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl.cc:675: }
On 2016/07/21 00:16:50, Łukasz Anforowicz wrote:
> On 2016/07/20 22:50:38, Charlie Reis wrote:
> > Thanks for following up!  LGTM with one sanity check below.
> > 
> >
>
https://codereview.chromium.org/2166113002/diff/1/content/browser/frame_host/...
> > File content/browser/frame_host/navigation_controller_impl.cc (left):
> > 
> >
>
https://codereview.chromium.org/2166113002/diff/1/content/browser/frame_host/...
> > content/browser/frame_host/navigation_controller_impl.cc:675: }
> > On 2016/07/20 22:23:04, Łukasz Anforowicz wrote:
> > > I think simply removing the DCHECK is the best option - it doesn't seem
> right
> > to
> > > try to enforce POST-only-in-http(s) condition only in NavigationConteller,
> > when
> > > other parts of the code apparently are already capable of handling POST to
> > > non-http(s) URIs (we already know about tests that exercise this for file:
> and
> > > chrome-extension://... URIs).
> > > 
> > > For completeness, below I try to list other (rejected) alternatives:
> > > - Keep the check above, but replace NOTREACHED with code that changes the
> > method
> > > to GET and drops the POST body.  From test behavior it seems that this is
> what
> > > //net code must be doing for file: URIs already, so not sure if there is a
> > point
> > > in duplicating this logic.
> > > - Keep the assert here (and maybe even add this assert in other places)
> after
> > > tweaking the callers to guarantee this condition.  This also seems to
> > duplicate
> > > the fallback-to-GET logic that apparently is already present in //net.
> > > 
> > > Also just for the record - before putting this CL out for review, I tried
to
> > > author a test that would trigger POST to chrome-extension://... URI. 
> > > Unfortunately I failed - despite various attempts of setting
> > > browser_handles_all_top_level_requests from
> > > ExtensionWebRequestApiTest.PostData2, I couldn't make this setting stick
> into
> > > the right renderer process.
> > 
> > Thanks.  I think I agree, if the net stack is just going to convert it
anyway.
> 
> > (Otherwise it's kind of meaningless to do a POST to a file URL.)
> 
> "the net stack is just going to convert it anyway" is sort of tautologically
> true - by "convert" I mean the final effect - just loading the file: URI and
> ignoring the POST data (at least for the resource request;  I am not quite
sure
> if it gets stored in the session history - see below).  At least this is what
I
> see when manually trying out
> fast/events/popup-allowed-from-gesture-initiated-form-submit.html - the target
> of the form appears (and obviously the POST data couldn't have been POSTed
> anywhere in case of a file: URI).

Sure.

> 
> > Do you know if we end up storing the POST data in the file://
NavigationEntry?
> 
> > Hopefully not.  (Should be easy to check on a file URL that submits to a new
> > window, or in the
> > fast/events/popup-allowed-from-gesture-initiated-form-submit.html test.)
> 
> I am not sure what you mean here / how to check this.

Sorry, I meant after commit.

> 
> NavigationEntryImpl::SetPostData is getting called with non-empty POST data
> before the actual navigation happens:
> 
> NavigationEntryImpl::SetPostData;
>
url=file:///usr/local/google/home/lukasza/src/chromium4/src/third_party/WebKit/LayoutTests/fast/events/resources/popup-allowed-from-gesture-initiated-form-submit-target.html;
> data=1; stack=#0 0x0000004c1ef1 __interceptor_backtrace
> #1 0x7f253f1bacf3 base::debug::StackTrace::StackTrace()
> #2 0x7f2540ff4d7a content::NavigationEntryImpl::SetPostData()
> #3 0x7f2540fde3df content::NavigationControllerImpl::LoadURLWithParams()
> #4 0x000000d47a2d content::Shell::OpenURLFromTab()
> #5 0x7f254198d913 content::WebContentsImpl::RequestOpenURL()
> #6 0x7f2541017579 content::NavigatorImpl::RequestOpenURL()
> #7 0x7f25410426aa content::RenderFrameHostImpl::OpenURL()
> #8 0x7f254102b242 content::RenderFrameHostImpl::OnOpenURL()
> 
> It gets reset once the page commits:
> 
> NavigationEntryImpl::SetPostData;
>
url=file:///usr/local/google/home/lukasza/src/chromium4/src/third_party/WebKit/LayoutTests/fast/events/resources/popup-allowed-from-gesture-initiated-form-submit-target.html;
> data=0; stack=#0 0x0000004c1ef1 __interceptor_backtrace
> #1 0x7f253f1bacf3 base::debug::StackTrace::StackTrace()
> #2 0x7f2540ff4d7a content::NavigationEntryImpl::SetPostData()
> #3 0x7f2540ff6f56 content::NavigationEntryImpl::ResetForCommit()
> #4 0x7f2540fdfb16 content::NavigationControllerImpl::RendererDidNavigate()
> #5 0x7f2541015877 content::NavigatorImpl::DidNavigate()
> #6 0x7f2541029efe content::RenderFrameHostImpl::OnDidCommitProvisionalLoad()

Ah, yes, the post data isn't stored in GetPostData after commmit, apparently. 
As you mention below, we store it in PageState.

> 
> OTOH, I see that once the page commits (i.e. in almost the same callstack as
> above), the POST data is present inside PageState (I've added some ad-hoc code
> to explode the page state in RendererDidNavigate and log it to stderr):
> 
>
[10867:10867:0720/171309:203259260102:ERROR:navigation_controller_impl.cc(888)]
> NavigationControllerImpl::RendererDidNavigate;
>
url=file:///usr/local/google/home/lukasza/src/chromium4/src/third_party/WebKit/LayoutTests/fast/events/resources/popup-allowed-from-gesture-initiated-form-submit-target.html;
> post_data=1

Now that's interesting.  I suppose it's not a problem, but it means that we'll
probably prompt for form resubmission on reload (etc) despite being on a file://
URL where that's meaningless.

Anyway, I don't see that causing any particularly bad behavior, so I think it
should be safe to proceed.

[Łukasz Anforowicz, 2016-07-21 23:40:14]

The CQ bit was checked by lukasza@chromium.org

[commit-bot: I haz the power, 2016-07-21 23:40:39]

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, 2016-07-22 03:38:15]

Description was changed from

==========
Simplify decidePolicyForNavigation + remove POST-only-in-http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Simplify decidePolicyForNavigation + remove POST-only-in-http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-07-22 03:38:16]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-22 03:39:54]

Description was changed from

==========
Simplify decidePolicyForNavigation + remove POST-only-in-http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Simplify decidePolicyForNavigation + remove POST-only-in-http(s) DCHECK.

This CL simplifies RenderFrameImpl::decidePolicyForNavigation check for
navigating to file: URIs.  The simplification is okay, because if it's
safe to fork for file-to-file navigations when the opener isn't found,
then it should be also safe to do it even when there is an opener.
The simplification is desirable, because it removes code that is not
yet quite compatible with OOPIFs (where top()->document() may be null).

The simplification means that after the CL
fast/events/popup-allowed-from-gesture-initiated-form-submit.html
layout test (which POSTs a form to a file URI) goes through
NavigationControllerImpl::LoadURLWithParams.  This exposes the need
to remove an overagressive DCHECK that expected POST method only
for http (or https) URIs.  Since apparently other parts of the system
work gracefully when POSTing to a non-http URI, the DCHECK is being
removed.

BUG=101395, 466297
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/96db896a6696944f84bb31f1baea9b4bb88deb91
Cr-Commit-Position: refs/heads/master@{#407033}
==========

[commit-bot: I haz the power, 2016-07-22 03:39:56]

Patchset 1 (id:??) landed as
https://crrev.com/96db896a6696944f84bb31f1baea9b4bb88deb91
Cr-Commit-Position: refs/heads/master@{#407033}