Suffix Chrome's appid on user-level installs

ShellIntegration::GetAppId() --> ShellIntegration::GetProfileAppId() for clarity (i.e. having two functions called "GetAppId" is confusing imo).

Patch From Gabriel Charette <gab@chromium.org>
BUG=125362, 133173
TEST=Make sure 125362 doesn't repro.
http://goo.gl/ZZ7gE
ShellIntegrationTest.GetAppModelIdForProfileTest
ShellUtilTest.BuildAppModelId*
SessionRestoreTest.RestoreAfterClosingTabbedBrowserWithAppAndLaunching
AppModeTest.EnableAppModeTest

Review URL: https://chromiumcodereview.appspot.com/10542031
Patch from Gabriel Charette <gab@chromium.org>.

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142909 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 64 insertions, 43 deletions

diff --git a/chrome/browser/jumplist_win.cc b/chrome/browser/jumplist_win.cc
index dc510c1..ddf53be 100644
--- a/chrome/browser/jumplist_win.cc
+++ b/chrome/browser/jumplist_win.cc
@@ -513,7 +513,7 @@ bool JumpList::AddObserver(Profile* profile) {
   if (!tab_restore_service)
     return false;
 
-  app_id_ = ShellIntegration::GetChromiumAppId(profile->GetPath());
+  app_id_ = ShellIntegration::GetChromiumModelIdForProfile(profile->GetPath());
   icon_dir_ = profile->GetPath().Append(chrome::kJumpListIconDirname);
   profile_ = profile;
   history::TopSites* top_sites = profile_->GetTopSites();
diff --git a/chrome/browser/shell_integration.h b/chrome/browser/shell_integration.h
index eb7d716..a1ce234 100644
--- a/chrome/browser/shell_integration.h
+++ b/chrome/browser/shell_integration.h
@@ -123,15 +123,19 @@ class ShellIntegration {
       bool is_platform_app);
 
 #if defined(OS_WIN)
-  // Generates Win7 app id for given app name and profile path. The returned app
-  // id is in the format of "|app_name|[.<profile_id>]". "profile_id" is
-  // appended when user override the default value.
-  static string16 GetAppId(const string16& app_name,
-                           const FilePath& profile_path);
-
-  // Generates Win7 app id for Chromium by calling GetAppId with
-  // chrome::kBrowserAppID as app_name.
-  static string16 GetChromiumAppId(const FilePath& profile_path);
+  // Generates an application user model ID (AppUserModelId) for a given app
+  // name and profile path. The returned app id is in the format of
+  // "|app_name|[.<profile_id>]". "profile_id" is appended when user override
+  // the default value.
+  // Note: If the app has an installation specific suffix (e.g. on user-level
+  // Chrome installs), |app_name| should already be suffixed, this method will
+  // then further suffix it with the profile id as described above.
+  static string16 GetAppModelIdForProfile(const string16& app_name,
+                                          const FilePath& profile_path);
+
+  // Generates an application user model ID (AppUserModelId) for Chromium by
+  // calling GetAppModelIdForProfile() with ShellUtil::GetAppId() as app_name.
+  static string16 GetChromiumModelIdForProfile(const FilePath& profile_path);
 
   // Returns the path to the Chromium icon. This is used to specify the icon
   // to use for the taskbar group on Win 7.
diff --git a/chrome/browser/shell_integration_unittest.cc b/chrome/browser/shell_integration_unittest.cc
index 714e295..b6168c7 100644
--- a/chrome/browser/shell_integration_unittest.cc
+++ b/chrome/browser/shell_integration_unittest.cc
@@ -365,27 +365,31 @@ TEST(ShellIntegrationTest, GetDesktopFileContents) {
   }
 }
 #elif defined(OS_WIN)
-TEST(ShellIntegrationTest, GetChromiumAppIdTest) {
+TEST(ShellIntegrationTest, GetAppModelIdForProfileTest) {
+  const string16 base_app_id(
+      BrowserDistribution::GetDistribution()->GetBaseAppId());
+
   // Empty profile path should get chrome::kBrowserAppID
   FilePath empty_path;
-  EXPECT_EQ(BrowserDistribution::GetDistribution()->GetBrowserAppId(),
-            ShellIntegration::GetChromiumAppId(empty_path));
+  EXPECT_EQ(base_app_id,
+            ShellIntegration::GetAppModelIdForProfile(base_app_id, empty_path));
 
   // Default profile path should get chrome::kBrowserAppID
   FilePath default_user_data_dir;
   chrome::GetDefaultUserDataDirectory(&default_user_data_dir);
   FilePath default_profile_path =
       default_user_data_dir.AppendASCII(chrome::kInitialProfile);
-  EXPECT_EQ(BrowserDistribution::GetDistribution()->GetBrowserAppId(),
-            ShellIntegration::GetChromiumAppId(default_profile_path));
+  EXPECT_EQ(base_app_id,
+            ShellIntegration::GetAppModelIdForProfile(base_app_id,
+                                                      default_profile_path));
 
   // Non-default profile path should get chrome::kBrowserAppID joined with
   // profile info.
   FilePath profile_path(FILE_PATH_LITERAL("root"));
   profile_path = profile_path.Append(FILE_PATH_LITERAL("udd"));
   profile_path = profile_path.Append(FILE_PATH_LITERAL("User Data - Test"));
-  EXPECT_EQ(BrowserDistribution::GetDistribution()->GetBrowserAppId() +
-            L".udd.UserDataTest",
-            ShellIntegration::GetChromiumAppId(profile_path));
+  EXPECT_EQ(base_app_id + L".udd.UserDataTest",
+            ShellIntegration::GetAppModelIdForProfile(base_app_id,
+                                                      profile_path));
 }
 #endif
diff --git a/chrome/browser/shell_integration_win.cc b/chrome/browser/shell_integration_win.cc
index 68e0cca..ceb412c 100644
--- a/chrome/browser/shell_integration_win.cc
+++ b/chrome/browser/shell_integration_win.cc
@@ -293,10 +293,12 @@ bool GetExpectedAppId(const FilePath& chrome_exe,
     app_name = UTF8ToUTF16(web_app::GenerateApplicationNameFromExtensionId(
         command_line.GetSwitchValueASCII(switches::kAppId)));
   } else {
-    app_name = BrowserDistribution::GetDistribution()->GetBrowserAppId();
+    BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+    app_name = ShellUtil::GetBrowserModelId(dist, chrome_exe.value());
   }
 
-  expected_app_id->assign(ShellIntegration::GetAppId(app_name, profile_path));
+  expected_app_id->assign(
+      ShellIntegration::GetAppModelIdForProfile(app_name, profile_path));
   return true;
 }
 
@@ -550,24 +552,27 @@ bool ShellIntegration::IsFirefoxDefaultBrowser() {
   return ff_default;
 }
 
-string16 ShellIntegration::GetAppId(const string16& app_name,
-                                    const FilePath& profile_path) {
-  string16 app_id(app_name);
-
-  string16 profile_id(GetProfileIdFromPath(profile_path));
-  if (!profile_id.empty()) {
-    app_id += L".";
-    app_id += profile_id;
-  }
-
-  // App id should be less than 128 chars.
-  DCHECK(app_id.length() < 128);
-  return app_id;
+string16 ShellIntegration::GetAppModelIdForProfile(
+    const string16& app_name,
+    const FilePath& profile_path) {
+  std::vector<string16> components;
+  components.push_back(app_name);
+  const string16 profile_id(GetProfileIdFromPath(profile_path));
+  if (!profile_id.empty())
+    components.push_back(profile_id);
+  return ShellUtil::BuildAppModelId(components);
 }
 
-string16 ShellIntegration::GetChromiumAppId(const FilePath& profile_path) {
-  return GetAppId(BrowserDistribution::GetDistribution()->GetBrowserAppId(),
-                  profile_path);
+string16 ShellIntegration::GetChromiumModelIdForProfile(
+    const FilePath& profile_path) {
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  FilePath chrome_exe;
+  if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
+    NOTREACHED();
+    return dist->GetBaseAppId();
+  }
+  return GetAppModelIdForProfile(
+      ShellUtil::GetBrowserModelId(dist, chrome_exe.value()), profile_path);
 }
 
 string16 ShellIntegration::GetChromiumIconPath() {
@@ -597,6 +602,12 @@ void ShellIntegration::MigrateChromiumShortcuts() {
 
 bool ShellIntegration::ActivateMetroChrome() {
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
-  const string16 app_id(dist->GetBrowserAppId());
+  FilePath chrome_exe;
+  if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
+    NOTREACHED();
+    return false;
+  }
+  const string16 app_id(
+      ShellUtil::GetBrowserModelId(dist, chrome_exe.value()));
   return ActivateApplication(app_id);
 }
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 5aa2819..895b108 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -534,8 +534,9 @@ void Browser::InitBrowserWindow() {
   // name.  See http://crbug.com/7028.
   ui::win::SetAppIdForWindow(
       is_app() && !is_type_panel() ?
-      ShellIntegration::GetAppId(UTF8ToWide(app_name_), profile_->GetPath()) :
-      ShellIntegration::GetChromiumAppId(profile_->GetPath()),
+      ShellIntegration::GetAppModelIdForProfile(UTF8ToWide(app_name_),
+                                                profile_->GetPath()) :
+      ShellIntegration::GetChromiumModelIdForProfile(profile_->GetPath()),
       window()->GetNativeWindow());
 
   if (is_type_panel()) {
diff --git a/chrome/browser/ui/views/extensions/shell_window_views.cc b/chrome/browser/ui/views/extensions/shell_window_views.cc
index 0d32d0a..b0a8ce6 100644
--- a/chrome/browser/ui/views/extensions/shell_window_views.cc
+++ b/chrome/browser/ui/views/extensions/shell_window_views.cc
@@ -204,8 +204,8 @@ ShellWindowViews::ShellWindowViews(Profile* profile,
   std::string app_name = web_app::GenerateApplicationNameFromExtensionId(
       extension->id());
   ui::win::SetAppIdForWindow(
-      ShellIntegration::GetAppId(UTF8ToWide(app_name),
-      profile->GetPath()),
+      ShellIntegration::GetAppModelIdForProfile(UTF8ToWide(app_name),
+                                                profile->GetPath()),
       GetWidget()->GetTopLevelWidget()->GetNativeWindow());
 #endif
   OnViewWasResized();
diff --git a/chrome/browser/ui/web_applications/web_app_ui.cc b/chrome/browser/ui/web_applications/web_app_ui.cc
index 6e36ba8..ed55194 100644
--- a/chrome/browser/ui/web_applications/web_app_ui.cc
+++ b/chrome/browser/ui/web_applications/web_app_ui.cc
@@ -8,6 +8,7 @@
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
+#include "base/string16.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/windows_version.h"
 #include "chrome/browser/extensions/extension_tab_helper.h"
@@ -248,7 +249,7 @@ void UpdateShortcutWorker::UpdateShortcutsOnFileThread() {
   CheckExistingShortcuts();
   if (!shortcut_files_.empty()) {
     // Generates app id from web app url and profile path.
-    std::wstring app_id = ShellIntegration::GetAppId(
+    string16 app_id = ShellIntegration::GetAppModelIdForProfile(
         UTF8ToWide(web_app::GenerateApplicationNameFromURL(shortcut_info_.url)),
         profile_path_);
 
diff --git a/chrome/browser/web_applications/web_app_win.cc b/chrome/browser/web_applications/web_app_win.cc
index 73fed7d..7c86dde 100644
--- a/chrome/browser/web_applications/web_app_win.cc
+++ b/chrome/browser/web_applications/web_app_win.cc
@@ -206,7 +206,7 @@ bool CreatePlatformShortcut(
   // Generates app id from web app url and profile path.
   std::string app_name =
       web_app::GenerateApplicationNameFromInfo(shortcut_info);
-  string16 app_id = ShellIntegration::GetAppId(
+  string16 app_id = ShellIntegration::GetAppModelIdForProfile(
       UTF8ToUTF16(app_name), profile_path);
 
   FilePath shortcut_to_pin;