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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/apps/chrome_native_app_window_views_win.h"
 
 #include "apps/ui/views/app_window_frame_view.h"
 #include "ash/shell.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
@@ skipping 94 matching lines @@
 }
 
 void ChromeNativeAppWindowViewsWin::InitializeDefaultWindow(
     const extensions::AppWindow::CreateParams& create_params) {
   ChromeNativeAppWindowViews::InitializeDefaultWindow(create_params);
 
   const extensions::Extension* extension = app_window()->GetExtension();
   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();
 }
 
 views::NonClientFrameView*
 ChromeNativeAppWindowViewsWin::CreateStandardDesktopAppFrame() {
   glass_frame_view_ = NULL;
   if (ui::win::IsAeroGlassEnabled()) {
     glass_frame_view_ = new GlassAppWindowFrameViewWin(this, widget());
@@ skipping 49 matching lines @@
                                               extension->id());
 
     ShellLinkItemList items;
     items.push_back(link);
     jumplist_updater.AddTasks(items);
   }
 
   // Note that an empty jumplist must still be committed to clear all items.
   jumplist_updater.CommitUpdate();
 }