New issue
Advanced search Search tips
Starred by 18 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Feature

Blocked on:
issue 898295
issue 909063


Show other hotlists

Hotlists containing this issue:
Chromium-bugs-related-to-Crostini


Sign in to add a comment
link

Issue 822514: Uninstall Linux app from Launcher

Reported by tbuck...@chromium.org, Mar 16 2018 Project Member

Issue description

By right-clicking a Linux app in Launcher, the user should have the option to uninstall it.

As part of this, we should expose Linux apps anywhere else the user has the option to uninstall.
 

Comment 1 by vapier@chromium.org, May 9 2018

Components: OS>Systems>Containers

Comment 2 by tbuck...@chromium.org, May 10 2018

Labels: -Hotlist-Crostini-Apps Hotlist-Crostini-UI

Comment 3 by jkardatzke@chromium.org, May 18 2018

Owner: jkardatzke@chromium.org

Comment 4 by jkardatzke@chromium.org, May 18 2018

Labels: -Restrict-View-Google

Comment 5 by jkardatzke@chromium.org, May 25 2018

Cc: tbuck...@chromium.org benwells@chromium.org nverne@chromium.org dgreid@chromium.org smbar...@chromium.org
This looks like it shouldn't be all that bad to do. The only link we have for this is the .desktop file, since that's the origin of the launcher entry. We can then run 'dpkg -S foo.desktop' and that'll give us the actual package that is installed that contained that .desktop file (this will totally break if the user manually creates a desktop file since it won't be part of an installed package).

Then after we know the package, we can either remove it with apt-get or dpkg. (even if it's installed with dpkg, apt-get will still let you remove it). We probably don't want to use the 'purge' option which removes all associated configuration files since that's not what's normally done by default on a remove.

Feedback to the user I think would be done via a Chrome OS notification (either success or failure)....we may even want to do the progress indication like we might do for .deb installs because we'd have the plumbing already installed for that (although it should go pretty fast for an uninstall since it's not removing all of the installed dependencies as well, which is generally what takes the majority of the time).

Comment 6 by benwells@chromium.org, May 28 2018

This will break if the user manually created the desktop file, or if they install it in some non-dpkg/apt way (e.g. via source).

Could we only show the uninstall option if we can detect the package with dpkg -S?

I'm also slightly concerned about the non-purginess as that is different to how chrome apps work. We could add an option to the uninstall confirmation.

Comment 7 by jkardatzke@chromium.org, May 29 2018

Sounds reasonable to make this another attribute for a VMApplication for whether or not the uninstall option should be shown.

And I'd let UX decide about how to handle the purging...if they're fine with having a checkbox option on the uninstall confirmation prompt, then that works for me.

Comment 8 by benhenry@chromium.org, Aug 2

Status: Assigned (was: Available)

Comment 9 by jkardatzke@chromium.org, Aug 27

Owner: iby@chromium.org

Comment 10 by iby@chromium.org, Aug 27

Status: Started (was: Assigned)

Comment 11 by bugdroid1@chromium.org, Sep 18

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/9939aeafb5ca330c1c20c6aa3aa01c2af9870424

commit 9939aeafb5ca330c1c20c6aa3aa01c2af9870424
Author: Ian Barkley-Yeung <iby@google.com>
Date: Tue Sep 18 21:03:48 2018

Add package info for Crostini apps

Add package_id of owning package to the Cicerone -> Host proto
ApplicationList

BUG=chromium:822514
TEST=Apps from .desktop files associated with a package now have
'uninstall' on right-click menu; Apps from .desktop files I created do
not have 'uninstall'

Change-Id: Ibe148209763a4633a653107b2819a56146d5706f
Reviewed-on: https://chromium-review.googlesource.com/1213615
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>

[modify] https://crrev.com/9939aeafb5ca330c1c20c6aa3aa01c2af9870424/dbus/vm_applications/apps.proto

Comment 12 by bugdroid1@chromium.org, Sep 18

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/084b5a95824dd37caca9a33c183512f6dbc8e5da

commit 084b5a95824dd37caca9a33c183512f6dbc8e5da
Author: Ian Barkley-Yeung <iby@google.com>
Date: Tue Sep 18 21:03:52 2018

vm_tools: Add package info to UpdateApplicationListRequest

When garcon sends the list of applications to the host, also include
info about what package (if any) owns each application.

Only applications owned by a package can be automatically uninstalled,
and we need the package_id to do the uninstallation.

BUG=chromium:822514

TEST=Installed multiple packages with .desktop files, and copied
.desktop files to new name so that it didn't have a package. Confirmed
that owned files had reasonable package_id and unowned files
did not have a package_id.

Change-Id: I63207eecbb250306df45716c51664c60b731a324
Reviewed-on: https://chromium-review.googlesource.com/1199947
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/084b5a95824dd37caca9a33c183512f6dbc8e5da/vm_tools/proto/container_host.proto
[modify] https://crrev.com/084b5a95824dd37caca9a33c183512f6dbc8e5da/vm_tools/garcon/host_notifier.h
[modify] https://crrev.com/084b5a95824dd37caca9a33c183512f6dbc8e5da/vm_tools/garcon/main.cc
[modify] https://crrev.com/084b5a95824dd37caca9a33c183512f6dbc8e5da/vm_tools/garcon/package_kit_proxy.cc
[modify] https://crrev.com/084b5a95824dd37caca9a33c183512f6dbc8e5da/vm_tools/garcon/package_kit_proxy.h
[modify] https://crrev.com/084b5a95824dd37caca9a33c183512f6dbc8e5da/vm_tools/garcon/host_notifier.cc

Comment 13 by bugdroid1@chromium.org, Sep 19

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/155e06d564adb41054a8629929c511510f452c78

commit 155e06d564adb41054a8629929c511510f452c78
Author: Ian Barkley-Yeung <iby@google.com>
Date: Wed Sep 19 15:59:31 2018

vm_tools: Pass package_id from container to host

Modified Cicerone to pass the package_id from garcon in the container
out to the host OS.

BUG=chromium:822514
TEST=Apps from .desktop files associated with a package now have
'uninstall' on right-click menu; Apps from .desktop files I created do
not have 'uninstall'
CG-DEPEND=CL:1199947,CL:1213615

Change-Id: I7c9c5f1dc0c5fc988c32c3d9b55fe330427c27b7
Reviewed-on: https://chromium-review.googlesource.com/1214114
Commit-Ready: Ian Barkley-Yeung <iby@chromium.org>
Tested-by: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/155e06d564adb41054a8629929c511510f452c78/vm_tools/cicerone/container_listener_impl.cc

Comment 14 by bugdroid1@chromium.org, Oct 5

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

commit e2a6f7d3bd60159a8e3eda31d92d80b0c5e215fd
Author: Ian Barkley-Yeung <iby@google.com>
Date: Fri Oct 05 22:43:07 2018

vm_tools: Add uninstall functionality to garcon

Add the ability to uninstall Linux packages to garcon. Note that the
package-to-be-uninstalled is identified by the .desktop file instead of
by the package ID. We send the package_id along with the .desktop file in
UpdateApplicationList; however, nothing watches for package upgrades, so
the package_id may be stale by the time the uninstall happens. (The
package_id in UpdateApplicationList is still useful for deciding IF a
.desktop file can be auto-uninstalled, so I'm leaving it in place.)

BUG=chromium:822514
TEST=Uninstalled Eclipse using cicerone_client. Tried multiple
simultaneous uninstalls; checked that one failed with
other-uninstall-is-in-progress message. Tried various combinations of
simultaneous installs and uninstalls, ensured only one worked at a time.

Change-Id: I5e112226c4e22e868151851580ad05bef72eb938
Reviewed-on: https://chromium-review.googlesource.com/1249673
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>

[modify] https://crrev.com/e2a6f7d3bd60159a8e3eda31d92d80b0c5e215fd/vm_tools/proto/container_host.proto
[modify] https://crrev.com/e2a6f7d3bd60159a8e3eda31d92d80b0c5e215fd/vm_tools/garcon/host_notifier.h
[modify] https://crrev.com/e2a6f7d3bd60159a8e3eda31d92d80b0c5e215fd/vm_tools/garcon/README.md
[modify] https://crrev.com/e2a6f7d3bd60159a8e3eda31d92d80b0c5e215fd/vm_tools/garcon/service_impl.h
[modify] https://crrev.com/e2a6f7d3bd60159a8e3eda31d92d80b0c5e215fd/vm_tools/garcon/service_impl.cc
[modify] https://crrev.com/e2a6f7d3bd60159a8e3eda31d92d80b0c5e215fd/vm_tools/proto/container_guest.proto
[modify] https://crrev.com/e2a6f7d3bd60159a8e3eda31d92d80b0c5e215fd/vm_tools/garcon/package_kit_proxy.cc
[modify] https://crrev.com/e2a6f7d3bd60159a8e3eda31d92d80b0c5e215fd/vm_tools/garcon/package_kit_proxy.h
[modify] https://crrev.com/e2a6f7d3bd60159a8e3eda31d92d80b0c5e215fd/vm_tools/garcon/host_notifier.cc

Comment 15 by bugdroid1@chromium.org, Oct 5

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2370e824c001b3e08fc8420725ef768cfccea937

commit 2370e824c001b3e08fc8420725ef768cfccea937
Author: Ian Barkley-Yeung <iby@google.com>
Date: Fri Oct 05 22:43:08 2018

vm_tools: Pass uninstall messages through cicerone

Pass the uninstall messages to and from garcon. Also update
cicerone_client to support uninstalling.

BUG=chromium:822514
TEST=Uninstalled stuff using cicerone_client. Attempted multiple
simultaneous uninstalls, got expected error messages ("Failed starting
the package uninstall because one is already active"). Tested various
combinations of simultaneous installs and uninstalls.
CQ-DEPEND=CL:1249673

Change-Id: Ibb741e04eda9da49e1cd23fb8c5991d2bd577f79
Reviewed-on: https://chromium-review.googlesource.com/1259589
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>

[modify] https://crrev.com/2370e824c001b3e08fc8420725ef768cfccea937/vm_tools/cicerone/container_listener_impl.cc
[modify] https://crrev.com/2370e824c001b3e08fc8420725ef768cfccea937/system_api/dbus/vm_cicerone/cicerone_service.proto
[modify] https://crrev.com/2370e824c001b3e08fc8420725ef768cfccea937/vm_tools/cicerone/container.cc
[modify] https://crrev.com/2370e824c001b3e08fc8420725ef768cfccea937/vm_tools/cicerone/client.cc
[modify] https://crrev.com/2370e824c001b3e08fc8420725ef768cfccea937/vm_tools/dbus/org.chromium.VmCicerone.conf
[modify] https://crrev.com/2370e824c001b3e08fc8420725ef768cfccea937/vm_tools/cicerone/service.cc
[modify] https://crrev.com/2370e824c001b3e08fc8420725ef768cfccea937/vm_tools/cicerone/service.h
[modify] https://crrev.com/2370e824c001b3e08fc8420725ef768cfccea937/vm_tools/cicerone/container_listener_impl.h
[modify] https://crrev.com/2370e824c001b3e08fc8420725ef768cfccea937/system_api/dbus/vm_cicerone/dbus-constants.h
[modify] https://crrev.com/2370e824c001b3e08fc8420725ef768cfccea937/vm_tools/cicerone/container.h

Comment 16 by bugdroid1@chromium.org, Oct 31

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

commit d1d88eb978c081cc819cce4ce8b2fdd5c6b47d87
Author: Ian Barkley-Yeung <iby@google.com>
Date: Wed Oct 31 19:42:37 2018

system_api: Add proto fields to support dependency message in uninstall

Per UX workup (slide 8), attempting to uninstall a package that has
dependencies should result in a special error message which lists those
dependencies. Prepare for this by adding the appropriate fields to the
cicerone -> host proto. This is needs to be done first so that I can
prepare the host for the new messages before I modify garcon to send
them.

BUG=chromium:822514
TEST=None

Change-Id: Iede114a239c8d42e7e18846fcddb544f2ee9d811
Reviewed-on: https://chromium-review.googlesource.com/1300197
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>

[modify] https://crrev.com/d1d88eb978c081cc819cce4ce8b2fdd5c6b47d87/system_api/dbus/vm_cicerone/cicerone_service.proto

Comment 18 by bugdroid1@chromium.org, Nov 30

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5aed8804b44b0a909ac975cd3db58a7895a2e055

commit 5aed8804b44b0a909ac975cd3db58a7895a2e055
Author: Ian Barkley-Yeung <iby@chromium.org>
Date: Fri Nov 30 23:47:22 2018

Add uninstall menu option to Crostini apps

Add uninstall menu option to Crostini apps that can be uninstalled from
the container.

BUG=chromium:822514
TEST=Apps from .desktop files associated with a package now have
'uninstall' on right-click menu; Apps from .desktop files I created do
not have 'uninstall'

R=timloh@chromium.org

Change-Id: If7bcd69c08a806b98a4200d2de7ca4f550c57c21
Reviewed-on: https://chromium-review.googlesource.com/c/1213519
Commit-Queue: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612850}
[modify] https://crrev.com/5aed8804b44b0a909ac975cd3db58a7895a2e055/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/5aed8804b44b0a909ac975cd3db58a7895a2e055/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/5aed8804b44b0a909ac975cd3db58a7895a2e055/chrome/browser/ui/app_list/crostini/crostini_app_context_menu.cc
[modify] https://crrev.com/5aed8804b44b0a909ac975cd3db58a7895a2e055/chrome/browser/ui/app_list/crostini/crostini_app_context_menu.h
[modify] https://crrev.com/5aed8804b44b0a909ac975cd3db58a7895a2e055/chrome/common/chrome_features.cc
[modify] https://crrev.com/5aed8804b44b0a909ac975cd3db58a7895a2e055/chrome/common/chrome_features.h

Comment 19 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42f06e96b2b61b432136007c24a989146bb152c9

commit 42f06e96b2b61b432136007c24a989146bb152c9
Author: Ian Barkley-Yeung <iby@chromium.org>
Date: Fri Dec 14 03:00:41 2018

Add Uninstall UI for Crostini Apps

Add UI to handle the "uninstall" menu option when right-clicking on a
Crostini app. This change brings up a confirmation dialog; if the user
confirms, it starts up the container and sends an uninstall message.

UI mocks https://docs.google.com/presentation/d/1akjEdjidRcMH54SK9zMmQVIPaPkwVuatXuKZLJKUqzA/edit

Several known bugs:
 * chromium:909071
 * chromium:909063
 * chromium:898295
so this is behind a flag (CrostiniAppUninstallGui)

BUG=chromium:822514
TEST=Uninstalled multiple applications. Uninstalled applications at the same time for
queuing. Closed notifications while applications were uninstalling or queued. Installed
and uninstalled applications at the same time.

Change-Id: If5bb80cb867e55c3f082c40f89357f05db151985
Reviewed-on: https://chromium-review.googlesource.com/c/1275292
Commit-Queue: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Anand Mistry <amistry@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616567}
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/app/chromeos_strings.grdp
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/crostini/crostini_manager.h
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/crostini/crostini_manager_unittest.cc
[delete] https://crrev.com/4a696c0c1c9c067cf321b795a488b28a2aafe1b4/chrome/browser/chromeos/crostini/crostini_package_installer_notification.cc
[delete] https://crrev.com/4a696c0c1c9c067cf321b795a488b28a2aafe1b4/chrome/browser/chromeos/crostini/crostini_package_installer_notification.h
[delete] https://crrev.com/4a696c0c1c9c067cf321b795a488b28a2aafe1b4/chrome/browser/chromeos/crostini/crostini_package_installer_service.cc
[delete] https://crrev.com/4a696c0c1c9c067cf321b795a488b28a2aafe1b4/chrome/browser/chromeos/crostini/crostini_package_installer_service.h
[add] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/crostini/crostini_package_notification.cc
[add] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/crostini/crostini_package_notification.h
[add] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/crostini/crostini_package_operation_status.h
[add] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/crostini/crostini_package_service.cc
[add] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/crostini/crostini_package_service.h
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/crostini/crostini_util.h
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/chromeos/extensions/file_manager/private_api_misc.h
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/ui/app_list/crostini/crostini_app_context_menu.cc
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/ui/browser_dialogs.h
[add] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/ui/views/crostini/crostini_app_uninstaller_view.cc
[add] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/browser/ui/views/crostini/crostini_app_uninstaller_view.h
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chrome/common/extensions/api/file_manager_private_internal.idl
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chromeos/dbus/cicerone_client.cc
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chromeos/dbus/cicerone_client.h
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chromeos/dbus/fake_cicerone_client.cc
[modify] https://crrev.com/42f06e96b2b61b432136007c24a989146bb152c9/chromeos/dbus/fake_cicerone_client.h

Comment 20 by iby@chromium.org, Dec 15

Blockedon: 909063 898295

Comment 21 by bugdroid1@chromium.org, Dec 20

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

commit f1ebe9f3d276323a87f91f9a7e5e3fbc9438f6b5
Author: Ian Barkley-Yeung <iby@chromium.org>
Date: Thu Dec 20 06:25:48 2018

Show uninstall / install completion notification

Show the Crostini install / uninstall completion notification even if
the user closed the pending or in-progress one.

BUG=chromium:822514
TEST=Uninstalled application. Uninstalled app and closed notification
while in progress. Uninstalled multiple applications. Uninstalled
multiple applications and closed notifications when queued. Uninstalled
applications multiple times to get failures. Uninstalled multiple
applications multiple times and close notifications when queued.
Installed application. Installed application and closed notification
while in progress.

R=timloh@chromium.org

Change-Id: I8d6b68c8145c0eef9e34690a0fce68cc44b4b612
Reviewed-on: https://chromium-review.googlesource.com/c/1385190
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Ian Barkley-Yeung <iby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618126}
[modify] https://crrev.com/f1ebe9f3d276323a87f91f9a7e5e3fbc9438f6b5/chrome/browser/chromeos/crostini/crostini_package_notification.cc
[modify] https://crrev.com/f1ebe9f3d276323a87f91f9a7e5e3fbc9438f6b5/chrome/browser/chromeos/crostini/crostini_package_notification.h
[modify] https://crrev.com/f1ebe9f3d276323a87f91f9a7e5e3fbc9438f6b5/chrome/browser/chromeos/crostini/crostini_package_service.cc
[modify] https://crrev.com/f1ebe9f3d276323a87f91f9a7e5e3fbc9438f6b5/chrome/browser/chromeos/crostini/crostini_package_service.h

Comment 22 by bugdroid, Feb 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/96b891cfc51f6aee8bc63cb069c3bb80e159e940

commit 96b891cfc51f6aee8bc63cb069c3bb80e159e940
Author: Ian Barkley-Yeung <iby@chromium.org>
Date: Fri Feb 08 22:05:49 2019

Unit test for CrostiniPackageService

Add unit test for CrostiniPackageService. Also fixes a corner case found
by unit test (queuing uninstalls while install starts up).

BUG=822514
TEST=Ran unit test

Change-Id: Ieede7119febda6a2fb67ca9835e8d859ddf08400
Reviewed-on: https://chromium-review.googlesource.com/c/1401611
Commit-Queue: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630485}
[modify] https://crrev.com/96b891cfc51f6aee8bc63cb069c3bb80e159e940/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/96b891cfc51f6aee8bc63cb069c3bb80e159e940/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/96b891cfc51f6aee8bc63cb069c3bb80e159e940/chrome/browser/chromeos/crostini/crostini_package_service.cc
[modify] https://crrev.com/96b891cfc51f6aee8bc63cb069c3bb80e159e940/chrome/browser/chromeos/crostini/crostini_package_service.h
[add] https://crrev.com/96b891cfc51f6aee8bc63cb069c3bb80e159e940/chrome/browser/chromeos/crostini/crostini_package_service_unittest.cc
[modify] https://crrev.com/96b891cfc51f6aee8bc63cb069c3bb80e159e940/chromeos/dbus/fake_cicerone_client.cc
[modify] https://crrev.com/96b891cfc51f6aee8bc63cb069c3bb80e159e940/chromeos/dbus/fake_cicerone_client.h

Comment 23 by bugdroid, Feb 12

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/8876e84aa842030ddd21df7bfa3c1f5eb6dad169

commit 8876e84aa842030ddd21df7bfa3c1f5eb6dad169
Author: Ian Barkley-Yeung <iby@google.com>
Date: Tue Feb 12 05:40:58 2019

system_api: Revert crrev.com/c/1300197

Remove the proto fields added to support a "dependency message" in
Crostini uninstall UI. This was never implemented and I don't think it
was the right approach. (See comment on UX slide deck.) If the new owner
of this feature wants to implement something like this, they should
decide how best to represent it in a proto.

This field was never used anywhere, so we shouldn't need to reserve the
tag number.

BUG=chromium:822514
TEST=Compiled

Change-Id: Icdbe7dd0a26404afdae9f461dcdf2df12d81fd7f
Reviewed-on: https://chromium-review.googlesource.com/1464143
Commit-Ready: Ian Barkley-Yeung <iby@chromium.org>
Tested-by: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>

[modify] https://crrev.com/8876e84aa842030ddd21df7bfa3c1f5eb6dad169/system_api/dbus/vm_cicerone/cicerone_service.proto

Sign in to add a comment