Restore auto-launched state on kiosk restart within session

Restore auto-launched state on kiosk restart within session

When kiosk app is restarted within a kiosk session - e.g. due to crash,
app's auto-launched state has to be preserved. Previously auto-launched
state was kept only in memory - if kiosk was restarted, the information
would get lost.
This CL is attempt to fix the problem by preserving auto-launched state
by setting a user session scoped flag that indicates that the kiosk app
is auto-launched (newly introduced app-auto-launched). The flag will be
scoped to current used session, and will be passed to Chrome in a case
Chrome restarts withing the user session. On restart, the flag can be
used to deduce whether the kiosk app should be considered auto-launched
or not.

Note that when setting the auto-launched flag, it's important to
consider current set of policy defined flags - they should be
preserved when auto-launched flag is added to set of user flags.

BUG=677340
Review-Url: https://codereview.chromium.org/2639033002
Cr-Original-Commit-Position: refs/heads/master@{#446166}
Committed: https://chromium.googlesource.com/chromium/src/+/663f7dfaa3b96b353ade5b9ce6b7616e51ba73bb
Review-Url: https://codereview.chromium.org/2639033002
Cr-Commit-Position: refs/heads/master@{#446427}
Committed: https://chromium.googlesource.com/chromium/src/+/c6b3e0d3813eb1bc13bbaba9e5d19054890606c7

[tbarzic, 2017-01-25 01:18:33]

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

[commit-bot: I haz the power, 2017-01-25 01:19: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-25 01:29:42]

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

[commit-bot: I haz the power, 2017-01-25 01:30:29]

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-25 03:11:25]

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

[tbarzic, 2017-01-25 03:11:26]

Let me know what you think - the test should also verify that kiosk session gets
policy flags without restarting Chrome.

[xiyuan, 2017-01-25 21:06:44]

lgtm

Just nits,

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/app_mode/kiosk_app_manager.cc (right):

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:277: bool autolaunched =
app_id == currently_auto_launched_with_zero_delay_app_;
nit: autolaunched -> auto_launched

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:280: bool
has_autolaunched_flag =
nit: has_autolaunched_flag -> has_auto_launched_flag

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/auto_launched_kiosk_browsertest.cc (right):

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/login/auto_launched_kiosk_browsertest.cc:103:
{kDefaultPolicySwitches[i].name, kDefaultPolicySwitches[i].value});
nit: wrap in {} since the body > 1 line

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

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/login/session/user_session_manager.cc:591: bool
autolaunched = base::CommandLine::ForCurrentProcess()->HasSwitch(
nit: autolaunched -> auto_launched

[tbarzic, 2017-01-25 21:33:24]

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

[tbarzic, 2017-01-25 21:33:43]

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/app_mode/kiosk_app_manager.cc (right):

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:277: bool autolaunched =
app_id == currently_auto_launched_with_zero_delay_app_;
On 2017/01/25 21:06:43, xiyuan wrote:
> nit: autolaunched -> auto_launched

Done.

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/kiosk_app_manager.cc:280: bool
has_autolaunched_flag =
On 2017/01/25 21:06:43, xiyuan wrote:
> nit: has_autolaunched_flag -> has_auto_launched_flag

Done.

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/auto_launched_kiosk_browsertest.cc (right):

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/login/auto_launched_kiosk_browsertest.cc:103:
{kDefaultPolicySwitches[i].name, kDefaultPolicySwitches[i].value});
On 2017/01/25 21:06:43, xiyuan wrote:
> nit: wrap in {} since the body > 1 line

Done.

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

https://codereview.chromium.org/2639033002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/login/session/user_session_manager.cc:591: bool
autolaunched = base::CommandLine::ForCurrentProcess()->HasSwitch(
On 2017/01/25 21:06:43, xiyuan wrote:
> nit: autolaunched -> auto_launched

Done.

[commit-bot: I haz the power, 2017-01-25 21:34:03]

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, 2017-01-25 22:54:32]

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

[commit-bot: I haz the power, 2017-01-25 22:54:33]

Dry run: This issue passed the CQ dry run.

[tbarzic, 2017-01-25 23:43:32]

The CQ bit was checked by tbarzic@chromium.org

[tbarzic, 2017-01-25 23:43:32]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/2639033002/#ps100001
(title: ".")

[commit-bot: I haz the power, 2017-01-25 23:44:06]

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-25 23:51:05]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485387812237050,
"parent_rev": "0d3069a5ad51f4d337927f5a1ed4e0945488d7c1", "commit_rev":
"663f7dfaa3b96b353ade5b9ce6b7616e51ba73bb"}

[commit-bot: I haz the power, 2017-01-25 23:51:36]

Description was changed from

==========
Restore auto-launched state on kiosk restart within session

When kiosk app is restarted within a kiosk session - e.g. due to crash,
app's auto-launched state has to be preserved. Previously auto-launched
state was kept only in memory - if kiosk was restarted, the information
would get lost.
This CL is attempt to fix the problem by preserving auto-launched state
by setting a user session scoped flag that indicates that the kiosk app
is auto-launched (newly introduced app-auto-launched). The flag will be
scoped to current used session, and will be passed to Chrome in a case
Chrome restarts withing the user session. On restart, the flag can be
used to deduce whether the kiosk app should be considered auto-launched
or not.

Note that when setting the auto-launched flag, it's important to
consider current set of policy defined flags - they should be
preserved when auto-launched flag is added to set of user flags.

BUG=677340
==========

to

==========
Restore auto-launched state on kiosk restart within session

When kiosk app is restarted within a kiosk session - e.g. due to crash,
app's auto-launched state has to be preserved. Previously auto-launched
state was kept only in memory - if kiosk was restarted, the information
would get lost.
This CL is attempt to fix the problem by preserving auto-launched state
by setting a user session scoped flag that indicates that the kiosk app
is auto-launched (newly introduced app-auto-launched). The flag will be
scoped to current used session, and will be passed to Chrome in a case
Chrome restarts withing the user session. On restart, the flag can be
used to deduce whether the kiosk app should be considered auto-launched
or not.

Note that when setting the auto-launched flag, it's important to
consider current set of policy defined flags - they should be
preserved when auto-launched flag is added to set of user flags.

BUG=677340

Review-Url: https://codereview.chromium.org/2639033002
Cr-Commit-Position: refs/heads/master@{#446166}
Committed:
https://chromium.googlesource.com/chromium/src/+/663f7dfaa3b96b353ade5b9ce6b7...
==========

[commit-bot: I haz the power, 2017-01-25 23:51:38]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/663f7dfaa3b96b353ade5b9ce6b7...

[kjellander_chromium, 2017-01-26 05:44:31]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/2658773002/ by kjellander@chromium.org.

The reason for reverting is: browser_tests test
AutoLaunchedKioskTest.PRE_CrashRestore starts timing out reliably with this CL,
starting with
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%....

[tbarzic, 2017-01-26 18:16:35]

Description was changed from

==========
Restore auto-launched state on kiosk restart within session

When kiosk app is restarted within a kiosk session - e.g. due to crash,
app's auto-launched state has to be preserved. Previously auto-launched
state was kept only in memory - if kiosk was restarted, the information
would get lost.
This CL is attempt to fix the problem by preserving auto-launched state
by setting a user session scoped flag that indicates that the kiosk app
is auto-launched (newly introduced app-auto-launched). The flag will be
scoped to current used session, and will be passed to Chrome in a case
Chrome restarts withing the user session. On restart, the flag can be
used to deduce whether the kiosk app should be considered auto-launched
or not.

Note that when setting the auto-launched flag, it's important to
consider current set of policy defined flags - they should be
preserved when auto-launched flag is added to set of user flags.

BUG=677340

Review-Url: https://codereview.chromium.org/2639033002
Cr-Commit-Position: refs/heads/master@{#446166}
Committed:
https://chromium.googlesource.com/chromium/src/+/663f7dfaa3b96b353ade5b9ce6b7...
==========

to

==========
Restore auto-launched state on kiosk restart within session

When kiosk app is restarted within a kiosk session - e.g. due to crash,
app's auto-launched state has to be preserved. Previously auto-launched
state was kept only in memory - if kiosk was restarted, the information
would get lost.
This CL is attempt to fix the problem by preserving auto-launched state
by setting a user session scoped flag that indicates that the kiosk app
is auto-launched (newly introduced app-auto-launched). The flag will be
scoped to current used session, and will be passed to Chrome in a case
Chrome restarts withing the user session. On restart, the flag can be
used to deduce whether the kiosk app should be considered auto-launched
or not.

Note that when setting the auto-launched flag, it's important to
consider current set of policy defined flags - they should be
preserved when auto-launched flag is added to set of user flags.

BUG=677340

Review-Url: https://codereview.chromium.org/2639033002
Cr-Commit-Position: refs/heads/master@{#446166}
Committed:
https://chromium.googlesource.com/chromium/src/+/663f7dfaa3b96b353ade5b9ce6b7...
==========

[tbarzic, 2017-01-26 18:19:42]

xiyuan, can you have a look at the revised version?

https://codereview.chromium.org/2639033002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/auto_launched_kiosk_browsertest.cc (right):

https://codereview.chromium.org/2639033002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/auto_launched_kiosk_browsertest.cc:351:
DCHECK(device_policy_.payload().SerializeToString(
this DCHECK made Release builder sad :/

[xiyuan, 2017-01-26 18:22:31]

still lgtm

[tbarzic, 2017-01-26 18:44:34]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2017-01-26 18:45: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-26 20:19:17]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1485456274846880,
"parent_rev": "a02187345a2ce4d5f929816aaddb1369a0afbed1", "commit_rev":
"c6b3e0d3813eb1bc13bbaba9e5d19054890606c7"}

[commit-bot: I haz the power, 2017-01-26 20:19:51]

Description was changed from

==========
Restore auto-launched state on kiosk restart within session

When kiosk app is restarted within a kiosk session - e.g. due to crash,
app's auto-launched state has to be preserved. Previously auto-launched
state was kept only in memory - if kiosk was restarted, the information
would get lost.
This CL is attempt to fix the problem by preserving auto-launched state
by setting a user session scoped flag that indicates that the kiosk app
is auto-launched (newly introduced app-auto-launched). The flag will be
scoped to current used session, and will be passed to Chrome in a case
Chrome restarts withing the user session. On restart, the flag can be
used to deduce whether the kiosk app should be considered auto-launched
or not.

Note that when setting the auto-launched flag, it's important to
consider current set of policy defined flags - they should be
preserved when auto-launched flag is added to set of user flags.

BUG=677340

Review-Url: https://codereview.chromium.org/2639033002
Cr-Commit-Position: refs/heads/master@{#446166}
Committed:
https://chromium.googlesource.com/chromium/src/+/663f7dfaa3b96b353ade5b9ce6b7...
==========

to

==========
Restore auto-launched state on kiosk restart within session

When kiosk app is restarted within a kiosk session - e.g. due to crash,
app's auto-launched state has to be preserved. Previously auto-launched
state was kept only in memory - if kiosk was restarted, the information
would get lost.
This CL is attempt to fix the problem by preserving auto-launched state
by setting a user session scoped flag that indicates that the kiosk app
is auto-launched (newly introduced app-auto-launched). The flag will be
scoped to current used session, and will be passed to Chrome in a case
Chrome restarts withing the user session. On restart, the flag can be
used to deduce whether the kiosk app should be considered auto-launched
or not.

Note that when setting the auto-launched flag, it's important to
consider current set of policy defined flags - they should be
preserved when auto-launched flag is added to set of user flags.

BUG=677340

Review-Url: https://codereview.chromium.org/2639033002
Cr-Original-Commit-Position: refs/heads/master@{#446166}
Committed:
https://chromium.googlesource.com/chromium/src/+/663f7dfaa3b96b353ade5b9ce6b7...
Review-Url: https://codereview.chromium.org/2639033002
Cr-Commit-Position: refs/heads/master@{#446427}
Committed:
https://chromium.googlesource.com/chromium/src/+/c6b3e0d3813eb1bc13bbaba9e5d1...
==========

[commit-bot: I haz the power, 2017-01-26 20:19:52]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/c6b3e0d3813eb1bc13bbaba9e5d1...