Ensure the entire page is secure for PWAs.

Ensure the entire page is secure for PWAs.

This CL changes app banners and WebAPKs to use SecurityTabHelper to
check the entire page's security level, rather than just the top level
origin. This makes the secure origin requirement more rigorous, and
prevents sites with mixed content warnings from erroneously being
permitted to display banners and install WebAPKs.

Localhost is whitelisted to ensure local machine development is not
blocked.

BUG=657739
Review-Url: https://codereview.chromium.org/2630523002
Cr-Commit-Position: refs/heads/master@{#443979}
Committed: https://chromium.googlesource.com/chromium/src/+/24f1eede17d4d2515fb0fb0d168f8a35cef9a851

[dominickn, 2017-01-12 07:18:36]

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

[commit-bot: I haz the power, 2017-01-12 07:18:46]

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

[dominickn, 2017-01-12 07:19:58]

Description was changed from

==========
Ensure the entire page is secure for PWAs.

This CL changes app banners and WebAPKs to use SecurityTabHelper to
check the entire page's security level, rather than just the top level
origin. This makes the secure origin requirement more rigourous, and
prevents sites with mixed content warnings from erroneously being
permitted to display banners and install WebAPKs.

BUG=657739
==========

to

==========
Ensure the entire page is secure for PWAs.

This CL changes app banners and WebAPKs to use SecurityTabHelper to
check the entire page's security level, rather than just the top level
origin. This makes the secure origin requirement more rigourous, and
prevents sites with mixed content warnings from erroneously being
permitted to display banners and install WebAPKs.

BUG=657739
==========

[dominickn, 2017-01-12 07:19:59]

dominickn@chromium.org changed reviewers:
+ benwells@chromium.org, pkotwicz@chromium.org

[dominickn, 2017-01-12 07:43:26]

dominickn@chromium.org changed reviewers:
- benwells@chromium.org, pkotwicz@chromium.org

[commit-bot: I haz the power, 2017-01-12 07:53:35]

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

[commit-bot: I haz the power, 2017-01-12 07:53:36]

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

[dominickn, 2017-01-12 07:54:57]

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

[commit-bot: I haz the power, 2017-01-12 07:55:05]

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

[dominickn, 2017-01-12 07:55:40]

Description was changed from

==========
Ensure the entire page is secure for PWAs.

This CL changes app banners and WebAPKs to use SecurityTabHelper to
check the entire page's security level, rather than just the top level
origin. This makes the secure origin requirement more rigourous, and
prevents sites with mixed content warnings from erroneously being
permitted to display banners and install WebAPKs.

BUG=657739
==========

to

==========
Ensure the entire page is secure for PWAs.

This CL changes app banners and WebAPKs to use SecurityTabHelper to
check the entire page's security level, rather than just the top level
origin. This makes the secure origin requirement more rigourous, and
prevents sites with mixed content warnings from erroneously being
permitted to display banners and install WebAPKs.

Localhost is whitelisted to ensure local machine development is not
blocked.

BUG=657739
==========

[dominickn, 2017-01-12 07:55:41]

dominickn@chromium.org changed reviewers:
+ benwells@chromium.org, pkotwicz@chromium.org

[dominickn, 2017-01-12 07:55:50]

Description was changed from

==========
Ensure the entire page is secure for PWAs.

This CL changes app banners and WebAPKs to use SecurityTabHelper to
check the entire page's security level, rather than just the top level
origin. This makes the secure origin requirement more rigourous, and
prevents sites with mixed content warnings from erroneously being
permitted to display banners and install WebAPKs.

Localhost is whitelisted to ensure local machine development is not
blocked.

BUG=657739
==========

to

==========
Ensure the entire page is secure for PWAs.

This CL changes app banners and WebAPKs to use SecurityTabHelper to
check the entire page's security level, rather than just the top level
origin. This makes the secure origin requirement more rigorous, and
prevents sites with mixed content warnings from erroneously being
permitted to display banners and install WebAPKs.

Localhost is whitelisted to ensure local machine development is not
blocked.

BUG=657739
==========

[commit-bot: I haz the power, 2017-01-12 08:38:49]

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

[commit-bot: I haz the power, 2017-01-12 08:38:50]

Dry run: This issue passed the CQ dry run.

[dominickn, 2017-01-12 08:44:26]

PTAL, thanks!

pkotwicz: WebAPK sanity check
benwells: OWNERS

[benwells, 2017-01-13 04:34:13]

lgtm

https://codereview.chromium.org/2630523002/diff/20001/chrome/browser/installa...
File chrome/browser/installable/installable_manager.cc (right):

https://codereview.chromium.org/2630523002/diff/20001/chrome/browser/installa...
chrome/browser/installable/installable_manager.cc:107: if
(net::IsLocalhost(web_contents->GetVisibleURL().HostNoBrackets()))
Hmm, it's a bit unfortunate you need to manually check this. Seems like
SecurityInfo should mark this SECURE automatically, or something.

[dominickn, 2017-01-13 04:38:54]

Thanks!

https://codereview.chromium.org/2630523002/diff/20001/chrome/browser/installa...
File chrome/browser/installable/installable_manager.cc (right):

https://codereview.chromium.org/2630523002/diff/20001/chrome/browser/installa...
chrome/browser/installable/installable_manager.cc:107: if
(net::IsLocalhost(web_contents->GetVisibleURL().HostNoBrackets()))
On 2017/01/13 04:34:13, benwells wrote:
> Hmm, it's a bit unfortunate you need to manually check this. Seems like
> SecurityInfo should mark this SECURE automatically, or something.

Yeah, that's what I expected too. Then it failed all of the tests (which use
localhost).

[dominickn, 2017-01-16 21:21:13]

pkotwicz: ping.

[pkotwicz, 2017-01-17 02:45:52]

LGTM

Sorry for the delay. I did not realize that I was a reviewer =P The CL is
reasonable from a WebAPK point of view.

[dominickn, 2017-01-17 02:50:09]

On 2017/01/17 02:45:52, pkotwicz wrote:
> LGTM
> 
> Sorry for the delay. I did not realize that I was a reviewer =P The CL is
> reasonable from a WebAPK point of view.

Thanks!

[dominickn, 2017-01-17 02:50:25]

The CQ bit was checked by dominickn@chromium.org

[commit-bot: I haz the power, 2017-01-17 02:50:37]

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-01-17 03:37:43]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484621425351380,
"parent_rev": "667766119057d345c5e79d0769870bc6e3dfb519", "commit_rev":
"24f1eede17d4d2515fb0fb0d168f8a35cef9a851"}

[commit-bot: I haz the power, 2017-01-17 03:38:16]

Description was changed from

==========
Ensure the entire page is secure for PWAs.

This CL changes app banners and WebAPKs to use SecurityTabHelper to
check the entire page's security level, rather than just the top level
origin. This makes the secure origin requirement more rigorous, and
prevents sites with mixed content warnings from erroneously being
permitted to display banners and install WebAPKs.

Localhost is whitelisted to ensure local machine development is not
blocked.

BUG=657739
==========

to

==========
Ensure the entire page is secure for PWAs.

This CL changes app banners and WebAPKs to use SecurityTabHelper to
check the entire page's security level, rather than just the top level
origin. This makes the secure origin requirement more rigorous, and
prevents sites with mixed content warnings from erroneously being
permitted to display banners and install WebAPKs.

Localhost is whitelisted to ensure local machine development is not
blocked.

BUG=657739

Review-Url: https://codereview.chromium.org/2630523002
Cr-Commit-Position: refs/heads/master@{#443979}
Committed:
https://chromium.googlesource.com/chromium/src/+/24f1eede17d4d2515fb0fb0d168f...
==========

[commit-bot: I haz the power, 2017-01-17 03:38:19]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/24f1eede17d4d2515fb0fb0d168f...