New issue
Advanced search Search tips

Issue 898739 link

Starred by 2 users
Status: Fixed
Owner:
sidereal@google.com
Closed: Nov 9
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Crostini installation should clean up on failure

Project Member Reported by nverne@chromium.org, Oct 25 
tbuckley: Currently there can be flakiness during setup that prevents setup from completing but still leaves Crostini usable afterwards, so we rely on users clicking "Remove Linux" to explicitly clean up for now. As setup becomes more reliable, we will have cancellation undo anything that has happened.'

When the Crostini installer fails, the user has Retry and Cancel options in the installer dialog.

 I believe if the user hits Cancel in the Installer dialog at any time after starting installation, we could then invoke the existing "Remove Crostini" logic.
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 9 

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

commit 0bc2c59840eea1947b1c6a6c7fc1636fb8df5d92
Author: Fergus Dall <sidereal@google.com>
Date: Fri Nov 09 04:39:43 2018

Clean up crostini install if the user cancels installation

Currently, if crostini installation fails for any reason, including
because the user canceled it, we abort and leave behind any downloaded
files, disk images etc. and only delete these if the user explicitly
uninstalls crostini. This is both confusing to the user (why does
crostini show up in the app launcher if the install
failed/was canceled?) and leaves a bunch of files on disk that they
don't want.

This CL changes the behavior of the cancel button on the install dialogue
to invoke the existing CrostiniRemover logic. As a protection against
accidental destruction of user data, this CL also checks if a VM disk
already existed when the CreateDiskImage step was run, and if so
skips the uninstall logic. This requires passing the result status from
concierge back up to the installer view.

Bug:  898739 
Change-Id: Id6a8a36dfcb59feec4da6d11efa453e1f15e07b5
Reviewed-on: https://chromium-review.googlesource.com/c/1316980
Reviewed-by: Renée Wright <rjwright@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Fergus Dall <sidereal@google.com>
Cr-Commit-Position: refs/heads/master@{#606735}
[modify] https://crrev.com/0bc2c59840eea1947b1c6a6c7fc1636fb8df5d92/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/0bc2c59840eea1947b1c6a6c7fc1636fb8df5d92/chrome/browser/chromeos/crostini/crostini_manager.h
[modify] https://crrev.com/0bc2c59840eea1947b1c6a6c7fc1636fb8df5d92/chrome/browser/chromeos/crostini/crostini_manager_unittest.cc
[modify] https://crrev.com/0bc2c59840eea1947b1c6a6c7fc1636fb8df5d92/chrome/browser/chromeos/crostini/crostini_remover.cc
[modify] https://crrev.com/0bc2c59840eea1947b1c6a6c7fc1636fb8df5d92/chrome/browser/ui/views/crostini/crostini_installer_view.cc
[modify] https://crrev.com/0bc2c59840eea1947b1c6a6c7fc1636fb8df5d92/chrome/browser/ui/views/crostini/crostini_installer_view.h

Comment 2 by sidereal@google.com, Nov 9

Status: Fixed (was: Assigned)
Implemented by above CL
►

Sign in to add a comment