Allow about:blank to be considered in-page if origin matches.

Allow about:blank to be considered in-page if origin matches.

This is necessary in cases that document.write changes the effective
URL of the about:blank page, and then the user goes back.

BUG=446959
TEST=See bug for repro steps.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

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

[Charlie Reis, 2016-06-03 23:23:30]

Description was changed from

==========
Allow about:blank to be considered in-page if origin matches.

This is necessary in cases that document.write changes the effective
URL of the about:blank page, and then the user goes back.

BUG=446959
TEST=See bug for repro steps.
==========

to

==========
Allow about:blank to be considered in-page if origin matches.

This is necessary in cases that document.write changes the effective
URL of the about:blank page, and then the user goes back.

BUG=446959
TEST=See bug for repro steps.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[Charlie Reis, 2016-06-06 19:30:53]

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

[Charlie Reis, 2016-06-06 19:30:54]

Avi, can you take a look?  This should fix the other known repro steps for the
NC_IN_PAGE_NAVIGATION renderer kill in default Chrome.

(I was having trouble with the test, but that turned out to be due to the lack
of document.close() after document.write().  Discussion in
https://crbug.com/617706.)

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

https://codereview.chromium.org/2032903007/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl.cc:1413:
last_committed_url.GetOrigin() == url.GetOrigin() ||
I'm eager to get rid of this line (since GURL::GetOrigin doesn't work for things
like about:blank that might have inherited an origin), but I don't want to make
this check any stricter in a CL that we're hoping to merge to beta.

Specifically, it's currently needed for the data URL vs about:blank case, both
of which look like unique origins.  That returns false on the line below, but
true on this line.  I have some plans to clean it up in the next (non-mergeable)
CL, but that's riskier since it might be more strict.

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

https://codereview.chromium.org/2032903007/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_unittest.cc:3779:
EXPECT_TRUE(controller.IsURLInPageNavigation(url, url::Origin(url), true,
These unit test changes are just mechanical: passing in the origin corresponding
to the URL.  I'll clean up the about:blank test here as I'm updating those
checks.

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

https://codereview.chromium.org/2032903007/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigator_impl.cc:478: bool is_navigation_within_page
= controller_->IsURLInPageNavigation(
I also plan to make this the only call site of IsURLInPageNavigation, since we
call it redundantly below in RendererDidNavigate.  Again, not in a CL that we
hope to merge.

[Avi (use Gerrit), 2016-06-06 19:46:41]

lgtm

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

https://codereview.chromium.org/2032903007/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl.cc:1413:
last_committed_url.GetOrigin() == url.GetOrigin() ||
On 2016/06/06 19:30:54, Charlie Reis wrote:
> I'm eager to get rid of this line (since GURL::GetOrigin doesn't work for
things
> like about:blank that might have inherited an origin), but I don't want to
make
> this check any stricter in a CL that we're hoping to merge to beta.
> 
> Specifically, it's currently needed for the data URL vs about:blank case, both
> of which look like unique origins.  That returns false on the line below, but
> true on this line.  I have some plans to clean it up in the next
(non-mergeable)
> CL, but that's riskier since it might be more strict.

Acknowledged.

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

https://codereview.chromium.org/2032903007/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_controller_impl_unittest.cc:3779:
EXPECT_TRUE(controller.IsURLInPageNavigation(url, url::Origin(url), true,
On 2016/06/06 19:30:54, Charlie Reis wrote:
> These unit test changes are just mechanical: passing in the origin
corresponding
> to the URL.  I'll clean up the about:blank test here as I'm updating those
> checks.

Acknowledged.

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

https://codereview.chromium.org/2032903007/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigator_impl.cc:478: bool is_navigation_within_page
= controller_->IsURLInPageNavigation(
On 2016/06/06 19:30:54, Charlie Reis wrote:
> I also plan to make this the only call site of IsURLInPageNavigation, since we
> call it redundantly below in RendererDidNavigate.  Again, not in a CL that we
> hope to merge.

Acknowledged.

[Charlie Reis, 2016-06-06 20:03:04]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2016-06-06 20:04:06]

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

[commit-bot: I haz the power, 2016-06-07 00:17:38]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-07 00:20:43]

Description was changed from

==========
Allow about:blank to be considered in-page if origin matches.

This is necessary in cases that document.write changes the effective
URL of the about:blank page, and then the user goes back.

BUG=446959
TEST=See bug for repro steps.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Allow about:blank to be considered in-page if origin matches.

This is necessary in cases that document.write changes the effective
URL of the about:blank page, and then the user goes back.

BUG=446959
TEST=See bug for repro steps.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

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

[commit-bot: I haz the power, 2016-06-07 00:20:45]

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