App install alerts: Disable on all channels for M35

App install alerts: Disable on all channels for M35

Disables app install alerts with no way to turn them back on through flags.

BUG=366169
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=265716

[gone, 2014-04-08 22:39:52]

Server-side portion of this is pending approval, but I can at least get this
part rolling.

[Nico, 2014-04-08 22:55:23]

I think for finch-controlled things one usually has an enable, a disable, and a
default (== what finch says) on about:flags (?)

https://chromiumcodereview.appspot.com/229533002/diff/1/chrome/browser/androi...
File chrome/browser/android/banners/app_banner_manager.cc (left):

https://chromiumcodereview.appspot.com/229533002/diff/1/chrome/browser/androi...
chrome/browser/android/banners/app_banner_manager.cc:27: namespace {
(const has implicit internal linkage, this unnamed namespace has no effect)

[Ted C, 2014-04-08 23:03:48]

lgtm -- for androidy things

[gone, 2014-04-08 23:22:20]

This is an odd experiment in that the flag in about:flags is for developers to
force the feature to be on for the Stable channel (the proposed Finch experiment
will start off with 0% of Stable users having it on) -- I changed the wording of
the flag accordingly.  AFAICT there are no plans to allow manually disabling the
alerts...

https://chromiumcodereview.appspot.com/229533002/diff/1/chrome/browser/androi...
File chrome/browser/android/banners/app_banner_manager.cc (left):

https://chromiumcodereview.appspot.com/229533002/diff/1/chrome/browser/androi...
chrome/browser/android/banners/app_banner_manager.cc:27: namespace {
On 2014/04/08 22:55:24, Nico wrote:
> (const has implicit internal linkage, this unnamed namespace has no effect)

Done.

[gone, 2014-04-21 18:00:45]

Repurposing this since people changed their minds again.

Patch set title should say that I'm disabling the feature on Stable, but not the
other channels.

[Ted C, 2014-04-21 18:02:47]

On 2014/04/21 18:00:45, dfalcantara wrote:
> Repurposing this since people changed their minds again.
> 
> Patch set title should say that I'm disabling the feature on Stable, but not
the
> other channels.

lgtm

[Nico, 2014-04-21 23:16:12]

We don't like to have channel checks checked in as far as I know. Can't you just
merge a "return false" into m35?

[Ted C, 2014-04-21 23:23:52]

On 2014/04/21 23:16:12, Nico wrote:
> We don't like to have channel checks checked in as far as I know. Can't you
just
> merge a "return false" into m35?

We want to leave it on in 35 beta and not stable (and 36 stable, etc...), so
developers could have the chance to play with it.  We haven't been able to do
much end-to-end testing (as we have external dependencies out of our control),
so this was the safe compromise we agreed upon today.

[Nico, 2014-04-22 00:25:24]

If we don't want to ship this in the foreseeable future, why keep the code
around?

On Desktop, we don't want to have things on or off depending on channel since
testing progresses canary -> dev -> beta -> stable, and this only helps the
stability of stable if all these channels actually test the same thing. For
chrome/android, this pipeline is shorter (just beta->stable), but it feels like
the same argument would still apply.

[Ted C, 2014-04-22 00:39:05]

On 2014/04/22 00:25:24, Nico wrote:
> If we don't want to ship this in the foreseeable future, why keep the code
> around?
> 
> On Desktop, we don't want to have things on or off depending on channel since
> testing progresses canary -> dev -> beta -> stable, and this only helps the
> stability of stable if all these channels actually test the same thing. For
> chrome/android, this pipeline is shorter (just beta->stable), but it feels
like
> the same argument would still apply.

We want to be able to ship this in 35.  The forces outside of our control might
be resolved and we can then begin developer outreach as this is a long pull
feature.  This reduces that potential turn around by 6 weeks, which was deemed
important enough to do it this way.

This is certainly an anomaly, but the forces at be believe it is worth doing.

[Nico, 2014-04-22 22:02:52]

On 2014/04/22 00:39:05, Ted C wrote:
> On 2014/04/22 00:25:24, Nico wrote:
> > If we don't want to ship this in the foreseeable future, why keep the code
> > around?
> > 
> > On Desktop, we don't want to have things on or off depending on channel
since
> > testing progresses canary -> dev -> beta -> stable, and this only helps the
> > stability of stable if all these channels actually test the same thing. For
> > chrome/android, this pipeline is shorter (just beta->stable), but it feels
> like
> > the same argument would still apply.
> 
> We want to be able to ship this in 35.  The forces outside of our control
might
> be resolved and we can then begin developer outreach as this is a long pull
> feature.  This reduces that potential turn around by 6 weeks, which was deemed
> important enough to do it this way.

Isn't an about:flags entry good enough for developer outreach? I think that's
what all the other developer outreach efforts (nacl, web platform, etc) do.

> This is certainly an anomaly, but the forces at be believe it is worth doing.

The forces that be are us :-)

[gone, 2014-04-23 17:35:15]

Decision was made to disable the feature for M35, without any way of turning it
back on.  As far as I can tell, this decision is finally final.  Absolutely for
serious, no take-backsies.

[Nico, 2014-04-23 17:41:21]

lgtm, but I don't see what's wrong with having this behind a flag for devs
*shrug*

[gone, 2014-04-23 17:43:15]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2014-04-23 17:43:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dfalcantara@chromium.org/229533002/60001

[commit-bot: I haz the power, 2014-04-23 18:36:28]

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

[commit-bot: I haz the power, 2014-04-23 18:36:28]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel

[gone, 2014-04-23 18:41:42]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2014-04-23 18:42:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dfalcantara@chromium.org/229533002/60001

[commit-bot: I haz the power, 2014-04-23 19:54:57]

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

[commit-bot: I haz the power, 2014-04-23 19:54:57]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel

[gone, 2014-04-23 19:56:13]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2014-04-23 19:56:37]

Submitting NOTRY=true because win_chromium_rel is drunk.

[commit-bot: I haz the power, 2014-04-23 19:57:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dfalcantara@chromium.org/229533002/60001

[commit-bot: I haz the power, 2014-04-23 20:01:56]

Change committed as 265716