Ugh, this is an extension, not an app. Anyway, the window does not seem to receive any unload/beforeunload events when the "Close" taskbar menu item is selected.

Devlin, is a non-platform-app extension's "app window" allowed to prevent itself from closing?

It seems like AppWindowLauncherItemController::Close() just calls aura::Window::Close(), so presumably a delegate for the "app" window is preventing the close from happening.

+vapier, is nassh doing something to prevent window closing? Took a peek at the source but couldn't find anything.

nassh currently doesn't have any callbacks in JS for unloading (although there are requests)

i just tested over here and it works fine w/Secure Shell

which ssh is this?  Are you referring to the legacy packaged app?  Or is there an extension version?

In either case, we don't have any "don't let this window be closed"-type functionality, AFAIK.

Have you been able to reproduce this defect with any consistency?
This may be related to my recent work, but my bigger changes were after M61.
(After crrev.com/c/692749 context commands are sent from Ash->Chrome via Mojo IPC)
If we had a reliable repro, this would be way easier to track down.

I can't repro on cyan/63.0.3238.0, context menu "Close" worked for all apps after sleep, including:
https://chrome.google.com/webstore/detail/secure-shell/pnhechapfaindjhompbnflcldabbghjo?hl=en

And a correction to #2, the context menu command is executed via LauncherContextMenu::ExecuteCommand, which handles MENU_CLOSE via ChromeLauncherController::Close. That no-ops if there's no ShelfItemDelegate found for the ShelfID; I'm not sure how that might happen, but it's a possible cause for the defect here. Not sure why there's a TODO(simonhong) there...

Yep, it's the Secure Shell v1 packaged app. I can repro it 100% with this extension's windows. I'll see if it repros on CrOS on Linux to try debugging it when I get a chance.

Found the issue. It does repro on ToT btw; I don't know why others haven't been able to reproduce.

AppShortcutLauncherItemController::Close() should close the window[1]. But it doesn't find the window: ChromeLauncherController::GetV1ApplicationsFromAppId() returns early unless the shelf item is for a *pinned* app.[2]

Even if the shelf item is pinned, the bug repros, I think the pinned shortcut gets replaced with the TYPE_APP shelf item when the packaged app is launched.

Full repro steps:

1. install a v1 packaged app like https://chrome.google.com/webstore/detail/secure-shell/pnhechapfaindjhompbnflcldabbghjo?hl=en
2. launch it, so it opens a window
3. in the shelf, right-click the item's icon and choose Close

Naturally I'm getting errors from Gerrit trying to upload my patch, but I do have one that I'll be sending out when Gerrit lets me.

[1] https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc?type=cs&q=getv1applicationsfromappid+file:app_shortcut_launcher_item_controller%5C.cc&sq=package:chromium

[2] https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc?type=cs&sq=package:chromium&q=file:chrome_launcher_controller%5C.cc+getv1applicationsfromappid+

Uploaded https://chromium-review.googlesource.com/c/chromium/src/+/724274 but I'm not super confident in what's going on with all the different shelf delegate types and probably outdated comments -- folks participating in the refactor should take a look.

To clarify the rpro:
* If the v1 app is pineed, this works as intended.
* If the v1 app is not pinned, trying to close from the right-click menu on the shelf icon does not work.

#12: Oh, you may be right. Weird, I could swear I saw this when I tried with the icon being pinned, but now closing the app seems to work when the icon is pinned.

i have Secure Shell pinned on all my systems :)

#14: But you can repro when you un-pin it, correct? Temporarily, of course ;-)

Just found  issue 707110  as well, doesn't seem directly related but it could easily be a similar ChromeLauncherController problem

#15: yes, if i unpin, close does nothing, but as soon as i pin it, close works again

#8: When I wrote that comment, the icon of dialog window is only handled 
 by ShelfWindowWatcher. Others were managed by ChromeLauncherController.
So, I thought that making ShelfWindowWatch manage all icons in shelf is more better architecture. That's why I wrote that comment. :)

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

commit 8ddd95ebd15690d3385a7845f45255d5af3ad82b
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Wed Nov 08 23:56:28 2017

Fix Close menu item in shelf for v1 apps

v1 packaged apps are failing to respond to the "Close" option in their
shelf (task bar) item's context menu. The fix is to include unpinned
apps when looking for the shelf item, since we currently find the
shelf item from the app ID in order to find its windows and close them.

The comments and class names for launcher items aren't completely
accurate (see discussion in bug and review) but this is at least a
minor improvement.

Bug:  775416 
Change-Id: I3e26a1cf5bf5f24d86d9e35379ce21ec677bc68a
Reviewed-on: https://chromium-review.googlesource.com/724274
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515014}
[modify] https://crrev.com/8ddd95ebd15690d3385a7845f45255d5af3ad82b/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc
[modify] https://crrev.com/8ddd95ebd15690d3385a7845f45255d5af3ad82b/chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.h
[modify] https://crrev.com/8ddd95ebd15690d3385a7845f45255d5af3ad82b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc