When restoring kiosk app after crash, skip app installation steps

When restoring kiosk app after crash, skip app installation steps.
Installed apps should already be restored as part of starting crash
recovery session.

During crash recovery, StartupAppLauncher::Delegate always reports
network as ready, so it cannot be determined whether the network is
truly available - if it's not update checks will take ~30 minutes
to fail, during which there will be no UI shown (as there is no login
UI in this case).

Additionally, it would be unexpected for the app to recover to updated
version after a Chroem crash.

BUG=686846
Review-Url: https://codereview.chromium.org/2662113002
Cr-Commit-Position: refs/heads/master@{#447127}
Committed: https://chromium.googlesource.com/chromium/src/+/6394703fb6ccb291ae4afce516471a1b6d76c3db

[tbarzic, 2017-01-30 22:51:07]

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

[tbarzic, 2017-01-30 22:51:27]

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

[tbarzic, 2017-01-30 22:51:51]

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

[commit-bot: I haz the power, 2017-01-30 22:52:15]

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

[xiyuan, 2017-01-30 23:13:06]

https://codereview.chromium.org/2662113002/diff/1/chrome/browser/chromeos/app...
File chrome/browser/chromeos/app_mode/startup_app_launcher.cc (right):

https://codereview.chromium.org/2662113002/diff/1/chrome/browser/chromeos/app...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:105: BeginInstall();
Should we skip all installation stuff and jump to MaybeLaunchApp ?

[commit-bot: I haz the power, 2017-01-30 23:21:26]

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

[commit-bot: I haz the power, 2017-01-30 23:21:27]

Dry run: This issue passed the CQ dry run.

[tbarzic, 2017-01-30 23:45:21]

https://codereview.chromium.org/2662113002/diff/1/chrome/browser/chromeos/app...
File chrome/browser/chromeos/app_mode/startup_app_launcher.cc (right):

https://codereview.chromium.org/2662113002/diff/1/chrome/browser/chromeos/app...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:105: BeginInstall();
On 2017/01/30 23:13:06, xiyuan wrote:
> Should we skip all installation stuff and jump to MaybeLaunchApp ?

I'm slightly concerned that in certain situations we might end up in
MaybeLaunchApp without the kiosk app being locally available (e.g. if there is a
crash during initial app installation) - though, in those cases it might be
reasonable to just cancel the launch instead of showing a black screen while the
app is downloaded...

[tbarzic, 2017-01-30 23:48:02]

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

[commit-bot: I haz the power, 2017-01-30 23:48:31]

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

[xiyuan, 2017-01-30 23:49:44]

lgtm

https://codereview.chromium.org/2662113002/diff/1/chrome/browser/chromeos/app...
File chrome/browser/chromeos/app_mode/startup_app_launcher.cc (right):

https://codereview.chromium.org/2662113002/diff/1/chrome/browser/chromeos/app...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:105: BeginInstall();
On 2017/01/30 23:45:21, tbarzic wrote:
> On 2017/01/30 23:13:06, xiyuan wrote:
> > Should we skip all installation stuff and jump to MaybeLaunchApp ?
> 
> I'm slightly concerned that in certain situations we might end up in
> MaybeLaunchApp without the kiosk app being locally available (e.g. if there is
a
> crash during initial app installation) - though, in those cases it might be
> reasonable to just cancel the launch instead of showing a black screen while
the
> app is downloaded...

Good point. BeginInstall() it is then.

[tbarzic, 2017-01-30 23:55:02]

https://codereview.chromium.org/2662113002/diff/1/chrome/browser/chromeos/app...
File chrome/browser/chromeos/app_mode/startup_app_launcher.cc (right):

https://codereview.chromium.org/2662113002/diff/1/chrome/browser/chromeos/app...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:105: BeginInstall();
On 2017/01/30 23:49:44, xiyuan wrote:
> On 2017/01/30 23:45:21, tbarzic wrote:
> > On 2017/01/30 23:13:06, xiyuan wrote:
> > > Should we skip all installation stuff and jump to MaybeLaunchApp ?
> > 
> > I'm slightly concerned that in certain situations we might end up in
> > MaybeLaunchApp without the kiosk app being locally available (e.g. if there
is
> a
> > crash during initial app installation) - though, in those cases it might be
> > reasonable to just cancel the launch instead of showing a black screen while
> the
> > app is downloaded...
> 
> Good point. BeginInstall() it is then.

Actually, we should only end up in that state if the crash happens before app
launch in original session - in that case it might be better to cancel the app
launch all together (as the newest patch does :) ), wdyt?

[xiyuan, 2017-01-30 23:59:48]

lgtm

https://codereview.chromium.org/2662113002/diff/1/chrome/browser/chromeos/app...
File chrome/browser/chromeos/app_mode/startup_app_launcher.cc (right):

https://codereview.chromium.org/2662113002/diff/1/chrome/browser/chromeos/app...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:105: BeginInstall();
On 2017/01/30 23:55:02, tbarzic wrote:
> On 2017/01/30 23:49:44, xiyuan wrote:
> > On 2017/01/30 23:45:21, tbarzic wrote:
> > > On 2017/01/30 23:13:06, xiyuan wrote:
> > > > Should we skip all installation stuff and jump to MaybeLaunchApp ?
> > > 
> > > I'm slightly concerned that in certain situations we might end up in
> > > MaybeLaunchApp without the kiosk app being locally available (e.g. if
there
> is
> > a
> > > crash during initial app installation) - though, in those cases it might
be
> > > reasonable to just cancel the launch instead of showing a black screen
while
> > the
> > > app is downloaded...
> > 
> > Good point. BeginInstall() it is then.
> 
> Actually, we should only end up in that state if the crash happens before app
> launch in original session - in that case it might be better to cancel the app
> launch all together (as the newest patch does :) ), wdyt?

SG.

[tbarzic, 2017-01-31 00:06:18]

Description was changed from

==========
When restoring kiosk app after crash, skip extension update checks

During crash recovery, StartupAppLauncher::Delegate always reports
network as ready, so it cannot be determined whether the network is
truly available - if it's not update checks will take ~30 minutes
to fail, during which there will be no UI shown (as there is no login
UI in this case).

Additionally, it would be unexpected for the app to recover to updated
version after a Chroem crash.

BUG=686846
==========

to

==========
When restoring kiosk app after crash, skip app installation steps.
Installed apps should already be restored as part of starting crash
recovery session.

During crash recovery, StartupAppLauncher::Delegate always reports
network as ready, so it cannot be determined whether the network is
truly available - if it's not update checks will take ~30 minutes
to fail, during which there will be no UI shown (as there is no login
UI in this case).

Additionally, it would be unexpected for the app to recover to updated
version after a Chroem crash.

BUG=686846
==========

[commit-bot: I haz the power, 2017-01-31 00:20:49]

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

[commit-bot: I haz the power, 2017-01-31 00:20:50]

Dry run: This issue passed the CQ dry run.

[tbarzic, 2017-01-31 00:22:36]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2017-01-31 00:23:09]

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-31 00:27:20]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485822156401640,
"parent_rev": "9fb56fe502090f3cf70b4c531b763c7ba82937ee", "commit_rev":
"6394703fb6ccb291ae4afce516471a1b6d76c3db"}

[commit-bot: I haz the power, 2017-01-31 00:27:52]

Description was changed from

==========
When restoring kiosk app after crash, skip app installation steps.
Installed apps should already be restored as part of starting crash
recovery session.

During crash recovery, StartupAppLauncher::Delegate always reports
network as ready, so it cannot be determined whether the network is
truly available - if it's not update checks will take ~30 minutes
to fail, during which there will be no UI shown (as there is no login
UI in this case).

Additionally, it would be unexpected for the app to recover to updated
version after a Chroem crash.

BUG=686846
==========

to

==========
When restoring kiosk app after crash, skip app installation steps.
Installed apps should already be restored as part of starting crash
recovery session.

During crash recovery, StartupAppLauncher::Delegate always reports
network as ready, so it cannot be determined whether the network is
truly available - if it's not update checks will take ~30 minutes
to fail, during which there will be no UI shown (as there is no login
UI in this case).

Additionally, it would be unexpected for the app to recover to updated
version after a Chroem crash.

BUG=686846

Review-Url: https://codereview.chromium.org/2662113002
Cr-Commit-Position: refs/heads/master@{#447127}
Committed:
https://chromium.googlesource.com/chromium/src/+/6394703fb6ccb291ae4afce51647...
==========

[commit-bot: I haz the power, 2017-01-31 00:27:53]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/6394703fb6ccb291ae4afce51647...