Introduce kiosk.autolaunched feature session type

Introduce kiosk.autolaunched feature session type

The session type will be used to restrict extension features to
auto-launched kiosk sessions - kiosk session that are launched
automatically from the login screen, without user interaction.

To be able properly est auto launched flag, browser process
feature session type initialization is moved to
AppLaunchController, class which controls kiosk app launch.

Also, move browser process feature session type initialization
from ChromeUserManager::UpdateLoginState to
UserSessionManager::PerformPostUserLoggedInActions (for regular
users), which seems like a more suitable place.

BUG=676213
Review-Url: https://codereview.chromium.org/2601803003
Cr-Commit-Position: refs/heads/master@{#442667}
Committed: https://chromium.googlesource.com/chromium/src/+/5babfd313562ebd60a9a7910912a764bed6d63c0

[tbarzic, 2016-12-27 22:19:19]

Description was changed from

==========
Introduce kiosk.autolaunched feature session type

The session type will be used to restrict extension features to
auto-launched kiosk sessions - kiosk session that are launched
automatically from the login screen, without user interaction.


BUG=676213
==========

to

==========
Introduce kiosk.autolaunched feature session type

The session type will be used to restrict extension features to
auto-launched kiosk sessions - kiosk session that are launched
automatically from the login screen, without user interaction.

To be able properly est auto launched flag, browser process
feature session type initialization is moved to
AppLaunchController, class which controls kiosk app launch.

Also, move browser process feature session type initialization
from ChromeUserManager::UpdateLoginState to
UserSessionManager::PerformPostUserLoggedInActions (for regular
users), which seems like a more suitable place.

BUG=676213
==========

[tbarzic, 2016-12-27 23:14:32]

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

[commit-bot: I haz the power, 2016-12-27 23:14:50]

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-12-27 23:30:42]

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

[commit-bot: I haz the power, 2016-12-27 23:30:43]

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

[tbarzic, 2016-12-28 23:36:58]

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

[commit-bot: I haz the power, 2016-12-28 23:37:17]

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-12-29 00:40:59]

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

[commit-bot: I haz the power, 2016-12-29 00:40:59]

Dry run: This issue passed the CQ dry run.

[tbarzic, 2016-12-29 00:46:08]

tbarzic@chromium.org changed reviewers:
+ alemate@chromium.org, rdevlin.cronin@chromium.org

[tbarzic, 2016-12-29 00:46:08]

alemate - for chrome/browser/chromeos/login pieces
rdevlin.cronin - for extensions pieces

[Devlin, 2016-12-29 17:15:37]

This largely looks okay, but I'm curious about the long-term vision here.  Is
there a canonical doc with all the proposed changes, permission states, session
types, etc?  I'm worried that if we keep adding them as we think of them, we'll
end up with more than we want and it will be hard to keep them straight.

https://codereview.chromium.org/2601803003/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/session/user_session_manager.h (right):

https://codereview.chromium.org/2601803003/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/login/session/user_session_manager.h:342: // feature
system accordign to |user|'s type.
typo: according

https://codereview.chromium.org/2601803003/diff/80001/chrome/common/extension...
File chrome/common/extensions/api/_features.md (right):

https://codereview.chromium.org/2601803003/diff/80001/chrome/common/extension...
chrome/common/extensions/api/_features.md:317: `kiosk` session is a session
launched for a kiosk app - app that runs on its
nit: insert "an"
... kiosk app - an app that runs...

https://codereview.chromium.org/2601803003/diff/80001/chrome/common/extension...
chrome/common/extensions/api/_features.md:320: `kiosk.autolaunched` represents
auto-launched kiosk session - kiosk session that
nit: insert "a"
... kiosk session - a kiosk session...

https://codereview.chromium.org/2601803003/diff/80001/extensions/common/featu...
File extensions/common/features/simple_feature.cc (right):

https://codereview.chromium.org/2601803003/diff/80001/extensions/common/featu...
extensions/common/features/simple_feature.cc:507: return session_type ==
FeatureSessionType::AUTOLAUNCHED_KIOSK &&
I kind of wonder if this is better done at compile time, by inferring from
kiosk.autolaunched that we should also add kiosk, and then we just keep the
simple base::ContainsValue check here.  WDYT?

[tbarzic, 2016-12-29 20:29:58]

Long-term vision for what? New feature properties? I don't expect to be adding
any new ones, but I can't be sure that there won't be a need for new ones at
some point.

I don't expect to add new kiosk related session types either - there might be
new session types in general (e.g. public_session), but they would be defined by
existing user session types (i.e. types for existing session types would be
added to FeatureSessionType enum as they are needed).

https://codereview.chromium.org/2601803003/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/session/user_session_manager.h (right):

https://codereview.chromium.org/2601803003/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/login/session/user_session_manager.h:342: // feature
system accordign to |user|'s type.
On 2016/12/29 17:15:37, Devlin wrote:
> typo: according

Done.

https://codereview.chromium.org/2601803003/diff/80001/chrome/common/extension...
File chrome/common/extensions/api/_features.md (right):

https://codereview.chromium.org/2601803003/diff/80001/chrome/common/extension...
chrome/common/extensions/api/_features.md:317: `kiosk` session is a session
launched for a kiosk app - app that runs on its
On 2016/12/29 17:15:37, Devlin wrote:
> nit: insert "an"
> ... kiosk app - an app that runs...

Done.

https://codereview.chromium.org/2601803003/diff/80001/chrome/common/extension...
chrome/common/extensions/api/_features.md:320: `kiosk.autolaunched` represents
auto-launched kiosk session - kiosk session that
On 2016/12/29 17:15:37, Devlin wrote:
> nit: insert "a"
> ... kiosk session - a kiosk session...

Done.

https://codereview.chromium.org/2601803003/diff/80001/extensions/common/featu...
File extensions/common/features/simple_feature.cc (right):

https://codereview.chromium.org/2601803003/diff/80001/extensions/common/featu...
extensions/common/features/simple_feature.cc:507: return session_type ==
FeatureSessionType::AUTOLAUNCHED_KIOSK &&
On 2016/12/29 17:15:37, Devlin wrote:
> I kind of wonder if this is better done at compile time, by inferring from
> kiosk.autolaunched that we should also add kiosk, and then we just keep the
> simple base::ContainsValue check here.  WDYT?

Yeah, that would work,too.
I find this approach slightly easier to follow, in sense that it's easier to
figure out the logic by looking it up in simple_feature.cc than having to go to
feature_compiler in order to notice KIOSK actually maps to 2 types.

[tbarzic, 2017-01-03 19:13:14]

tbarzic@chromium.org changed reviewers:
+ xiyuan@chromium.org
- alemate@chromium.org

[tbarzic, 2017-01-03 19:13:14]

s/alemate/xiyuan for review of login parts

[xiyuan, 2017-01-03 21:16:12]

login parts lgtm

[achuithb, 2017-01-04 23:18:32]

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

[commit-bot: I haz the power, 2017-01-04 23:19:04]

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

[achuithb, 2017-01-04 23:25:31]

Also lgtm

https://codereview.chromium.org/2601803003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/2601803003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.cc:586: // kiosk app
profile would alredy be initialized - feature session type should
already

https://codereview.chromium.org/2601803003/diff/100001/chrome/common/extensio...
File chrome/common/extensions/api/_features.md (right):

https://codereview.chromium.org/2601803003/diff/100001/chrome/common/extensio...
chrome/common/extensions/api/_features.md:323: `kiosk.autolanched` session.
autolaunched

https://codereview.chromium.org/2601803003/diff/100001/extensions/common/feat...
File extensions/common/features/simple_feature.cc (right):

https://codereview.chromium.org/2601803003/diff/100001/extensions/common/feat...
extensions/common/features/simple_feature.cc:264: FeatureSessionType
session_type = GetCurrentFeatureSessionType();
nit: const

https://codereview.chromium.org/2601803003/diff/100001/extensions/common/feat...
extensions/common/features/simple_feature.cc:306: FeatureSessionType
session_type = GetCurrentFeatureSessionType();
nit: const

https://codereview.chromium.org/2601803003/diff/100001/extensions/common/feat...
extensions/common/features/simple_feature.cc:505: // kiosk session if kiosk
session is allowed. This is only exception to
the only exception

[tbarzic, 2017-01-04 23:39:48]

https://codereview.chromium.org/2601803003/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/2601803003/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/session/user_session_manager.cc:586: // kiosk app
profile would alredy be initialized - feature session type should
On 2017/01/04 23:25:30, achuithb wrote:
> already

Done.

https://codereview.chromium.org/2601803003/diff/100001/chrome/common/extensio...
File chrome/common/extensions/api/_features.md (right):

https://codereview.chromium.org/2601803003/diff/100001/chrome/common/extensio...
chrome/common/extensions/api/_features.md:323: `kiosk.autolanched` session.
On 2017/01/04 23:25:30, achuithb wrote:
> autolaunched

Done.

https://codereview.chromium.org/2601803003/diff/100001/extensions/common/feat...
File extensions/common/features/simple_feature.cc (right):

https://codereview.chromium.org/2601803003/diff/100001/extensions/common/feat...
extensions/common/features/simple_feature.cc:264: FeatureSessionType
session_type = GetCurrentFeatureSessionType();
On 2017/01/04 23:25:30, achuithb wrote:
> nit: const

Done.

https://codereview.chromium.org/2601803003/diff/100001/extensions/common/feat...
extensions/common/features/simple_feature.cc:306: FeatureSessionType
session_type = GetCurrentFeatureSessionType();
On 2017/01/04 23:25:30, achuithb wrote:
> nit: const

Done.

https://codereview.chromium.org/2601803003/diff/100001/extensions/common/feat...
extensions/common/features/simple_feature.cc:505: // kiosk session if kiosk
session is allowed. This is only exception to
On 2017/01/04 23:25:30, achuithb wrote:
> the only exception

Done.

[Devlin, 2017-01-06 20:55:28]

On 2016/12/29 20:29:58, tbarzic wrote:
> Long-term vision for what? New feature properties? I don't expect to be adding
> any new ones, but I can't be sure that there won't be a need for new ones at
> some point.
> 
> I don't expect to add new kiosk related session types either - there might be
> new session types in general (e.g. public_session), but they would be defined
by
> existing user session types (i.e. types for existing session types would be
> added to FeatureSessionType enum as they are needed).

There has been an influx of new features-related things for session types,
kiosk, etc, in order to allow many of these things.  If there's more planned
(APIs we need to expose under different conditions than what we have discussed +
this CL), it probably warrants a longer discussion and a design doc of some
kind.  Is this the last of the bunch, after which we only need to update
existing APIs feature declarations, or are there more conditions under which we
will expose different things?

[tbarzic, 2017-01-06 21:21:01]

On 2017/01/06 20:55:28, Devlin wrote:
> On 2016/12/29 20:29:58, tbarzic wrote:
> > Long-term vision for what? New feature properties? I don't expect to be
adding
> > any new ones, but I can't be sure that there won't be a need for new ones at
> > some point.
> > 
> > I don't expect to add new kiosk related session types either - there might
be
> > new session types in general (e.g. public_session), but they would be
defined
> by
> > existing user session types (i.e. types for existing session types would be
> > added to FeatureSessionType enum as they are needed).
> 
> There has been an influx of new features-related things for session types,
> kiosk, etc, in order to allow many of these things.  If there's more planned
> (APIs we need to expose under different conditions than what we have discussed
+
> this CL), it probably warrants a longer discussion and a design doc of some
> kind.  Is this the last of the bunch, after which we only need to update
> existing APIs feature declarations, or are there more conditions under which
we
> will expose different things?

I don't expect new features-related things to be added for kiosk apps.

It might make sense to eventually expand allowed session_types with additional
values,
but that would be part of discussion whether it's desirable to restrict a
feature to
that particular session type. I don't plan to add any new conditions to support
kiosk apps.

[Devlin, 2017-01-09 23:34:25]

extensions lgtm

https://codereview.chromium.org/2601803003/diff/120001/extensions/common/feat...
File extensions/common/features/feature_session_type.cc (left):

https://codereview.chromium.org/2601803003/diff/120001/extensions/common/feat...
extensions/common/features/feature_session_type.cc:34:
CHECK_EQ(g_current_session_type, kDefaultSessionType);
Why this change?  If it's something we want, we should do it in a separate CL
(it's very orthogonal to this change, it seems).

https://codereview.chromium.org/2601803003/diff/120001/extensions/common/feat...
File extensions/common/features/simple_feature.cc (right):

https://codereview.chromium.org/2601803003/diff/120001/extensions/common/feat...
extensions/common/features/simple_feature.cc:503: if
(!base::ContainsValue(session_types_, session_type)) {
nitty nit: I think this is slightly cleaner as:
if (base::ContainsValue(session_types_, session_type))
  return true;

// AUTOLAUNCHED...
return ...

[tbarzic, 2017-01-10 18:14:58]

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

[commit-bot: I haz the power, 2017-01-10 18:15:21]

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

[tbarzic, 2017-01-10 18:28:45]

https://codereview.chromium.org/2601803003/diff/120001/extensions/common/feat...
File extensions/common/features/feature_session_type.cc (left):

https://codereview.chromium.org/2601803003/diff/120001/extensions/common/feat...
extensions/common/features/feature_session_type.cc:34:
CHECK_EQ(g_current_session_type, kDefaultSessionType);
On 2017/01/09 23:34:25, Devlin wrote:
> Why this change?  If it's something we want, we should do it in a separate CL
> (it's very orthogonal to this change, it seems).

Done.

https://codereview.chromium.org/2601803003/diff/120001/extensions/common/feat...
File extensions/common/features/simple_feature.cc (right):

https://codereview.chromium.org/2601803003/diff/120001/extensions/common/feat...
extensions/common/features/simple_feature.cc:503: if
(!base::ContainsValue(session_types_, session_type)) {
On 2017/01/09 23:34:25, Devlin wrote:
> nitty nit: I think this is slightly cleaner as:
> if (base::ContainsValue(session_types_, session_type))
>   return true;
> 
> // AUTOLAUNCHED...
> return ...

Done.

[commit-bot: I haz the power, 2017-01-10 19:28:58]

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

[commit-bot: I haz the power, 2017-01-10 19:28:59]

Dry run: This issue passed the CQ dry run.

[tbarzic, 2017-01-10 20:07:33]

The CQ bit was checked by tbarzic@chromium.org

[tbarzic, 2017-01-10 20:07:33]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
achuith@chromium.org, rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2601803003/#ps140001
(title: ".")

[commit-bot: I haz the power, 2017-01-10 20:08:10]

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-10 20:13:22]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1484078853059190,
"parent_rev": "6a418942c5e74b35a9d6e4a43a517270a94b1a5f", "commit_rev":
"5babfd313562ebd60a9a7910912a764bed6d63c0"}

[commit-bot: I haz the power, 2017-01-10 20:13:51]

Description was changed from

==========
Introduce kiosk.autolaunched feature session type

The session type will be used to restrict extension features to
auto-launched kiosk sessions - kiosk session that are launched
automatically from the login screen, without user interaction.

To be able properly est auto launched flag, browser process
feature session type initialization is moved to
AppLaunchController, class which controls kiosk app launch.

Also, move browser process feature session type initialization
from ChromeUserManager::UpdateLoginState to
UserSessionManager::PerformPostUserLoggedInActions (for regular
users), which seems like a more suitable place.

BUG=676213
==========

to

==========
Introduce kiosk.autolaunched feature session type

The session type will be used to restrict extension features to
auto-launched kiosk sessions - kiosk session that are launched
automatically from the login screen, without user interaction.

To be able properly est auto launched flag, browser process
feature session type initialization is moved to
AppLaunchController, class which controls kiosk app launch.

Also, move browser process feature session type initialization
from ChromeUserManager::UpdateLoginState to
UserSessionManager::PerformPostUserLoggedInActions (for regular
users), which seems like a more suitable place.

BUG=676213

Review-Url: https://codereview.chromium.org/2601803003
Cr-Commit-Position: refs/heads/master@{#442667}
Committed:
https://chromium.googlesource.com/chromium/src/+/5babfd313562ebd60a9a7910912a...
==========

[commit-bot: I haz the power, 2017-01-10 20:13:53]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/5babfd313562ebd60a9a7910912a...