Allow data URL navigations for Android WebView until PlzNavigate ships

Allow data URL navigations for Android WebView until PlzNavigate ships

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

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

[meacer, 2017-06-29 22:47:07]

Description was changed from

==========
Allow data URL navigations for Android WebView until PlzNavigate ships

BUG=732976
==========

to

==========
Allow data URL navigations for Android WebView until PlzNavigate ships

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

[meacer, 2017-06-29 22:54:57]

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

[commit-bot: I haz the power, 2017-06-29 22:55:20]

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-06-29 23:20:44]

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

[commit-bot: I haz the power, 2017-06-29 23:20:45]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[meacer, 2017-07-06 22:28:23]

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

[commit-bot: I haz the power, 2017-07-06 22:28:42]

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-07-06 22:36:10]

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

[commit-bot: I haz the power, 2017-07-06 22:36:11]

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

[meacer, 2017-07-06 23:23:34]

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

[commit-bot: I haz the power, 2017-07-06 23:23:49]

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-07-06 23:54:43]

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

[commit-bot: I haz the power, 2017-07-06 23:54:44]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[meacer, 2017-07-07 18:23:55]

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

[commit-bot: I haz the power, 2017-07-07 18:24:19]

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-07-07 18:56:24]

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

[commit-bot: I haz the power, 2017-07-07 18:56:25]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[meacer, 2017-07-07 19:44:09]

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

[commit-bot: I haz the power, 2017-07-07 19:44:34]

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-07-07 22:09:06]

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

[commit-bot: I haz the power, 2017-07-07 22:09:07]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[meacer, 2017-07-10 18:05:41]

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

[commit-bot: I haz the power, 2017-07-10 18:06:01]

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-07-10 19:25:42]

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

[commit-bot: I haz the power, 2017-07-10 19:25:43]

Dry run: This issue passed the CQ dry run.

[meacer, 2017-07-10 23:52:48]

meacer@chromium.org changed reviewers:
+ clamy@chromium.org, sgurun@chromium.org

[meacer, 2017-07-10 23:52:50]

clamy, sgurun: Can you PTAL? Thanks.

[sgurun-gerrit only, 2017-07-11 00:38:51]

aw lgtm

thanks for the investigation and hard work here!

[clamy, 2017-07-11 12:11:06]

Thanks! content/ lgtm with a few comments.

https://codereview.chromium.org/2969473003/diff/60001/content/browser/frame_h...
File content/browser/frame_host/data_url_navigation_throttle.cc (right):

https://codereview.chromium.org/2969473003/diff/60001/content/browser/frame_h...
content/browser/frame_host/data_url_navigation_throttle.cc:39: if
(navigation_handle()
Maybe you could add a !IsBrowserSideNavigationEnabled() to the if-clause. This
way the data url blocking would still be enabled with PlzNavigate, where we
don't have the issue.

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

https://codereview.chromium.org/2969473003/diff/60001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:176: #if
defined(OS_ANDROID)
nit: for consistency, it'd be nicer if either the two Android WebView globals
were if defed or none were.

[meacer, 2017-07-11 22:59:08]

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

[commit-bot: I haz the power, 2017-07-11 22:59:50]

Dry run: CQ is trying da patch.

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

[meacer, 2017-07-11 23:00:05]

Thanks!

https://codereview.chromium.org/2969473003/diff/60001/content/browser/frame_h...
File content/browser/frame_host/data_url_navigation_throttle.cc (right):

https://codereview.chromium.org/2969473003/diff/60001/content/browser/frame_h...
content/browser/frame_host/data_url_navigation_throttle.cc:39: if
(navigation_handle()
On 2017/07/11 12:11:06, clamy wrote:
> Maybe you could add a !IsBrowserSideNavigationEnabled() to the if-clause. This
> way the data url blocking would still be enabled with PlzNavigate, where we
> don't have the issue.

Good point, done.

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

https://codereview.chromium.org/2969473003/diff/60001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:176: #if
defined(OS_ANDROID)
On 2017/07/11 12:11:06, clamy wrote:
> nit: for consistency, it'd be nicer if either the two Android WebView globals
> were if defed or none were.

Done.

[commit-bot: I haz the power, 2017-07-12 02:44:56]

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

[commit-bot: I haz the power, 2017-07-12 02:44:57]

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

[meacer, 2017-07-12 18:38:51]

The CQ bit was checked by meacer@chromium.org

[meacer, 2017-07-12 18:38:51]

The patchset sent to the CQ was uploaded after l-g-t-m from sgurun@chromium.org,
clamy@chromium.org
Link to the patchset: https://codereview.chromium.org/2969473003/#ps80001
(title: "clamy comments")

[commit-bot: I haz the power, 2017-07-12 18:39:11]

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-07-12 20:11:12]

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

[commit-bot: I haz the power, 2017-07-12 20:11:13]

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

[meacer, 2017-07-12 20:25:14]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2017-07-12 20:25:38]

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-07-12 21:44:46]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1499891114782420,
"parent_rev": "b980b4b5d4408bea7e83cbf1f70e7143edb41716", "commit_rev":
"e23f989bb00a9207fd3dc67cc27f725c32b2be77"}

[commit-bot: I haz the power, 2017-07-12 21:45:00]

Description was changed from

==========
Allow data URL navigations for Android WebView until PlzNavigate ships

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

to

==========
Allow data URL navigations for Android WebView until PlzNavigate ships

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

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

[commit-bot: I haz the power, 2017-07-12 21:45:01]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/e23f989bb00a9207fd3dc67cc27f...