New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 833125 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature

Blocked on:
issue 822504



Sign in to add a comment

Add disable function for Crostini

Project Member Reported by rjwright@chromium.org, Apr 15 2018

Issue description

Termina VMs and Crostini containers need to be able to be removed once installed. This involves adding code to clean up the VM disk image, plus a settings section to access it & manage the preference.


 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 17 2018

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

commit e57e2a162df2da81a4e58c7897af8835b869dd9c
Author: Renee Wright <rjwright@chromium.org>
Date: Tue Apr 17 04:47:41 2018

Add DestroyDiskImage to crostini_manager and concierge_client

Add a dbus client method for the DestroyDiskImage Concierge service. Add a method to CorstiniManager to check the input. This is not used yet. Will be used in my next CL which adds a button in the Settings page to delete Crostini VMs.

Bug:  833125 

Change-Id: I6dd817a23764b484f4ac507adb0566ccbfeac5df
Reviewed-on: https://chromium-review.googlesource.com/1013657
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Renée Wright <rjwright@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551244}
[modify] https://crrev.com/e57e2a162df2da81a4e58c7897af8835b869dd9c/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/e57e2a162df2da81a4e58c7897af8835b869dd9c/chrome/browser/chromeos/crostini/crostini_manager.h
[modify] https://crrev.com/e57e2a162df2da81a4e58c7897af8835b869dd9c/chrome/browser/chromeos/crostini/crostini_manager_unittest.cc
[modify] https://crrev.com/e57e2a162df2da81a4e58c7897af8835b869dd9c/chromeos/dbus/concierge_client.cc
[modify] https://crrev.com/e57e2a162df2da81a4e58c7897af8835b869dd9c/chromeos/dbus/concierge_client.h
[modify] https://crrev.com/e57e2a162df2da81a4e58c7897af8835b869dd9c/chromeos/dbus/fake_concierge_client.cc
[modify] https://crrev.com/e57e2a162df2da81a4e58c7897af8835b869dd9c/chromeos/dbus/fake_concierge_client.h

Blockedon: 822504
Labels: -Pri-3 Hotlist-Announce Proj-Containers OS-Chrome Pri-1
Project Member

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

Labels: ReleaseBlock-Dev
Labels: M-68
Project Member

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

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

commit 6a8e3e332ff33922e81c47b37963359b34cd9712
Author: Renee Wright <rjwright@chromium.org>
Date: Fri Apr 27 03:39:29 2018

Add code to delete a Crostini VM when the preference is set to false

Adds a SettingsPageUIHandler subclass called CrostiniHandler which
- launches the CrostiniInstallerView when requested by settings
- destroys a Crostini VM/Container when requested by settings

This code doesn't really do anything yet, because it relies on Settings
UI to send requests. Settings UI section for Crostini is added in the
next CL.

Bug:  833125 
Change-Id: I556077e50979d39a887629d9d5f5aa748166f193
Reviewed-on: https://chromium-review.googlesource.com/1013824
Commit-Queue: Renée Wright <rjwright@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554292}
[modify] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/chromeos/crostini/crostini_manager.h
[add] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/chromeos/crostini/crostini_remover.cc
[add] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/chromeos/crostini/crostini_remover.h
[modify] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/chromeos/crostini/crostini_util.cc
[modify] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/chromeos/crostini/crostini_util.h
[modify] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/extensions/api/settings_private/prefs_util.cc
[modify] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc
[add] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/ui/webui/settings/chromeos/crostini_handler.h
[modify] https://crrev.com/6a8e3e332ff33922e81c47b37963359b34cd9712/chrome/browser/ui/webui/settings/md_settings_ui.cc

Project Member

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

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

commit f63af09a47e4ce36fe42f08427201c3152c120c7
Author: Renee Wright <rjwright@chromium.org>
Date: Sat Apr 28 00:57:53 2018

Add a Chrome Settings section to toggle the costini.enabled preference

This CL adds a new settings section - crostini-page - to the basic
settings page if --enable-features=ExperimentalCrostiniUI is set.

If kCrostiniEnabled is false, crostini-page has a "TURN ON" button
which launches the CrostiniInstallerView. Accepting the
CrostiniInstallerView installs a new Crostini VM/Container.

If kCrostiniEnabled is true crostini-page has an arrow to navigate to
the subpage - crostini-subpage.

On crostini-subpage there is a "REMOVE" button which launches a dialog
to confirm removing the Crostini VM/Container. Confirming remove
deletes the VM/Container, and navigates back to the basic settings
page.

Install/remove requests are all made by sending messages to
CrostiniHandler via crostini_browser_proxy.


-------------------------TO TEST THIS CL---------------------------
Patch this CL + this dependency CL
- https://chromium-review.googlesource.com/c/chromium/src/+/1013824/

You may also want to patch the next CL in the set
- https://chromium-review.googlesource.com/c/chromium/src/+/1023510

To make it possible to toggle the kCrostiniEnabled preference back
to false in  on ChromeOS on Linux you may need to change where we set
it in CrostiniHandler.cc

Build ChromeOS on Linux & run with
--enable-features=ExperimentalCrostiniUI

You can also test the full version by running it on an Eve
------------------------------------------------------------------


Bug:  833125 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If421b9780414336f058469f28192dd3c761cc7bf
Reviewed-on: https://chromium-review.googlesource.com/1013957
Commit-Queue: Renée Wright <rjwright@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554589}
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/app/settings_strings.grdp
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/basic_page/basic_page.html
[add] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/crostini_page/BUILD.gn
[add] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/crostini_page/crostini_browser_proxy.html
[add] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/crostini_page/crostini_browser_proxy.js
[add] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/crostini_page/crostini_page.html
[add] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/crostini_page/crostini_page.js
[add] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/crostini_page/crostini_subpage.html
[add] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/crostini_page/crostini_subpage.js
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/icons.html
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/route.js
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/settings_main/settings_main.html
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/settings_menu/settings_menu.html
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/settings_resources.grd
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/settings_ui/settings_ui.html
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/resources/settings/settings_ui/settings_ui.js
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
[modify] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/test/data/webui/settings/cr_settings_browsertest.js
[add] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/test/data/webui/settings/crostini_page_test.js
[add] https://crrev.com/f63af09a47e4ce36fe42f08427201c3152c120c7/chrome/test/data/webui/settings/test_crostini_browser_proxy.js

Project Member

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

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

commit c9db730671b7df468e32b6e0246f4484aae38109
Author: Renee Wright <rjwright@chromium.org>
Date: Sat Apr 28 04:21:24 2018

Add spinner to indicate Crostini uninstall in progress

Doesn't close/message on error yet.

Bug:  833125 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I8ab6007c0d9c5bbdb7e8bf84759355ad500e4cca
Reviewed-on: https://chromium-review.googlesource.com/1023510
Commit-Queue: Renée Wright <rjwright@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554628}
[modify] https://crrev.com/c9db730671b7df468e32b6e0246f4484aae38109/chrome/browser/resources/settings/crostini_page/crostini_subpage.html
[modify] https://crrev.com/c9db730671b7df468e32b6e0246f4484aae38109/chrome/browser/resources/settings/crostini_page/crostini_subpage.js

This has multiple CLs, is this fixed?
Status: Fixed (was: Started)

Sign in to add a comment