Treat loads of invalid URLs as no-ops unless triggered by window.open.

Treat loads of invalid URLs as no-ops unless triggered by window.open.

The HTML5 spec indicates that a load of an invalid URL via links or the
window.location API should be a no-op.  However, window.open calls
with an invalid URL should still create a new window, but pointed to
about:blank.

BUG=709059
Review-Url: https://codereview.chromium.org/2799953002
Cr-Commit-Position: refs/heads/master@{#462700}
Committed: https://chromium.googlesource.com/chromium/src/+/45b26c61d1eec665cd55bc17d6c0799b092d66e7

[kkhorimoto, 2017-04-06 18:07:12]

kkhorimoto@chromium.org changed reviewers:
+ eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2017-04-06 18:20:13]

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

[commit-bot: I haz the power, 2017-04-06 18:21:22]

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

[kkhorimoto, 2017-04-06 18:29:00]

Description was changed from

==========
Treat loads of invalid URLs as no-ops unless triggered by window.open.

The HTML5 spec indicates that a load of an invalid URL via links or the
window.location API.  However, window.open calls with an invalid URL
should still create a new window, but pointed to about:blank.

BUG=709059
==========

to

==========
Treat loads of invalid URLs as no-ops unless triggered by window.open.

The HTML5 spec indicates that a load of an invalid URL via links or the
window.location API should be a no-op.  However, window.open calls
with an invalid URL should still create a new window, but pointed to
about:blank.

BUG=709059
==========

[Eugene But (OOO till 7-30), 2017-04-06 18:34:19]

lgtm

https://codereview.chromium.org/2799953002/diff/1/ios/testing/data/http_serve...
File ios/testing/data/http_server_files/window_location.js (right):

https://codereview.chromium.org/2799953002/diff/1/ios/testing/data/http_serve...
ios/testing/data/http_server_files/window_location.js:45: buttonWasTapped();
|locationAssign| and other methods are named in generic way w/o implying button
tapping. How about renaming |buttonWasTapped| to updateNoOpTextAfterTimeout ?
Alternatively we could rename locationAssign -> locationAssignButtonTapped

https://codereview.chromium.org/2799953002/diff/1/ios/web/web_state/ui/crw_we...
File ios/web/web_state/ui/crw_web_controller.mm (right):

https://codereview.chromium.org/2799953002/diff/1/ios/web/web_state/ui/crw_we...
ios/web/web_state/ui/crw_web_controller.mm:4317: if (isFirstLoadInOpenedWindow
&& isMainFrame) {
I thought this will break CRWWebControllerInvalidUrlTest.LoadInvalidURL, but it
does not. Do you know why?

[commit-bot: I haz the power, 2017-04-06 18:42:14]

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

[commit-bot: I haz the power, 2017-04-06 18:42:15]

Dry run: This issue passed the CQ dry run.

[kkhorimoto, 2017-04-07 00:03:41]

https://codereview.chromium.org/2799953002/diff/1/ios/testing/data/http_serve...
File ios/testing/data/http_server_files/window_location.js (right):

https://codereview.chromium.org/2799953002/diff/1/ios/testing/data/http_serve...
ios/testing/data/http_server_files/window_location.js:45: buttonWasTapped();
On 2017/04/06 18:34:19, Eugene But wrote:
> |locationAssign| and other methods are named in generic way w/o implying
button
> tapping. How about renaming |buttonWasTapped| to updateNoOpTextAfterTimeout ?
> Alternatively we could rename locationAssign -> locationAssignButtonTapped

Done.

https://codereview.chromium.org/2799953002/diff/1/ios/web/web_state/ui/crw_we...
File ios/web/web_state/ui/crw_web_controller.mm (right):

https://codereview.chromium.org/2799953002/diff/1/ios/web/web_state/ui/crw_we...
ios/web/web_state/ui/crw_web_controller.mm:4317: if (isFirstLoadInOpenedWindow
&& isMainFrame) {
On 2017/04/06 18:34:19, Eugene But wrote:
> I thought this will break CRWWebControllerInvalidUrlTest.LoadInvalidURL, but
it
> does not. Do you know why?

It looks like WKWebView's |-loadHTMLString:baseURL:| rewrites invalid URLs to
about:blank before reaching this delegate callback.  I've created a bug and
added a TODO to revisit what we want to do with that test.  I'm not sure if we
depend on that behavior for that specific WKWebView API.

[kkhorimoto, 2017-04-07 00:03:47]

The CQ bit was checked by kkhorimoto@chromium.org

[commit-bot: I haz the power, 2017-04-07 00:04:43]

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-04-07 00:13:28]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1491523427881460, "parent_rev":
"a7ea3b49dfc918cb632611272418dcf23d465b1e", "commit_rev":
"45b26c61d1eec665cd55bc17d6c0799b092d66e7"}

[commit-bot: I haz the power, 2017-04-07 00:14:46]

Description was changed from

==========
Treat loads of invalid URLs as no-ops unless triggered by window.open.

The HTML5 spec indicates that a load of an invalid URL via links or the
window.location API should be a no-op.  However, window.open calls
with an invalid URL should still create a new window, but pointed to
about:blank.

BUG=709059
==========

to

==========
Treat loads of invalid URLs as no-ops unless triggered by window.open.

The HTML5 spec indicates that a load of an invalid URL via links or the
window.location API should be a no-op.  However, window.open calls
with an invalid URL should still create a new window, but pointed to
about:blank.

BUG=709059

Review-Url: https://codereview.chromium.org/2799953002
Cr-Commit-Position: refs/heads/master@{#462700}
Committed:
https://chromium.googlesource.com/chromium/src/+/45b26c61d1eec665cd55bc17d6c0...
==========

[commit-bot: I haz the power, 2017-04-07 00:14:47]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/45b26c61d1eec665cd55bc17d6c0...

[Eugene But (OOO till 7-30), 2017-04-07 01:03:36]

https://codereview.chromium.org/2799953002/diff/1/ios/web/web_state/ui/crw_we...
File ios/web/web_state/ui/crw_web_controller.mm (right):

https://codereview.chromium.org/2799953002/diff/1/ios/web/web_state/ui/crw_we...
ios/web/web_state/ui/crw_web_controller.mm:4317: if (isFirstLoadInOpenedWindow
&& isMainFrame) {
On 2017/04/07 00:03:40, kkhorimoto_ wrote:
> On 2017/04/06 18:34:19, Eugene But wrote:
> > I thought this will break CRWWebControllerInvalidUrlTest.LoadInvalidURL, but
> it
> > does not. Do you know why?
> 
> It looks like WKWebView's |-loadHTMLString:baseURL:| rewrites invalid URLs to
> about:blank before reaching this delegate callback.  I've created a bug and
> added a TODO to revisit what we want to do with that test.  I'm not sure if we
> depend on that behavior for that specific WKWebView API.
Sorry looks like that test was useless :( I will delete it.