Offline discussion with calamity and mgiuca:
In the scenario where a PWA is set to open as tab we will still show "Open in App window" and still open the current tab to open in a window.

I recommend we use the text used in Android, "Open {App name}".

Deleted: mobile menu.png 102 KB

	mobile menu.png 102 KB View Download

Does that just open the app or does it launch it at the tab's location?

Sorry, I mistook Android for ARC++. It opens at the tab's location.

We already have similar text on the right-click context menu: "Open link in <appname>".

The problem is I find it a bit confusing, because you're clicking on a link to a website, and there's a separate option to "Open link in <website name>"... it's like, what's the difference between left-clicking an Inbox link, and opening link in Inbox?

I think it's even more confusing if I'm already in Inbox, and there's an option to "Open Inbox".

Maybe it's okay to diverge from Android here as the concept of windows as strong as it is on desktop.

*isn't as strong as it is on desktop.

+srahim - could you provide guidance on the menu UI string in the place of "Add to shelf..." in the 3-dot menu when the app is already added to self? 

re: c5-c7

I'm concerned of introducing a new concept "App window" by calling it so. 
Since app windows are apps, "Open <Appname>" seems natural, imo. 

WIP screencast.

Deleted: Open as app window.webm 8.7 MB

	Open as app window.webm 8.7 MB View Download

Nice! What's our plan for PWAs that loaded mixed content? We don't allow mixed content in PWA windows.

I think we should just disable the option if the page has any mixed content in it. Another option would be to start a new navigation in the app window when the the site has mixed content in it, but I think it'll just confuse users and might cause data loss.

Nice work Alan!

I found a more compelling precedent: if you right-click a link from within an app to its own scope, there is an option "Open link in new <APPNAME> window." That seems sufficiently similar to this case to convince me that we can say "Open in <APPNAME> window" on the Chrome menu.

Deleted: Screenshot 2018-03-14 at 11.18.01 AM.png 47.5 KB

	Screenshot 2018-03-14 at 11.18.01 AM.png 47.5 KB View Download

Those were added by me in  Issue 760457 . See that bug for other string changes. I never got feedback from UI so happy to revisit them if we think there is a better alternative!

Concern: What happens when PWA names are very long?
Answer: The menu tooling auto elides when menu item names are too long.

Deleted: Screenshot from 2018-03-14 14-39-13.png 72.1 KB

	Screenshot from 2018-03-14 14-39-13.png 72.1 KB View Download

FWIW so does the right click menu.

Deleted: Screenshot from 2018-03-14 14-43-13.png 63.6 KB

	Screenshot from 2018-03-14 14-43-13.png 63.6 KB View Download

- Is there a short_name we can use in menus?
- Is there a max length in short_name?

> - Is there a short_name we can use in menus?

Yes. We are using it.

> - Is there a max length in short_name?

No.

Should the "Open in app window" menu item have the app icon next to it?

c13-17

re: max length on the browser-side display and eliding (assuming adding a max length at the spec level is not desired)
Could we set the eliding length to be at about 30th letter to make the room ample but not too wide? 

re: app icon
I think only the context menu has the app icon. Not the 3-dot menu item. 

I don't *think* we want a max length at the spec level. If we did, we would want to elide strings that are longer than the max, rather than ignoring them and showing an error. So there's no point speccing this, we can just do it in Chrome.

I am happy to move eliding logic for short_name up higher, perhaps even to Manifest, so short_name is always elided to a fixed length. But for now, let's just put the elide logic in here (the menu code).

Elide the name, not the entire menu string, so it reads "Open in A very very long app name... window."

Added eliding to 30 characters.

Deleted: Screenshot from 2018-03-14 18-38-15.png 72.8 KB

	Screenshot from 2018-03-14 18-38-15.png 72.8 KB View Download

FYI: Turns out extension->short_name() doesn't get the short_name from the webmanifest as we don't appear to store that value when we install a PWA.

#21: We should probably fix that, possibly in BookmarkAppHelper::UpdateWebAppInfoFromManifest.

Wow, good find... that's sad.

BookmarkAppHelper::UpdateWebAppInfoFromManifest copies the content::Manifest into a WebApplicationInfo. ConvertWebAppToExtension in convert_web_app.cc copies WebApplicationInfo into an extension manifest.

WebApplicationInfo has no short_name field, which both of the other manifests do. So you'll need to:

- Add a short_name field to WebApplicationInfo.
- UpdateWebAppInfoFromManifest should set the short_name field of WebApplicationInfo.
- ConvertWebAppToExtension should set the kShortName key.

Filed  issue 822162  for the short_name being dropped.

Updated screencast with new name.

Deleted: Video-Mon-Mar-19-2018-11-23-31.webm 8.8 MB

	Video-Mon-Mar-19-2018-11-23-31.webm 8.8 MB View Download

 Issue 789024  has been merged into this issue.

Bumping to Pri-1 because it involves adding a string. (Must be landed by Friday March 30).

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

commit 09965805e0127744db69a6b63a9a1fc92f7875e3
Author: Alan Cutter <alancutter@chromium.org>
Date: Tue Mar 27 07:25:29 2018

Show "Open in app window" for tab if corresponding PWA is already installed

This CL changes the "Install to shelf" app menu item to "Open in <app>"
for secure pages that are within a PWA's scope.

Screencast: https://bugs.chromium.org/p/chromium/issues/attachment?aid=329979&signed_aid=nYQJIKfjlAWeVah-WIOxWg==&inline=1

Bug:  819457 
Change-Id: Iafa04c7850099eb0a83a2b230a2dd273ff71b11f
Reviewed-on: https://chromium-review.googlesource.com/959983
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546049}
[modify] https://crrev.com/09965805e0127744db69a6b63a9a1fc92f7875e3/chrome/app/chrome_command_ids.h
[modify] https://crrev.com/09965805e0127744db69a6b63a9a1fc92f7875e3/chrome/app/generated_resources.grd
[modify] https://crrev.com/09965805e0127744db69a6b63a9a1fc92f7875e3/chrome/browser/extensions/extension_util.cc
[modify] https://crrev.com/09965805e0127744db69a6b63a9a1fc92f7875e3/chrome/browser/extensions/extension_util.h
[modify] https://crrev.com/09965805e0127744db69a6b63a9a1fc92f7875e3/chrome/browser/ui/browser_command_controller.cc
[modify] https://crrev.com/09965805e0127744db69a6b63a9a1fc92f7875e3/chrome/browser/ui/extensions/application_launch.cc
[modify] https://crrev.com/09965805e0127744db69a6b63a9a1fc92f7875e3/chrome/browser/ui/extensions/application_launch.h
[modify] https://crrev.com/09965805e0127744db69a6b63a9a1fc92f7875e3/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/09965805e0127744db69a6b63a9a1fc92f7875e3/chrome/browser/ui/toolbar/app_menu_model.cc
[modify] https://crrev.com/09965805e0127744db69a6b63a9a1fc92f7875e3/tools/metrics/actions/actions.xml

Verified on M68 (10663.0.0, 68.0.3425.0).