New issue
Advanced search Search tips

Issue 848116 link

Starred by 10 users

Issue metadata

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



Sign in to add a comment

UI to shut down / restart Crostini

Project Member Reported by tbuck...@chromium.org, May 31 2018

Issue description

We should allow users to shut down / restart Crostini without going through crosh.

Some initial ideas:
- Right-click Terminal to see "Shut down" and "Restart" options
- Crostini row in the system area
 
If we do the right click, we probably should (a) put it on all Crostini apps (not just terminal) and (b) make it clear it is the entire VM, not just the terminal.

What do you mean by 'Crostini row in the system area'? Do you mean just a little icon in the tray (like a notification), or something bigger?
I'd disagree with putting it on all apps....that seems like way to many places to have an option to Restart/Shutdown something.

We did talk about having an icon in the system tray for when Crostini is running...and having it as right click options on there would make sense. Having it only on the Terminal icon sort of makes sense. But I don't see any other good options.
I'd much prefer an icon in the system tray. Tom - can we get UI review or a UI designer to weigh in and help us come to a decision?
Who wants to make a call here? I've got some code for putting it on the Terminal app in the launcher, but not in the shelf. 

If it was in the shelf (only), then you'd be unable to shut down Crostini if you weren't running Terminal.

Comment 5 Deleted

Personally, I think it would look nice next to night light (or anywhere in that toggle space), as the 3 icons I have are massive compared to the rest of the panel. Then it would be persistent, and the action would update the button.
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 4

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

commit d2b8d0b4621985af6ef9f4a3ce515df8ef80858e
Author: Nicholas Verne <nverne@chromium.org>
Date: Wed Jul 04 07:10:59 2018

Adds a "Shut Down Linux" context menu item.

Crostini Terminal icon now has a Shut Down Linux item, which is enabled
if and only if the default vm is running for this user.
CrostiniManager now tracks |running_vms_| and updates this multimap when
vms start and stop.

Bug:  848116 
Change-Id: I1130f0ff2e122828c81c286fa3a4362e3cb3138d
Reviewed-on: https://chromium-review.googlesource.com/1124065
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572501}
[modify] https://crrev.com/d2b8d0b4621985af6ef9f4a3ce515df8ef80858e/chrome/app/chromeos_strings.grdp
[modify] https://crrev.com/d2b8d0b4621985af6ef9f4a3ce515df8ef80858e/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/d2b8d0b4621985af6ef9f4a3ce515df8ef80858e/chrome/browser/chromeos/crostini/crostini_manager.h
[modify] https://crrev.com/d2b8d0b4621985af6ef9f4a3ce515df8ef80858e/chrome/browser/chromeos/crostini/crostini_util.cc
[modify] https://crrev.com/d2b8d0b4621985af6ef9f4a3ce515df8ef80858e/chrome/browser/chromeos/crostini/crostini_util.h
[modify] https://crrev.com/d2b8d0b4621985af6ef9f4a3ce515df8ef80858e/chrome/browser/ui/app_list/crostini/crostini_app_context_menu.cc

Labels: M-69
Owner: nverne@chromium.org
Labels: -Pri-2 Pri-1
Labels: ReleaseBlock-Beta
@nverne, I'm not seeing this in 70.0.3502.0 canary. Is this still coming for M69?
Does the CL above complete the feature request? If so, please submit a merge request for M69.
How do I submit a merge request? 
Labels: Merge-Request-69
Hope that does it.
Project Member

Comment 16 by sheriffbot@chromium.org, Jul 31

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: There is .grd file changes and we are only 34 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-69 Merge-Approved-69
Merge approved for M69.
Project Member

Comment 19 by bugdroid1@chromium.org, Aug 2

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51209d5f50005ccb7dadb8af029f6a22aa93763d

commit 51209d5f50005ccb7dadb8af029f6a22aa93763d
Author: Nicholas Verne <nverne@chromium.org>
Date: Thu Aug 02 00:15:56 2018

Adds a "Shut Down Linux" context menu item.

Crostini Terminal icon now has a Shut Down Linux item, which is enabled
if and only if the default vm is running for this user.
CrostiniManager now tracks |running_vms_| and updates this multimap when
vms start and stop.

TBR=nverne@chromium.org

(cherry picked from commit d2b8d0b4621985af6ef9f4a3ce515df8ef80858e)

Bug:  848116 
Change-Id: I1130f0ff2e122828c81c286fa3a4362e3cb3138d
Reviewed-on: https://chromium-review.googlesource.com/1124065
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#572501}
Reviewed-on: https://chromium-review.googlesource.com/1159543
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#326}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/51209d5f50005ccb7dadb8af029f6a22aa93763d/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/51209d5f50005ccb7dadb8af029f6a22aa93763d/chrome/browser/chromeos/crostini/crostini_manager.h

Status: Fixed (was: Assigned)
Project Member

Comment 21 by bugdroid1@chromium.org, Aug 2

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

commit b043a658595a6f9b1c743a09178a5b335bbca308
Author: Nicholas Verne <nverne@chromium.org>
Date: Thu Aug 02 05:17:20 2018

Revert "Adds a "Shut Down Linux" context menu item."

This reverts commit 51209d5f50005ccb7dadb8af029f6a22aa93763d.

Reason for revert: git drover workflow failed - merge conflicts were not properly resolved.

Original change's description:
> Adds a "Shut Down Linux" context menu item.
> 
> Crostini Terminal icon now has a Shut Down Linux item, which is enabled
> if and only if the default vm is running for this user.
> CrostiniManager now tracks |running_vms_| and updates this multimap when
> vms start and stop.
> 
> TBR=nverne@chromium.org
> 
> (cherry picked from commit d2b8d0b4621985af6ef9f4a3ce515df8ef80858e)
> 
> Bug:  848116 
> Change-Id: I1130f0ff2e122828c81c286fa3a4362e3cb3138d
> Reviewed-on: https://chromium-review.googlesource.com/1124065
> Reviewed-by: Timothy Loh <timloh@chromium.org>
> Commit-Queue: Nicholas Verne <nverne@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#572501}
> Reviewed-on: https://chromium-review.googlesource.com/1159543
> Reviewed-by: Nicholas Verne <nverne@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3497@{#326}
> Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}

TBR=timloh@chromium.org,nverne@chromium.org

Change-Id: I5083f6180137724545785498019fea5279ccae05
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  848116 
Reviewed-on: https://chromium-review.googlesource.com/1160001
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#331}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/b043a658595a6f9b1c743a09178a5b335bbca308/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/b043a658595a6f9b1c743a09178a5b335bbca308/chrome/browser/chromeos/crostini/crostini_manager.h

It would be nice if the option to stop Crostini would also be available on the terminal icon in the shelf.

Sign in to add a comment