Register external extension providers also for supervised users

Register external extension providers also for supervised users.

Not registering them broke voice search for supervised users, because the corresponding extension comes from one of these providers.
It also resulted in inconsistent behavior for child accounts, where the providers would exist after the first sign-in, but not after the next browser restart.

BUG=458096, 397951
Committed: https://crrev.com/cc82ab850cbe39794fdc59e244ec96a6da6479fc
Cr-Commit-Position: refs/heads/master@{#318670}

[Marc Treib, 2015-02-26 13:36:31]

treib@chromium.org changed reviewers:
+ bauerb@chromium.org, dpolukhin@chromium.org, pam@chromium.org

[Marc Treib, 2015-02-26 13:36:32]

+dpolukhin, who originally added the "not-for-supervised-users" check in
crrev.com/7da1221b85. What was the reason for that? Will this CL break anything?

Also +bauerb +pam, just in case one of you knows something about this.

(I'm aware none of you own this code. I'll add OWNERS after the initial sanity
check.)

[Dmitry Polukhin, 2015-02-26 13:42:46]

Local supervised user didn't have GAIA account so most of default apps doesn't
work for them so there was decision to don't install default apps. For new GAIA
supervised users with GAIA account we may want to reconsider it. It is very
strange that you see default apps after second login. It seems that account is
not marked appropriately on second login - it needs to be fixed.

[Marc Treib, 2015-02-26 13:55:09]

On 2015/02/26 13:42:46, Dmitry Polukhin wrote:
> Local supervised user didn't have GAIA account so most of default apps doesn't
> work for them so there was decision to don't install default apps. For new
GAIA
> supervised users with GAIA account we may want to reconsider it. It is very
> strange that you see default apps after second login. It seems that account is
> not marked appropriately on second login - it needs to be fixed.

For the new child accounts, we definitely want default apps to be installed. (We
might selectively blacklist individual ones, where we know they won't work, but
that should be few, if any.)
But even for old supervised users, not all of the default apps are
non-functional, e.g. voice search.

The issue with new supervised users/child accounts is that their "supervised"
state can change at runtime, so initialization code (like
CreateExternalProviders here) must not rely on Profile::IsSupervised.

I see two options:
1) Keep this CL as it is, and accept that old supervised users might get some
non-functional apps.
2) Replace the IsSupervised checks with IsLegacySupervised, and accept that
voice search won't work for old supervised users.
Either way, everything will be fine for child accounts.

[Dmitry Polukhin, 2015-02-26 14:36:52]

On 2015/02/26 13:55:09, Marc Treib wrote:
> On 2015/02/26 13:42:46, Dmitry Polukhin wrote:
> > Local supervised user didn't have GAIA account so most of default apps
doesn't
> > work for them so there was decision to don't install default apps. For new
> GAIA
> > supervised users with GAIA account we may want to reconsider it. It is very
> > strange that you see default apps after second login. It seems that account
is
> > not marked appropriately on second login - it needs to be fixed.
> 
> For the new child accounts, we definitely want default apps to be installed.
(We
> might selectively blacklist individual ones, where we know they won't work,
but
> that should be few, if any.)
> But even for old supervised users, not all of the default apps are
> non-functional, e.g. voice search.
> 
> The issue with new supervised users/child accounts is that their "supervised"
> state can change at runtime, so initialization code (like
> CreateExternalProviders here) must not rely on Profile::IsSupervised.
> 
> I see two options:
> 1) Keep this CL as it is, and accept that old supervised users might get some
> non-functional apps.

It is not some but almost all of them like GMail, Docs, etc. So I still don't
see value of installing these apps.


> 2) Replace the IsSupervised checks with IsLegacySupervised, and accept that
> voice search won't work for old supervised users.
> Either way, everything will be fine for child accounts.

I suspect voice search get installed via ExternalComponentLoader so it seems
that you need register it unconditionally.

[Marc Treib, 2015-02-26 14:50:18]

On 2015/02/26 14:36:52, Dmitry Polukhin wrote:
> On 2015/02/26 13:55:09, Marc Treib wrote:
> > On 2015/02/26 13:42:46, Dmitry Polukhin wrote:
> > > Local supervised user didn't have GAIA account so most of default apps
> doesn't
> > > work for them so there was decision to don't install default apps. For new
> > GAIA
> > > supervised users with GAIA account we may want to reconsider it. It is
very
> > > strange that you see default apps after second login. It seems that
account
> is
> > > not marked appropriately on second login - it needs to be fixed.
> > 
> > For the new child accounts, we definitely want default apps to be installed.
> (We
> > might selectively blacklist individual ones, where we know they won't work,
> but
> > that should be few, if any.)
> > But even for old supervised users, not all of the default apps are
> > non-functional, e.g. voice search.
> > 
> > The issue with new supervised users/child accounts is that their
"supervised"
> > state can change at runtime, so initialization code (like
> > CreateExternalProviders here) must not rely on Profile::IsSupervised.
> > 
> > I see two options:
> > 1) Keep this CL as it is, and accept that old supervised users might get
some
> > non-functional apps.
> 
> It is not some but almost all of them like GMail, Docs, etc. So I still don't
> see value of installing these apps.
> 
> 
> > 2) Replace the IsSupervised checks with IsLegacySupervised, and accept that
> > voice search won't work for old supervised users.
> > Either way, everything will be fine for child accounts.
> 
> I suspect voice search get installed via ExternalComponentLoader so it seems
> that you need register it unconditionally.

Correct.
I've updated the CL to only register the ExternalComponentLoader
unconditionally. The other providers are now registered for regular and child
users, but not legacy supervised users.

[Dmitry Polukhin, 2015-02-26 14:51:03]

lgtm

[Marc Treib, 2015-02-26 15:00:11]

treib@chromium.org changed reviewers:
+ asargent@chromium.org

[Marc Treib, 2015-02-26 15:00:12]

+asargent: Can I get an OWNERS review, please? Thanks!

[asargent_no_longer_on_chrome, 2015-02-28 00:03:44]

lgtm

[Marc Treib, 2015-02-28 17:20:20]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2015-02-28 17:21:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/955683002/20001

[commit-bot: I haz the power, 2015-02-28 17:25:59]

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

[commit-bot: I haz the power, 2015-02-28 17:26:00]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Marc Treib, 2015-03-01 11:26:34]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2015-03-01 11:27:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/955683002/20001

[commit-bot: I haz the power, 2015-03-01 11:32:04]

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

[commit-bot: I haz the power, 2015-03-01 11:32:05]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Marc Treib, 2015-03-02 08:50:00]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2015-03-02 08:50:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/955683002/20001

[commit-bot: I haz the power, 2015-03-02 09:41:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-02 09:42:41]

Patchset 2 (id:??) landed as
https://crrev.com/cc82ab850cbe39794fdc59e244ec96a6da6479fc
Cr-Commit-Position: refs/heads/master@{#318670}