Remove AppBannerManager code to skip secure context checks for tests.

Remove AppBannerManager code to skip secure context checks for tests.

localhost is always considered secure.

BUG=none

Committed: https://crrev.com/3ab83d44b0f52114c2e6344ba7ab516f967ed8a8
Cr-Commit-Position: refs/heads/master@{#428235}

[dcheng, 2016-10-19 05:51:32]

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

[commit-bot: I haz the power, 2016-10-19 05:52:32]

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, 2016-10-19 12:08:05]

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

[commit-bot: I haz the power, 2016-10-19 12:08:06]

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

[dcheng, 2016-10-20 18:02:12]

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

[commit-bot: I haz the power, 2016-10-20 18:02:41]

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, 2016-10-20 21:20:17]

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

[commit-bot: I haz the power, 2016-10-20 21:20:18]

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

[dcheng, 2016-10-24 19:20:28]

Description was changed from

==========
Remove AppBannerManager code to skip secure context checks for tests.

BUG=none
==========

to

==========
Remove AppBannerManager code to skip secure context checks for tests.

localhost is always considered secure.

BUG=none
==========

[dcheng, 2016-10-24 19:20:28]

dcheng@chromium.org changed reviewers:
+ benwells@chromium.org

[dcheng, 2016-10-24 19:30:28]

I'm not actually 100% sure how to test this. I just reimaged my machine, I don't
have a test Android device atm, and the trybots don't actually appaer to be
running the tests in question. Any thoughts?

[benwells, 2016-10-25 03:00:51]

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

[benwells, 2016-10-25 03:00:51]

punting to Dom to review...

[dominickn, 2016-10-25 03:13:39]

The android bots should be running the test in chrome_public_test_apk (see
https://chromium-swarm.appspot.com/user/task/31fc228f0d0de110)?

https://codereview.chromium.org/2434623002/diff/20001/chrome/browser/banners/...
File chrome/browser/banners/app_banner_manager.cc (right):

https://codereview.chromium.org/2434623002/diff/20001/chrome/browser/banners/...
chrome/browser/banners/app_banner_manager.cc:33: bool
gDisableSecureCheckForTesting = false;
If you're removing the method, you should remove this global too.

https://codereview.chromium.org/2434623002/diff/20001/chrome/browser/banners/...
chrome/browser/banners/app_banner_manager.cc:117: LOG(ERROR) << validated_url;
Remove this log spam.

https://codereview.chromium.org/2434623002/diff/20001/chrome/browser/banners/...
chrome/browser/banners/app_banner_manager.cc:119:
!gDisableSecureCheckForTesting) {
Remove the check for the global here. It's useless if you remove the static
setter.

[dcheng, 2016-10-25 17:04:30]

On 2016/10/25 03:13:39, dominickn wrote:
> The android bots should be running the test in chrome_public_test_apk (see
> https://chromium-swarm.appspot.com/user/task/31fc228f0d0de110)
> 
>
https://codereview.chromium.org/2434623002/diff/20001/chrome/browser/banners/...
> File chrome/browser/banners/app_banner_manager.cc (right):
> 
>
https://codereview.chromium.org/2434623002/diff/20001/chrome/browser/banners/...
> chrome/browser/banners/app_banner_manager.cc:33: bool
> gDisableSecureCheckForTesting = false;
> If you're removing the method, you should remove this global too.
> 
>
https://codereview.chromium.org/2434623002/diff/20001/chrome/browser/banners/...
> chrome/browser/banners/app_banner_manager.cc:117: LOG(ERROR) << validated_url;
> Remove this log spam.
> 
>
https://codereview.chromium.org/2434623002/diff/20001/chrome/browser/banners/...
> chrome/browser/banners/app_banner_manager.cc:119:
> !gDisableSecureCheckForTesting) {
> Remove the check for the global here. It's useless if you remove the static
> setter.

Yeah, there's some noise in this CL because I was hoping to verify that my
assumptions were correct on the trybots. But if you're OK with proceeding as-is,
I'll go ahead and just prepare it for landing.

[dcheng, 2016-10-27 22:28:19]

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

[commit-bot: I haz the power, 2016-10-27 22:29:11]

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, 2016-10-27 23:42:50]

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

[commit-bot: I haz the power, 2016-10-27 23:42:51]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-10-28 00:37:21]

PTAL. I'm skeptical that the trybots are running the test at all, but if the
waterfall explodes, it's easy enough to revert (and we'll have a nice postmortem
about trybots not running enough tests)

[dominickn, 2016-10-28 00:47:49]

lgtm. Thanks for the clean up. :)

I patched the CL in on my machine and everything passes on my M Nexus 5 test
device.

[dcheng, 2016-10-28 00:49:43]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-10-28 00:50:04]

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, 2016-10-28 01:39:59]

Description was changed from

==========
Remove AppBannerManager code to skip secure context checks for tests.

localhost is always considered secure.

BUG=none
==========

to

==========
Remove AppBannerManager code to skip secure context checks for tests.

localhost is always considered secure.

BUG=none
==========

[commit-bot: I haz the power, 2016-10-28 01:40:00]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-28 01:43:27]

Description was changed from

==========
Remove AppBannerManager code to skip secure context checks for tests.

localhost is always considered secure.

BUG=none
==========

to

==========
Remove AppBannerManager code to skip secure context checks for tests.

localhost is always considered secure.

BUG=none

Committed: https://crrev.com/3ab83d44b0f52114c2e6344ba7ab516f967ed8a8
Cr-Commit-Position: refs/heads/master@{#428235}
==========

[commit-bot: I haz the power, 2016-10-28 01:43:28]

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