App windows launched in windows ASH should not be messing with the AppUserModelId for the remote HW…

chrome/browser/ui/views/apps/chrome_native_app_window_views_win.cc

Index: chrome/browser/ui/views/apps/chrome_native_app_window_views_win.cc
diff --git a/chrome/browser/ui/views/apps/chrome_native_app_window_views_win.cc b/chrome/browser/ui/views/apps/chrome_native_app_window_views_win.cc
index c0c63a61e6ad3ed2d1dc0c56b5de8975055ca3a0..9ffbd976a046038e85e66462e65a870f522c4f5e 100644
--- a/chrome/browser/ui/views/apps/chrome_native_app_window_views_win.cc
+++ b/chrome/browser/ui/views/apps/chrome_native_app_window_views_win.cc
@@ -112,19 +112,19 @@ void ChromeNativeAppWindowViewsWin::InitializeDefaultWindow(
   if (!extension)
     return;
 
-  std::string app_name =
-      web_app::GenerateApplicationNameFromExtensionId(extension->id());
-  base::string16 app_name_wide = base::UTF8ToWide(app_name);
-  HWND hwnd = GetNativeAppWindowHWND();
-  Profile* profile =
-      Profile::FromBrowserContext(app_window()->browser_context());
-  app_model_id_ =
-      ShellIntegration::GetAppModelIdForProfile(app_name_wide,
-                                                profile->GetPath());
-  ui::win::SetAppIdForWindow(app_model_id_, hwnd);
-
-  web_app::UpdateRelaunchDetailsForApp(profile, extension, hwnd);
-
+  if (!IsRunningInAsh()) {

[tapted, 2014/09/23 01:24:39]

I don't think we want the stuff in UpdateShelfMenu() either - that's for
updating the taskbar jumplist entries. Also, EnsureCaptionStyleSet()  below is
already in !IsRunningInAsh.

I think we can remove the `!IsRunningInAsh()` from the condition below, and here
just do

  // Remaining initialization is for Windows shell integration, which doesn't
  // apply to app windows in Ash.
  if (IsRunningInAsh())
    return;


But UpdateShelfMenu() is called from other code, and it depends on app_model_id_
being set (which now won't happen, since it's a few lines down from here).

So UpdateShelfMenu now also needs an update to bail out. Probably:

void ChromeNativeAppWindowViewsWin::UpdateShelfMenu() {
  if (!JumpListUpdater::IsEnabled() || IsRunningInAsh())
    return;

  DCHECK(!app_model_id_.empty());
  /* etc */
}


(we could also exchange app_model_id_.empty <-> IsRunningInAsh there, but I
think the former will get more error cases. But note there's not much bot
coverage for jump lists, since in tests Windows picks up a random prefix that
results in lots of small files in system dirs which explode bot disk space over
time)


Then can you update the comment on ChromeNativeAppWindowViewsWin::app_model_id_.
Just to add, "Not set for windows running inside Ash.".

(we could also just set it, but I think that's less robust).

[ananta, 2014/09/23 18:18:49]

Done.

+    std::string app_name =
+        web_app::GenerateApplicationNameFromExtensionId(extension->id());
+    base::string16 app_name_wide = base::UTF8ToWide(app_name);
+    HWND hwnd = GetNativeAppWindowHWND();
+    Profile* profile =
+        Profile::FromBrowserContext(app_window()->browser_context());
+    app_model_id_ =
+        ShellIntegration::GetAppModelIdForProfile(app_name_wide,
+                                                  profile->GetPath());
+    ui::win::SetAppIdForWindow(app_model_id_, hwnd);
+    web_app::UpdateRelaunchDetailsForApp(profile, extension, hwnd);
+  }
   if (!create_params.alpha_enabled && !IsRunningInAsh())
     EnsureCaptionStyleSet();
   UpdateShelfMenu();