New issue
Advanced search Search tips

Issue 862199 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature

Blocked on:
issue 865305



Sign in to add a comment

Crostini: Ask component updater to update termina on login

Project Member Reported by adlr@chromium.org, Jul 10

Issue description

Feature request:

Crostini code should call component updater to update Termina component after login (only for users who have crostini enabled).

The reason is that we will be locking termina version (EVN) to CrOS major version. After a user does an autoupdate to a new major version number of CrOS, they will not have a valid Termina component anymore.

xiaochu (cc'd) can help. Here's the link to the API to call: https://chromium-review.googlesource.com/c/chromium/src/+/1119199/8/chrome/browser/component_updater/cros_component_installer_chromeos.h#20
 
Note that by using kForce, component updater will NOT try to mount the image if update check fails (disconnected, throttled, etc.) even it is up-to-date.

Comment 2 Deleted

I'm currently working on a CL for this, with adlr@ and xiaochu@ as reviewers.

What I'm not sure of right now: what is the right way to determine that cros-termina is installed, but out of date?
 
I thought that imageloader's GetComponentVersion together with GetCompatiblePath would do the trick, but it looks like cros-termina has no latest-version file.
After crostini is installed the first time, there are two scenarios for an update:

1. update when current component is not compatible. This is already how it works. Component Updater forces update if current component is not compatible (by checking EVN internally) when Load API is called.
2. update when current component is still compatible (EVNs match). This is what we want to achieve here. Crostini and the platform are designed to be compatible if their EVNs match. But due to active development on canary/dev channel it's possible to introduce in-compatible changes which causes broken crostini experience on these channels. 

Though crostini and platform are versioned in the same way (crostini guest image is an overlay of platform), it is not accurate to check platform version against crostini version since crostini release is independent from the platform. e.g. the latest platform could be 10899 while the latest crostini (termina) could be 10689. 

So I suggest that instead of checking platform&crostini versions, we can force an update (kForce) after each system reboot since this is the only timing when an incompatibility could happen.

Note that we don't want to use kForce too much since that is going to increase the chance future update is throttled by the server.
There is a problem here: if the device is offline at system reboot, then the component update will fail. If it then comes online, and we try to start Crostini, then the component updater will force update? Can we know in advance that this might happen? 

i.e. I'd like to know that the component needs updating and we can't safely load Crostini in the offline scenario. I'd also like to be able to tell users that we're updating the component in the "originally offline, now online" scenario. 
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebb1555d7b1e066770439513f25e31eafa6bb449

commit ebb1555d7b1e066770439513f25e31eafa6bb449
Author: Nicholas Verne <nverne@chromium.org>
Date: Thu Jul 19 01:49:41 2018

Adds strings for use in Crostini upgrade dialog.

When a user's cros-termina component needs upgrading, but the user
is not connected to the internet, a dialog is shown.

Bug:  862199 
Change-Id: I76808e24b0ff1a256331576f56418302bb52bca5
Reviewed-on: https://chromium-review.googlesource.com/1142706
Reviewed-by: Renée Wright <rjwright@chromium.org>
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576314}
[modify] https://crrev.com/ebb1555d7b1e066770439513f25e31eafa6bb449/chrome/app/chromeos_strings.grdp

Correct, using kForce will return false always when it's offline (after reboot). If it comes back online later, it either got automatically updated by periodically update check (component updater) or we call Load API with kForce to force an update. 

Assume crostini installed is compatible (EVNs match), we can't really tell whether a newer version is available until you use kForce to check for update. So we have to make a choice: either always disable crostini after reboot until a update check returns result, or just try our best to update (using kForce) while allow user to load the existing one. I prefer the latter.
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7d9dcdd425c525414bc10be10fa57ed61046d1c0

commit 7d9dcdd425c525414bc10be10fa57ed61046d1c0
Author: Nicholas Verne <nverne@chromium.org>
Date: Thu Jul 19 23:02:59 2018

MaybeUpgradeCrostini is called at login.

If there is an already installed cros-termina component, and that component
is not compatible, an attempt is made to dowload and install a new version.

If the user is offline at login, the install attempt may be made later.

Bug:  862199 
Change-Id: I20b8790f1258dcfba8003f1031fdeb5c198d5095
Reviewed-on: https://chromium-review.googlesource.com/1139852
Reviewed-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Xiaochu Liu <xiaochu@chromium.org>
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576678}
[modify] https://crrev.com/7d9dcdd425c525414bc10be10fa57ed61046d1c0/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/7d9dcdd425c525414bc10be10fa57ed61046d1c0/chrome/browser/chromeos/crostini/crostini_manager.h
[modify] https://crrev.com/7d9dcdd425c525414bc10be10fa57ed61046d1c0/chrome/browser/chromeos/login/session/user_session_manager.cc
[add] https://crrev.com/7d9dcdd425c525414bc10be10fa57ed61046d1c0/chrome/browser/ui/views/crostini/crostini_browser_test_util.cc
[add] https://crrev.com/7d9dcdd425c525414bc10be10fa57ed61046d1c0/chrome/browser/ui/views/crostini/crostini_browser_test_util.h
[modify] https://crrev.com/7d9dcdd425c525414bc10be10fa57ed61046d1c0/chrome/browser/ui/views/crostini/crostini_installer_view_browsertest.cc
[modify] https://crrev.com/7d9dcdd425c525414bc10be10fa57ed61046d1c0/chrome/browser/ui/views/crostini/crostini_uninstaller_view_browsertest.cc
[modify] https://crrev.com/7d9dcdd425c525414bc10be10fa57ed61046d1c0/chrome/test/BUILD.gn

We still have a problem when there's a major version change. I'd like to add a method IsAnyVersionInstalled() to CrosComponentUpdater. This would return true even if the installed version is incompatible (and therefore GetCompatiblePath would return empty).

Without this, when the major version changes and GetCompatiblePath returns empty, we show the Crostini installer dialog again.
Blockedon: 865305
sg. i've created an issue for adding IsAnyVersionInstalled.
On a second thought, I'm not sure if IsAnyVersionInstalled solves the problem:

'major version changes and GetCompatiblePath returns empty and Crostini installer dialog showing' is actually expected since a compatible Termina is not installed. In this case, what crostini_manager should do is to Load(kDontForce, kMount) again (same operation as first time install). 

However to avoid confusing users, Crostini checks prefs and shows 'Update' instead of 'Install' dialog if crostini has been enabled. 

Does this make sense?
I think an Update dialog is called for. I'm not sure I want only to check prefs. If users can install crostini "behind the back of prefs" (and they can - using crosh), we'll still want to know if an incompatible version is present. 

In fact, IsIncompitableVersionInstalled might be the API I want. Until then, I can use the pref.
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0894f23fae7ecb2c5b183ece70559b9b66ac87fc

commit 0894f23fae7ecb2c5b183ece70559b9b66ac87fc
Author: Nicholas Verne <nverne@chromium.org>
Date: Wed Aug 01 00:24:39 2018

Adds IsRegistered to CrosComponentUpdater.

Returns true if any version still exists, even if incompatible with the
current Chrome version.

Bug:  862199 
Change-Id: I604e16c92a06aca57aeaa8d05215819fce2daa13
Reviewed-on: https://chromium-review.googlesource.com/1144591
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579636}
[modify] https://crrev.com/0894f23fae7ecb2c5b183ece70559b9b66ac87fc/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/0894f23fae7ecb2c5b183ece70559b9b66ac87fc/chrome/browser/component_updater/cros_component_installer_chromeos.cc
[modify] https://crrev.com/0894f23fae7ecb2c5b183ece70559b9b66ac87fc/chrome/browser/component_updater/cros_component_installer_chromeos.h

Status: Fixed (was: Untriaged)

Sign in to add a comment