New issue
Advanced search Search tips

Issue 821662 link

Starred by 10 users

Issue metadata

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



Sign in to add a comment

Implement a Crostini App Registry

Project Member Reported by timloh@chromium.org, Mar 14 2018

Issue description

We need a persistent registry for Crostini apps in the browser so e.g. we don't need to start Crostini to populate the app launcher.
 
Labels: Hotlist-Crostini-Apps
Labels: Hotlist-Announce
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 22 2018

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

commit dc95cdac24c1ebaa9a0b6a70d91fa49023378875
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Mar 22 01:02:03 2018

Begin implementation of Crostini App Registry

This patch begins the implementation of the Crostini App Registry, which
will be used to maintain a cache of the installed apps so that we can
populate the app launcher without requiring Crostini to be running.
For now we just support the most basic fields from .desktop files, but
this will be expanded later to support other fields and localization.

We key the stored data off an extension id generated from the desktop
file id to simplify integration with the App List, which currently
expects the id to be a valid extension id.

Bug:  821662 
Change-Id: I196de114cf975c1402dce4b7550bc4eb5ce0ef1e
Reviewed-on: https://chromium-review.googlesource.com/961169
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544929}
[modify] https://crrev.com/dc95cdac24c1ebaa9a0b6a70d91fa49023378875/chrome/browser/chromeos/BUILD.gn
[add] https://crrev.com/dc95cdac24c1ebaa9a0b6a70d91fa49023378875/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[add] https://crrev.com/dc95cdac24c1ebaa9a0b6a70d91fa49023378875/chrome/browser/chromeos/crostini/crostini_registry_service.h
[add] https://crrev.com/dc95cdac24c1ebaa9a0b6a70d91fa49023378875/chrome/browser/chromeos/crostini/crostini_registry_service_factory.cc
[add] https://crrev.com/dc95cdac24c1ebaa9a0b6a70d91fa49023378875/chrome/browser/chromeos/crostini/crostini_registry_service_factory.h
[add] https://crrev.com/dc95cdac24c1ebaa9a0b6a70d91fa49023378875/chrome/browser/chromeos/crostini/crostini_registry_service_unittest.cc
[modify] https://crrev.com/dc95cdac24c1ebaa9a0b6a70d91fa49023378875/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/dc95cdac24c1ebaa9a0b6a70d91fa49023378875/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
[modify] https://crrev.com/dc95cdac24c1ebaa9a0b6a70d91fa49023378875/chrome/test/BUILD.gn

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/9c3e7ed65adc533bdff4ec098a34c7bf169a99fe

commit 9c3e7ed65adc533bdff4ec098a34c7bf169a99fe
Author: Timothy Loh <timloh@chromium.org>
Date: Wed Apr 04 01:47:08 2018

Add constants and proto for updating Crostini App Registry

This CL adds the vm_tools.apps.ApplicationList proto, for use as an
argument in the also newly added org.chromium.VmApplicationsService
D-Bus service.

Bug:  821662 
Change-Id: I05b05f03966eb60dd293cd9d0b46163eb9cc77e5
Reviewed-on: https://chromium-review.googlesource.com/965724
Commit-Ready: Jeffrey Kardatzke <jkardatzke@google.com>
Tested-by: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>

[modify] https://crrev.com/9c3e7ed65adc533bdff4ec098a34c7bf169a99fe/system_api.pc
[modify] https://crrev.com/9c3e7ed65adc533bdff4ec098a34c7bf169a99fe/dbus/service_constants.h
[add] https://crrev.com/9c3e7ed65adc533bdff4ec098a34c7bf169a99fe/dbus/vm_applications/apps.proto
[modify] https://crrev.com/9c3e7ed65adc533bdff4ec098a34c7bf169a99fe/system_api.gyp
[add] https://crrev.com/9c3e7ed65adc533bdff4ec098a34c7bf169a99fe/dbus/vm_applications/dbus-constants.h

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 4 2018

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

commit f41124443330887a7e062715a713815da2a92606
Author: Timothy Loh <timloh@chromium.org>
Date: Wed Apr 04 03:20:13 2018

Roll src/third_party/cros_system_api/ 4725eead5..9c3e7ed65 (2 commits)

https://chromium.googlesource.com/chromiumos/platform/system_api.git/+log/4725eead5924..9c3e7ed65adc

$ git log 4725eead5..9c3e7ed65 --date=short --no-merges --format='%ad %ae %s'
2018-04-03 timloh Add constants and proto for updating Crostini App Registry
2018-04-02 derat system_api: Add org.chromium.LockScreenService constants.

Created with:
  roll-dep src/third_party/cros_system_api

Bug:  821662 
Change-Id: I15e4f707e7b7060a2bc67f8f75d58d3d71b84995
Reviewed-on: https://chromium-review.googlesource.com/994652
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547962}
[modify] https://crrev.com/f41124443330887a7e062715a713815da2a92606/DEPS

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 4 2018

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

commit 71d65441b6c5409be898a817066b60d063380aa9
Author: Timothy Loh <timloh@chromium.org>
Date: Wed Apr 04 05:30:05 2018

Add a D-Bus interface for updating the Crostini App Registry

This patch adds an interface for updating the Crostini App Registry
over D-Bus at org.chromium.VmApplicationsService:UpdateApplicationList.

The proto is being added in:
https://chromium-review.googlesource.com/q/I05b05f03

Bug:  821662 
Change-Id: I7effada1c0de237ef3ec49da84d5c1f76415781e
Reviewed-on: https://chromium-review.googlesource.com/965803
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547990}
[modify] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chrome/browser/chromeos/crostini/crostini_registry_service_unittest.cc
[add] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chrome/browser/chromeos/dbus/vm_applications_service_provider_delegate.cc
[add] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chrome/browser/chromeos/dbus/vm_applications_service_provider_delegate.h
[modify] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chromeos/BUILD.gn
[add] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chromeos/dbus/services/org.chromium.VmApplicationsService.conf
[add] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chromeos/dbus/services/vm_applications_service_provider.cc
[add] https://crrev.com/71d65441b6c5409be898a817066b60d063380aa9/chromeos/dbus/services/vm_applications_service_provider.h

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 5 2018

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

commit 095215dfffe69b4c6dd7c3289575b3b18ed55ad1
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Apr 05 02:40:33 2018

Create app launcher items for Crostini apps

This patch adds launcher items for Crostini apps, which launch apps via
a D-Bus call to concierge (the D-Bus client code is also added in this
patch).

There are of course a lot of things missing, e.g. proper icons, name
localization, the context menu for app icons, and updating the launcher
when the registry is updated.

Bug:  821662 
Change-Id: I12f9be995a56a29ed438a54c38d8f55cd140dffb
Reviewed-on: https://chromium-review.googlesource.com/995113
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548294}
[modify] https://crrev.com/095215dfffe69b4c6dd7c3289575b3b18ed55ad1/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/095215dfffe69b4c6dd7c3289575b3b18ed55ad1/chrome/browser/chromeos/crostini/crostini_manager.h
[modify] https://crrev.com/095215dfffe69b4c6dd7c3289575b3b18ed55ad1/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/095215dfffe69b4c6dd7c3289575b3b18ed55ad1/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/095215dfffe69b4c6dd7c3289575b3b18ed55ad1/chrome/browser/ui/app_list/crostini/crostini_app_item.cc
[modify] https://crrev.com/095215dfffe69b4c6dd7c3289575b3b18ed55ad1/chrome/browser/ui/app_list/crostini/crostini_app_model_builder.cc
[modify] https://crrev.com/095215dfffe69b4c6dd7c3289575b3b18ed55ad1/chromeos/dbus/concierge_client.cc
[modify] https://crrev.com/095215dfffe69b4c6dd7c3289575b3b18ed55ad1/chromeos/dbus/concierge_client.h
[modify] https://crrev.com/095215dfffe69b4c6dd7c3289575b3b18ed55ad1/chromeos/dbus/fake_concierge_client.cc
[modify] https://crrev.com/095215dfffe69b4c6dd7c3289575b3b18ed55ad1/chromeos/dbus/fake_concierge_client.h

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 5 2018

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

commit 384e80083438b34b5a399f629ac404185f35253f
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Apr 05 05:54:09 2018

Add support for more fields in the Crostini App Registry

This patch extends the Crostini App Registry to record:
- Localized names
- Localized comments
- Mime types
- VM/Container name

The launcher items are updated to use localized names, and the concierge
call now passes the actual VM and container names. We also change the
"app_id" we use for Crostini apps to also include these names, which
should allow us to have multiple icons for an app installed in multiple
containers.

Bug:  821662 
Change-Id: I47e4da783c987a026b09decc6a67612fff3f55de
Reviewed-on: https://chromium-review.googlesource.com/995115
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548346}
[modify] https://crrev.com/384e80083438b34b5a399f629ac404185f35253f/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/384e80083438b34b5a399f629ac404185f35253f/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/384e80083438b34b5a399f629ac404185f35253f/chrome/browser/chromeos/crostini/crostini_registry_service_unittest.cc
[modify] https://crrev.com/384e80083438b34b5a399f629ac404185f35253f/chrome/browser/ui/app_list/crostini/crostini_app_item.cc
[modify] https://crrev.com/384e80083438b34b5a399f629ac404185f35253f/chrome/browser/ui/app_list/crostini/crostini_app_model_builder.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 11 2018

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

commit d4853f2bcda70fb641b4c46c6ebed522bb070652
Author: Timothy Loh <timloh@chromium.org>
Date: Wed Apr 11 04:09:33 2018

Update the app launcher when the Crostini App Registry receives updates

This patch adds an observer interface to the CrostiniAppRegistry so that
the CrostiniAppModelBuilder can be updated when the app registry is
updated.

We also fix the CrostiniAppRegistry to not delete registrations from
unrelated containers when receiving an update, to allow support for
multiple VMs or containers.

Bug:  821662 
Change-Id: I2ee679c94a7d833ad83a28da3120ad5eb3d67737
Reviewed-on: https://chromium-review.googlesource.com/1001077
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549753}
[modify] https://crrev.com/d4853f2bcda70fb641b4c46c6ebed522bb070652/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/d4853f2bcda70fb641b4c46c6ebed522bb070652/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/d4853f2bcda70fb641b4c46c6ebed522bb070652/chrome/browser/chromeos/crostini/crostini_registry_service_unittest.cc
[modify] https://crrev.com/d4853f2bcda70fb641b4c46c6ebed522bb070652/chrome/browser/ui/app_list/crostini/crostini_app_model_builder.cc
[modify] https://crrev.com/d4853f2bcda70fb641b4c46c6ebed522bb070652/chrome/browser/ui/app_list/crostini/crostini_app_model_builder.h

@timloh, can this be marked as fixed? The icon work will be tracked in  Issue 836044 .
Status: Fixed (was: Started)
Closing this as fixed (although I think I tagged the patches for startup notification against this patch)
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/1f64aefc280ef03010d5dd1cb22954ed3bfc0056

commit 1f64aefc280ef03010d5dd1cb22954ed3bfc0056
Author: Timothy Loh <timloh@chromium.org>
Date: Wed Apr 25 15:26:45 2018

Add startup_notify field to apps.proto

This patch adds a bool field startup_notify to apps.proto,
corresponding to the StartupNotify field from the Desktop Entry Spec.

https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-1.1.html#recognized-keys

Bug:  821662 
Change-Id: Ibebdca22d8c52a175113ffcf4d7e169512926b4b
Reviewed-on: https://chromium-review.googlesource.com/1021202
Commit-Ready: Timothy Loh <timloh@chromium.org>
Tested-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/1f64aefc280ef03010d5dd1cb22954ed3bfc0056/dbus/vm_applications/apps.proto

Project Member

Comment 13 by bugdroid1@chromium.org, Apr 26 2018

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

commit bd518d421b21ed772d4d9c82e02d68ba5b9faf7a
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Apr 26 03:26:02 2018

Roll src/third_party/cros_system_api/ 62d5021bb..1f64aefc2 (1 commit)

https://chromium.googlesource.com/chromiumos/platform/system_api.git/+log/62d5021bb6d4..1f64aefc280e

$ git log 62d5021bb..1f64aefc2 --date=short --no-merges --format='%ad %ae %s'
2018-04-20 timloh Add startup_notify field to apps.proto

Created with:
  roll-dep src/third_party/cros_system_api

Bug:  821662 
Change-Id: I5126d2b48bb46832aea047cb857cf8bab6cee518
Reviewed-on: https://chromium-review.googlesource.com/1029330
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553887}
[modify] https://crrev.com/bd518d421b21ed772d4d9c82e02d68ba5b9faf7a/DEPS

Project Member

Comment 14 by bugdroid1@chromium.org, Apr 26 2018

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

commit 72e5f423db422aa459faa16b16fa5aa0a23f7cce
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Apr 26 05:54:49 2018

Support StartupWMClass and StartupNotify fields in Crostini Registry

This patch adds support for storing the StartupWMClass and StartupNotify
fields in the Crostini Registry, and expands the algorithm for matching
windows to Crostini apps to use these fields. We match startup ids
against desktop file ids as the code for launching apps in the container
sets DESKTOP_STARTUP_ID to the desktop file id.

Bug:  821662 
Change-Id: I34ba361a167cd5907d76cfbfed57e3903bbf9cfd
Reviewed-on: https://chromium-review.googlesource.com/1021063
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553930}
[modify] https://crrev.com/72e5f423db422aa459faa16b16fa5aa0a23f7cce/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/72e5f423db422aa459faa16b16fa5aa0a23f7cce/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/72e5f423db422aa459faa16b16fa5aa0a23f7cce/chrome/browser/chromeos/crostini/crostini_registry_service_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/bdc5c6460b32da5d025a4e2a019a8ea7210c4f82

commit bdc5c6460b32da5d025a4e2a019a8ea7210c4f82
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Apr 26 06:00:48 2018

vm_tools: Support startup notification in garcon

Pass the StartupNotify bool to chrome when updating the application list
and set the DESKTOP_STARTUP_ID env variable to the desktop file id when
the bool is true.

BUG= chromium:821662 
TEST=unit test passes
CQ-DEPEND=CL:1021202
Change-Id: Ie4b9441cbf56cfc8c85a5d0ea4007ee509048a92
Reviewed-on: https://chromium-review.googlesource.com/1020982
Commit-Ready: Timothy Loh <timloh@chromium.org>
Tested-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>

[modify] https://crrev.com/bdc5c6460b32da5d025a4e2a019a8ea7210c4f82/vm_tools/proto/container_host.proto
[modify] https://crrev.com/bdc5c6460b32da5d025a4e2a019a8ea7210c4f82/vm_tools/garcon/desktop_file_unittest.cc
[modify] https://crrev.com/bdc5c6460b32da5d025a4e2a019a8ea7210c4f82/vm_tools/garcon/desktop_file.cc
[modify] https://crrev.com/bdc5c6460b32da5d025a4e2a019a8ea7210c4f82/vm_tools/garcon/desktop_file.h
[modify] https://crrev.com/bdc5c6460b32da5d025a4e2a019a8ea7210c4f82/vm_tools/garcon/service_impl.cc
[modify] https://crrev.com/bdc5c6460b32da5d025a4e2a019a8ea7210c4f82/vm_tools/concierge/container_listener_impl.cc
[modify] https://crrev.com/bdc5c6460b32da5d025a4e2a019a8ea7210c4f82/vm_tools/garcon/host_notifier.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Apr 26 2018

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

commit 8456ca9004deec75d7360ad74030bac5652e53f8
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Apr 26 07:52:34 2018

Add install and last launch time fields to the Crostini registry

This patch adds install_time and last_launch_time fields to the Crostini
registry. These will be used in the app launcher's recently launched
apps list, where currently Crostini apps do not appear.

As we would also like the Terminal to appear in the recents list, this
patch makes the registry support the Terminal, returning it in both
GetRegistration and GetRegisteredAppIds. We don't need to keep track of
Terminal install time as it is just a fallback for apps that haven't
been launched. Internally, we only store the last launched time, so
methods in the registry need to be careful they don't expect all pref
entries will have all the standard fields.

Bug:  821662 , 836137
Change-Id: I43f34434afe61bd80c165c3a42047b184f96fcbe
Reviewed-on: https://chromium-review.googlesource.com/1027292
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553945}
[modify] https://crrev.com/8456ca9004deec75d7360ad74030bac5652e53f8/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/8456ca9004deec75d7360ad74030bac5652e53f8/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/8456ca9004deec75d7360ad74030bac5652e53f8/chrome/browser/chromeos/crostini/crostini_registry_service_unittest.cc
[modify] https://crrev.com/8456ca9004deec75d7360ad74030bac5652e53f8/chrome/browser/chromeos/crostini/crostini_util.cc
[modify] https://crrev.com/8456ca9004deec75d7360ad74030bac5652e53f8/chrome/browser/ui/app_list/crostini/crostini_app_model_builder.cc

Sign in to add a comment