Add a warning for the deprecation of content-initiated data URL navigations

Add a warning for the deprecation of content-initiated data URL navigations

This CL adds a console warning when a page navigates the top level frame to a 
data URL.

The browser tests are added to WebContentsImpl tests to be consistent with
the view-source URL tests.

This CL also updates most of the layout tests to avoid loading data URLs at
the top level. The only exceptions are xss-DENIED-* tests which will be updated
when the actual blocking happens.

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

Review-Url: https://codereview.chromium.org/2694903007
Cr-Commit-Position: refs/heads/master@{#455226}
Committed: https://chromium.googlesource.com/chromium/src/+/b29954e8ac243142eaedbcc3c6e54d480373ee38

[meacer, 2017-02-15 22:17:30]

Description was changed from

==========
Add a warning for the deprecation of content-initiated data URL navigations

BUG=594215
==========

to

==========
Add a warning for the deprecation of content-initiated data URL navigations

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

[meacer, 2017-02-15 22:17:58]

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

[commit-bot: I haz the power, 2017-02-15 22:18:59]

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

[meacer, 2017-02-15 22:19:28]

Description was changed from

==========
Add a warning for the deprecation of content-initiated data URL navigations

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

to

==========
Add a warning for the deprecation of content-initiated data URL navigations

The test cases are added to WebContentsImpl browser tests to be consistent
with the view-source URL tests.

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

[meacer, 2017-02-15 22:21:12]

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

[meacer, 2017-02-15 22:21:13]

Charlie, can you PTAL? Thanks.

[commit-bot: I haz the power, 2017-02-15 22:37:36]

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

[commit-bot: I haz the power, 2017-02-15 22:37:36]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Charlie Reis, 2017-02-15 23:36:43]

[cc nasko, clamy]

Good idea to add a deprecation warning, and glad we're still moving towards
this.  A few thoughts and questions below.

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

https://codereview.chromium.org/2694903007/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_handle_impl.cc:157: if
(url_.SchemeIs(url::kDataScheme) && IsInMainFrame() &&
Why is this done from the NavigationHandleImpl destructor?  The test failures
seem to indicate that it's not safe to call GetRenderFrameHost() from there.

Maybe it would be better to do this somewhere related to the navigation commit
instead, like DidCommitNavigation?

https://codereview.chromium.org/2694903007/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_handle_impl.cc:162: "to data: URLs. For
more information, see https://crbug.com/594215.");
This bug link doesn't seem like great documentation for people to reference. 
There's not much discussion of the reasoning or alternatives there, and it has a
link to a Google internal design doc (http://go/crbug-pseudo-urls).  Is there a
better link we can provide to developers?  Or maybe update the description of
that bug to provide a better overview?

https://codereview.chromium.org/2694903007/diff/1/content/browser/web_content...
File content/browser/web_contents/web_contents_impl_browsertest.cc (right):

https://codereview.chromium.org/2694903007/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl_browsertest.cc:891: // Test that
window.open to a data URL shows a console warning.
Just curious-- have you checked whether any of our existing tests do
content-initiated main frame navigations to data URLs?  I'm curious if they'll
break when we get around to blocking it.

[meacer, 2017-02-16 01:02:32]

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

[commit-bot: I haz the power, 2017-02-16 01:04:18]

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

[meacer, 2017-02-16 01:58:43]

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

https://codereview.chromium.org/2694903007/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_handle_impl.cc:157: if
(url_.SchemeIs(url::kDataScheme) && IsInMainFrame() &&
On 2017/02/15 23:36:43, Charlie Reis wrote:
> Why is this done from the NavigationHandleImpl destructor?  The test failures
> seem to indicate that it's not safe to call GetRenderFrameHost() from there.
> 
> Maybe it would be better to do this somewhere related to the navigation commit
> instead, like DidCommitNavigation?

Done, moved to DidCommitNavigation.

https://codereview.chromium.org/2694903007/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_handle_impl.cc:162: "to data: URLs. For
more information, see https://crbug.com/594215.");
On 2017/02/15 23:36:43, Charlie Reis wrote:
> This bug link doesn't seem like great documentation for people to reference. 
> There's not much discussion of the reasoning or alternatives there, and it has
a
> link to a Google internal design doc (http://go/crbug-pseudo-urls).  Is there
a
> better link we can provide to developers?  Or maybe update the description of
> that bug to provide a better overview?

Do you think blink intent to deprecate thread would be more useful? I can add a
goo.gl version of
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/GbVcuwg_QjM

I think it has the most complete context about this, and also has links to
tracking bugs and the feature dashboard.

https://codereview.chromium.org/2694903007/diff/1/content/browser/web_content...
File content/browser/web_contents/web_contents_impl_browsertest.cc (right):

https://codereview.chromium.org/2694903007/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl_browsertest.cc:891: // Test that
window.open to a data URL shows a console warning.
On 2017/02/15 23:36:43, Charlie Reis wrote:
> Just curious-- have you checked whether any of our existing tests do
> content-initiated main frame navigations to data URLs?  I'm curious if they'll
> break when we get around to blocking it.

Yes, there are tests that use data URLs. Some examples:

- Android popups
https://cs.chromium.org/chromium/src/chrome/test/data/android/popup.html?q=%2...

- Desktop popups
https://cs.chromium.org/chromium/src/chrome/test/data/popup_blocker/popup-man...

- unload browsertests 
https://cs.chromium.org/chromium/src/chrome/browser/unload_browsertest.cc?q=%...

- click modifier (not sure what this is)
https://cs.chromium.org/chromium/src/chrome/test/data/click_modifier/window_o...

and more.

There are also many more tests that use ui_test_utils::NavigateToURL but those
shouldn't be affected since NavigateToURL initiates a direct navigation.

[commit-bot: I haz the power, 2017-02-16 02:51:39]

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

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

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

[Charlie Reis, 2017-02-16 17:59:50]

Thanks, LGTM once the link is updated.

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

https://codereview.chromium.org/2694903007/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_handle_impl.cc:162: "to data: URLs. For
more information, see https://crbug.com/594215.");
On 2017/02/16 01:58:42, Mustafa Emre Acer wrote:
> On 2017/02/15 23:36:43, Charlie Reis wrote:
> > This bug link doesn't seem like great documentation for people to reference.

> > There's not much discussion of the reasoning or alternatives there, and it
has
> a
> > link to a Google internal design doc (http://go/crbug-pseudo-urls).  Is
there
> a
> > better link we can provide to developers?  Or maybe update the description
of
> > that bug to provide a better overview?
> 
> Do you think blink intent to deprecate thread would be more useful? I can add
a
> goo.gl version of
> https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/GbVcuwg_QjM
> 
> I think it has the most complete context about this, and also has links to
> tracking bugs and the feature dashboard.

Yes, a link to that intent would be perfect.  Thanks!

https://codereview.chromium.org/2694903007/diff/1/content/browser/web_content...
File content/browser/web_contents/web_contents_impl_browsertest.cc (right):

https://codereview.chromium.org/2694903007/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl_browsertest.cc:891: // Test that
window.open to a data URL shows a console warning.
On 2017/02/16 01:58:43, Mustafa Emre Acer wrote:
> On 2017/02/15 23:36:43, Charlie Reis wrote:
> > Just curious-- have you checked whether any of our existing tests do
> > content-initiated main frame navigations to data URLs?  I'm curious if
they'll
> > break when we get around to blocking it.
> 
> Yes, there are tests that use data URLs. Some examples:
> 
> - Android popups
>
https://cs.chromium.org/chromium/src/chrome/test/data/android/popup.html?q=%2...
> 
> - Desktop popups
>
https://cs.chromium.org/chromium/src/chrome/test/data/popup_blocker/popup-man...
> 
> - unload browsertests 
>
https://cs.chromium.org/chromium/src/chrome/browser/unload_browsertest.cc?q=%...
> 
> - click modifier (not sure what this is)
>
https://cs.chromium.org/chromium/src/chrome/test/data/click_modifier/window_o...
> 
> and more.
> 
> There are also many more tests that use ui_test_utils::NavigateToURL but those
> shouldn't be affected since NavigateToURL initiates a direct navigation.

Ah, ok.  So we'll have to update the ones that have a content-initiated
navigation when the time comes.  (I suppose this warning could theoretically
affect some layout tests which pick up the warning in the output-- you might
want to run some try jobs that run layout tests, like win10_blink_rel.)

[meacer, 2017-02-16 22:19:42]

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

[commit-bot: I haz the power, 2017-02-16 22:20:30]

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-16 23:51:49]

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

[commit-bot: I haz the power, 2017-02-16 23:51:50]

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

[meacer, 2017-02-17 21:13:44]

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

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

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

[meacer, 2017-02-17 22:47:04]

Thanks Charlie. Changed the URL and updated the layout tests.
crrev.com/2702503002 is an incomplete CL for full blockage with try jobs. It
should give an idea of the expected test breakage (hint: it's large :).

[commit-bot: I haz the power, 2017-02-17 23:16:55]

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

[commit-bot: I haz the power, 2017-02-17 23:16:56]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[meacer, 2017-02-18 00:03:22]

The CQ bit was checked by meacer@chromium.org

[meacer, 2017-02-18 00:03:23]

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/2694903007/#ps60001
(title: "Fix layout tests")

[commit-bot: I haz the power, 2017-02-18 00:05:23]

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

[Charlie Reis, 2017-02-18 00:20:33]

On 2017/02/17 22:47:04, Mustafa Emre Acer wrote:
> Thanks Charlie. Changed the URL and updated the layout tests.

Wow, that's a lot of tests.  I'm wondering if it's better to update these tests
now (to avoid loading data URLs in the top frame) rather than putting
deprecation messages into all the results.  I suppose that's just delaying this
CL without much benefit, though.  Still, you might want to update those tests
sooner than the CL that does the actual blocking.  :)

> crrev.com/2702503002 is an incomplete CL for full blockage with try jobs. It
> should give an idea of the expected test breakage (hint: it's large :).

I realize that CL is incomplete, but I'm wondering why it's doing the blocking
in the renderer process.  That seems less trustworthy and unlikely to work with
PlzNavigate.  Maybe it allows more graceful error handling?  Either way, I'd
suggest having a backup check in the browser process when the time comes.

[meacer, 2017-02-18 00:28:18]

On 2017/02/18 00:20:33, Charlie Reis wrote:
> On 2017/02/17 22:47:04, Mustafa Emre Acer wrote:
> > Thanks Charlie. Changed the URL and updated the layout tests.
> 
> Wow, that's a lot of tests.  I'm wondering if it's better to update these
tests
> now (to avoid loading data URLs in the top frame) rather than putting
> deprecation messages into all the results.  I suppose that's just delaying
this
> CL without much benefit, though.  Still, you might want to update those tests
> sooner than the CL that does the actual blocking.  :)

Yes, I think updating the tests in this CL rather than trying to fix them will
be better and faster since this is potentially going to be merged to M57.

I'll start look into fixing the tests shortly.

> 
> > crrev.com/2702503002 is an incomplete CL for full blockage with try jobs. It
> > should give an idea of the expected test breakage (hint: it's large :).
> 
> I realize that CL is incomplete, but I'm wondering why it's doing the blocking
> in the renderer process.  That seems less trustworthy and unlikely to work
with
> PlzNavigate.  Maybe it allows more graceful error handling?  Either way, I'd
> suggest having a backup check in the browser process when the time comes.

I wanted to follow the SecurityOrigin::canDisplay checks, but the renderer side
implementation still needs to refer to plugins for mime type checks so it might
indeed not
be the best place to do this. In the meanwhile, I wrote this doc to explore
alternatives and
get your opinion:
https://docs.google.com/document/d/1UnFi4Idz1wkpIopnBb2TKkYYIJXM5zrjQYq3CQE5A...

[commit-bot: I haz the power, 2017-02-18 02:09:09]

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

[commit-bot: I haz the power, 2017-02-18 02:09:10]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[meacer, 2017-02-21 18:44:49]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2017-02-21 18:45:50]

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-21 20:48:11]

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

[commit-bot: I haz the power, 2017-02-21 20:48:13]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[meacer, 2017-02-21 22:22:03]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2017-02-21 22:23:20]

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-22 00:27:48]

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

[commit-bot: I haz the power, 2017-02-22 00:27:49]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[meacer, 2017-02-22 21:35:07]

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

[commit-bot: I haz the power, 2017-02-22 21:35:47]

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-22 23:21:03]

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

[commit-bot: I haz the power, 2017-02-22 23:21:05]

Dry run: This issue passed the CQ dry run.

[meacer, 2017-02-22 23:44:53]

Charlie: I updated all the layout tests that I could find to not use data URLs
(except for xss-DENIED-from-javascript-url-window-open.html which I'll tackle
later).

Can you please take another look? It's a large number of files, but most of them
have the same pattern so hopefully this won't consume too much time :) Thanks!

[Charlie Reis, 2017-02-23 18:39:10]

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

[Charlie Reis, 2017-02-23 18:39:11]

[+japhet]

Whew, that's a lot to update!  Generally looks good, though, with one concern
and a few questions.

Nate, you don't need to review the whole thing, but can you help sanity check
that this is ok in general?  We're changing layout tests to avoid navigating to
data URLs in the top level, since that will soon be disallowed.

Mustafa, please also update the CL description to say that the layout tests are
being updated to avoid loading data URLs at the top level.

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/dom/id-attribute-shared.html (right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/id-attribute-shared.html:46:
document.location = 'Window/resources/destination.html';
Convenient that was already there.  :)

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/files/null-origin-string.html (right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/files/null-origin-string.html:18:
console.log('PASS if no crash.');
This isn't an equivalent test.  Not sure if it mattered at the time, but I can
easily imagine a bug where navigating away from a page with a file input caused
a crash (e.g., when unloading the page).  Can you replace this with something
like notify-success.html?

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/forms/resources/multiple-form-submission-protection-post-target.html
(right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/resources/multiple-form-submission-protection-post-target.html:3:
opener.postMessage('trololo', '*'); window.close();
nit: Might as well split this into two lines, like the rest.

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/forms/state-restore-to-non-autocomplete-form.html
(left):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/state-restore-to-non-autocomplete-form.html:12:
<form action="data:text/html,<script>history.back()&lt;/script>" id=form1
autocomplete=off>
Wow, &lt; can't possibly be right there.  Glad it's going away.  :)

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/loader/form-state-restore-with-frames.html
(left):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/loader/form-state-restore-with-frames.html:11:
<form id="form1" action="data:text/html,&lt;script>history.back();&lt;/script>">
How did this ever work??  Once again, glad it's gone.

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/loader/reload-zero-byte-plugin.html
(right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/loader/reload-zero-byte-plugin.html:1:
<iframe src="data:application/x-blink-test-plugin," id="f"></iframe>
Interesting.  I guess this is close enough that it should be testing the same
thing.

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/dataURL/xss-DENIED-from-javascript-url-window-open-expected.txt
(right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/dataURL/xss-DENIED-from-javascript-url-window-open-expected.txt:1:
CONSOLE WARNING: Upcoming versions will block content-initiated top frame
navigations to data: URLs. For more information, see https://goo.gl/BaZAea.
What's the plan for this test when data URLs are blocked?

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/dataURL/xss-DENIED-to-data-url-window-open-expected.txt
(right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/dataURL/xss-DENIED-to-data-url-window-open-expected.txt:1:
CONSOLE WARNING: Upcoming versions will block content-initiated top frame
navigations to data: URLs. For more information, see https://goo.gl/BaZAea.
Same question here.

[meacer, 2017-02-23 20:45:26]

Description was changed from

==========
Add a warning for the deprecation of content-initiated data URL navigations

The test cases are added to WebContentsImpl browser tests to be consistent
with the view-source URL tests.

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

to

==========
Add a warning for the deprecation of content-initiated data URL navigations

This CL adds a console warning when a page navigates the top level frame to a 
data URL.

The browser tests are added to WebContentsImpl tests to be consistent with
the view-source URL tests.

This CL also updates most of the layout tests to avoid loading data URLs at
the top level. The only exceptions are xss-DENIED-* tests which will be updated
when the actual blocking happens.

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

[meacer, 2017-02-23 20:47:25]

Thanks Charlie, updated the CL description.

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/files/null-origin-string.html (right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/files/null-origin-string.html:18:
console.log('PASS if no crash.');
On 2017/02/23 18:39:11, Charlie Reis wrote:
> This isn't an equivalent test.  Not sure if it mattered at the time, but I can
> easily imagine a bug where navigating away from a page with a file input
caused
> a crash (e.g., when unloading the page).  Can you replace this with something
> like notify-success.html?

Done. This was a bit frustrating as I can't seem to navigate the top frame
(and also found https://crbug.com/695583 while testing), so I'm now passing a
message to the top frame to navigate itself.

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/forms/resources/multiple-form-submission-protection-post-target.html
(right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/resources/multiple-form-submission-protection-post-target.html:3:
opener.postMessage('trololo', '*'); window.close();
On 2017/02/23 18:39:11, Charlie Reis wrote:
> nit: Might as well split this into two lines, like the rest.

Done.

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/forms/state-restore-to-non-autocomplete-form.html
(left):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/state-restore-to-non-autocomplete-form.html:12:
<form action="data:text/html,<script>history.back()&lt;/script>" id=form1
autocomplete=off>
On 2017/02/23 18:39:11, Charlie Reis wrote:
> Wow, &lt; can't possibly be right there.  Glad it's going away.  :)

&lt; seems to work on both Chrome and Firefox :)

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/loader/reload-zero-byte-plugin.html
(right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/loader/reload-zero-byte-plugin.html:1:
<iframe src="data:application/x-blink-test-plugin," id="f"></iframe>
On 2017/02/23 18:39:13, Charlie Reis wrote:
> Interesting.  I guess this is close enough that it should be testing the same
> thing.

I was slightly concerned too, but the plugin prints a string and the test
outputs are still the same so it seemed okay to me to do this.

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/dataURL/xss-DENIED-from-javascript-url-window-open-expected.txt
(right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/dataURL/xss-DENIED-from-javascript-url-window-open-expected.txt:1:
CONSOLE WARNING: Upcoming versions will block content-initiated top frame
navigations to data: URLs. For more information, see https://goo.gl/BaZAea.
On 2017/02/23 18:39:13, Charlie Reis wrote:
> What's the plan for this test when data URLs are blocked?

I think this will be best tested by adding a new browsertest for this scenario,
and the converting this layout test to use an iframe.

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/dataURL/xss-DENIED-to-data-url-window-open-expected.txt
(right):

https://codereview.chromium.org/2694903007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/dataURL/xss-DENIED-to-data-url-window-open-expected.txt:1:
CONSOLE WARNING: Upcoming versions will block content-initiated top frame
navigations to data: URLs. For more information, see https://goo.gl/BaZAea.
On 2017/02/23 18:39:13, Charlie Reis wrote:
> Same question here.

Same as the other one, a combination of browser test + layout test using iframe.

[meacer, 2017-02-28 23:56:34]

japhet: Ping as I'm planning to merge this to M57 :)

[Nate Chapin, 2017-03-06 23:54:23]

Skimmed, seems reasonable. LGTM

[meacer, 2017-03-06 23:57:30]

On 2017/03/06 23:54:23, Nate Chapin wrote:
> Skimmed, seems reasonable. LGTM

Thanks!

[meacer, 2017-03-06 23:57:37]

The CQ bit was checked by meacer@chromium.org

[meacer, 2017-03-06 23:57:38]

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/2694903007/#ps100001
(title: "creis comments")

[commit-bot: I haz the power, 2017-03-06 23:58:14]

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-03-07 02:42:56]

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

[commit-bot: I haz the power, 2017-03-07 02:42:57]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[meacer, 2017-03-07 02:54:41]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2017-03-07 02:55:27]

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-03-07 05:18:31]

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

[commit-bot: I haz the power, 2017-03-07 05:18:33]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[meacer, 2017-03-07 18:40:21]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2017-03-07 18:41:05]

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-03-07 21:22:00]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1488912021903000,
"parent_rev": "0c08ed56a3e5089b3cc4094e83daae196a6300c4", "commit_rev":
"b29954e8ac243142eaedbcc3c6e54d480373ee38"}

[commit-bot: I haz the power, 2017-03-07 21:22:41]

Description was changed from

==========
Add a warning for the deprecation of content-initiated data URL navigations

This CL adds a console warning when a page navigates the top level frame to a 
data URL.

The browser tests are added to WebContentsImpl tests to be consistent with
the view-source URL tests.

This CL also updates most of the layout tests to avoid loading data URLs at
the top level. The only exceptions are xss-DENIED-* tests which will be updated
when the actual blocking happens.

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

to

==========
Add a warning for the deprecation of content-initiated data URL navigations

This CL adds a console warning when a page navigates the top level frame to a 
data URL.

The browser tests are added to WebContentsImpl tests to be consistent with
the view-source URL tests.

This CL also updates most of the layout tests to avoid loading data URLs at
the top level. The only exceptions are xss-DENIED-* tests which will be updated
when the actual blocking happens.

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

Review-Url: https://codereview.chromium.org/2694903007
Cr-Commit-Position: refs/heads/master@{#455226}
Committed:
https://chromium.googlesource.com/chromium/src/+/b29954e8ac243142eaedbcc3c6e5...
==========

[commit-bot: I haz the power, 2017-03-07 21:22:43]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/b29954e8ac243142eaedbcc3c6e5...