Add a flag to control whether there is a shortcut for the app list / launcher in the Windows taskba…

chrome/browser/ui/views/app_list/app_list_controller_win.cc

Index: chrome/browser/ui/views/app_list/app_list_controller_win.cc
diff --git a/chrome/browser/ui/views/app_list/app_list_controller_win.cc b/chrome/browser/ui/views/app_list/app_list_controller_win.cc
index 982a0171829e228301c6a8df6edfc271e8579393..53fe0efb0e81ce7522315a3a13d34da776d454e2 100644
--- a/chrome/browser/ui/views/app_list/app_list_controller_win.cc
+++ b/chrome/browser/ui/views/app_list/app_list_controller_win.cc
@@ -5,11 +5,13 @@
 #include <sstream>
 
 #include "base/command_line.h"
+#include "base/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/path_service.h"
 #include "base/time.h"
 #include "base/timer.h"
 #include "base/utf_string_conversions.h"
+#include "base/win/shortcut.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -23,7 +25,10 @@
 #include "chrome/browser/ui/views/browser_dialogs.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
+#include "content/public/browser/browser_thread.h"
+#include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
+#include "grit/google_chrome_strings.h"
 #include "ui/app_list/app_list_view.h"
 #include "ui/app_list/pagination_model.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -40,6 +45,30 @@ namespace {
 // amount.
 static const int kAnchorOffset = 25;
 
+CommandLine GetAppListCommandLine() {
+  const char* const kSwitchesToCopy[] = { switches::kUserDataDir };
+  CommandLine* current = CommandLine::ForCurrentProcess();
+  CommandLine command_line(current->GetProgram());
+  command_line.CopySwitchesFrom(*current, kSwitchesToCopy,
+                                arraysize(kSwitchesToCopy));
+  command_line.AppendSwitch(switches::kShowAppList);
+  return command_line;
+}
+
+string16 GetAppModelId() {
+  // The AppModelId should be the same for all profiles in a user data directory
+  // but different for different user data directories, so base it on the
+  // initial profile in the current user data directory.
+  FilePath initial_profile_path;
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kUserDataDir)) {
+    initial_profile_path =
+        command_line->GetSwitchValuePath(switches::kUserDataDir).AppendASCII(
+            chrome::kInitialProfile);
+  }
+  return ShellIntegration::GetAppListAppModelIdForProfile(initial_profile_path);
+}
+
 class AppListControllerDelegateWin : public AppListControllerDelegate {
  public:
   AppListControllerDelegateWin();
@@ -86,9 +115,7 @@ class AppListController {
       views::BubbleBorder::ArrowLocation* arrow,
       gfx::Point* anchor);
   void UpdateArrowPositionAndAnchorPoint(app_list::AppListView* view);
-  CommandLine GetAppListCommandLine();
   string16 GetAppListIconPath();
-  string16 GetAppModelId();
 
   // Check if the app list or the taskbar has focus. The app list is kept
   // visible whenever either of these have focus, which allows it to be
@@ -264,8 +291,8 @@ void AppListController::GetArrowLocationAndUpdateAnchor(
 
 void AppListController::UpdateArrowPositionAndAnchorPoint(
     app_list::AppListView* view) {
-  static const int kArrowSize = 10;
-  static const int kPadding = 20;
+  const int kArrowSize = 10;
+  const int kPadding = 20;
 
   gfx::Size preferred = view->GetPreferredSize();
   // Add the size of the arrow to the space needed, as the preferred size is
@@ -287,44 +314,18 @@ void AppListController::UpdateArrowPositionAndAnchorPoint(
   view->SetAnchorPoint(anchor);
 }
 
-CommandLine AppListController::GetAppListCommandLine() {
-  CommandLine* current = CommandLine::ForCurrentProcess();
-  CommandLine command_line(current->GetProgram());
-
-  if (current->HasSwitch(switches::kUserDataDir)) {
-    FilePath user_data_dir = current->GetSwitchValuePath(
-        switches::kUserDataDir);
-    command_line.AppendSwitchPath(switches::kUserDataDir, user_data_dir);
-  }
-
-  command_line.AppendSwitch(switches::kShowAppList);
-  return command_line;
-}
-
 string16 AppListController::GetAppListIconPath() {
   FilePath icon_path;
-  if (!PathService::Get(base::DIR_MODULE, &icon_path))
+  if (!PathService::Get(base::DIR_EXE, &icon_path))
     return string16();
 
-  icon_path = icon_path.Append(chrome::kBrowserResourcesDll);
   std::stringstream ss;
-  ss << ",-" << IDI_APP_LIST;
+  ss << ",-" << IDR_APP_LIST;

[grt (UTC plus 2), 2012/11/02 19:39:38]

i suspect this doesn't work; see my comment in the .rc file.

[benwells, 2012/11/03 00:54:18]

Thanks for that, it was broken. Fixed now, and tested. It needed to have the '-'
removed, contrary to the documentation
(http://msdn.microsoft.com/en-us/library/windows/desktop/dd391573(v=vs.85).aspx).
I guess .exe's are treated as .ico's for the purposes of this command. Is there
some logical explanation of that?

   string16 result = icon_path.value();
   result.append(UTF8ToUTF16(ss.str()));
   return result;
 }
 
-string16 AppListController::GetAppModelId() {
-  static const wchar_t kAppListId[] = L"ChromeAppList";
-  // The AppModelId should be the same for all profiles in a user data directory
-  // but different for different user data directories, so base it on the
-  // initial profile in the current user data directory.
-  FilePath initial_profile_path =
-      g_browser_process->profile_manager()->GetInitialProfileDir();
-  return ShellIntegration::GetAppModelIdForProfile(kAppListId,
-                                                   initial_profile_path);
-}
-
 void AppListController::CheckTaskbarOrViewHasFocus() {
 #if !defined(USE_AURA)
   // Don't bother checking if the view has been closed.
@@ -357,6 +358,70 @@ void AppListController::CheckTaskbarOrViewHasFocus() {
 #endif
 }
 
+// Check that a taskbar shortcut exists if it should, or does not exist if
+// it should not. A taskbar shortcut should exist if the switch
+// kShowAppListShortcut is set. The shortcut will be created or deleted in
+// |user_data_dir| and will use a Windows Application Model Id of
+// |app_model_id|.
+// This runs on the FILE thread and not in the blocking IO thread pool as there
+// are other tasks running (also on the FILE thread) which fiddle with shortcut
+// icons (ShellIntegration::MigrateWin7ShortcutsOnPath). Having different
+// threads fiddle with the same threads could cause race issues.

[grt (UTC plus 2), 2012/11/02 19:39:38]

"same threads" -> "same shortcuts"

[benwells, 2012/11/03 00:54:18]

Done.

+void CheckAppListTaskbarShortcutOnFileThread(const FilePath& user_data_dir,
+                                             const string16& app_model_id) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+
+  const string16 shortcut_name = l10n_util::GetStringUTF16(
+      IDS_APP_LIST_SHORTCUT_NAME);
+  const FilePath shortcut_path(user_data_dir.Append(shortcut_name)
+      .AddExtension(L"lnk"));

[gab, 2012/11/02 12:37:28]

optional: There is a kLnkExt constant in chrome\installer\util\util_constants.h
defined as L".lnk", I didn't know there was a method .AddExtension() on
FilePath; could be nice to change the constant to L"lnk" and change usages from
.Append(name.append(kLnkExt)) to .Append(name).AddExtension(kLnkExt), I've
always hated the previous paradigm with the mix of path appends and string
appends.

Feel free to disregard and add it to our fixit:
https://docs.google.com/a/google.com/document/d/1Js17FLROy5L83Q8Qp9G6l6hRq1fH...

[benwells, 2012/11/03 00:54:18]

If it wasn't Saturday maybe I'd do it, I'll just add it to the fixit for now :)
Or I would if I had edit / comment.

[gab, 2012/11/04 16:50:34]

It's open to "anyone at google.com" maybe I should make it "anyone with the
link"..?

+  const bool should_show = CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kShowAppListShortcut);
+
+  // This will not reshow a shortcut if it has been unpinned manually by the
+  // user, as that will not delete the shortcut file.
+  if (should_show && !file_util::PathExists(shortcut_path)) {
+    FilePath chrome_exe;
+    if (!PathService::Get(base::FILE_EXE, &chrome_exe))
+      return;
+
+    base::win::ShortcutProperties shortcut_properties;
+    shortcut_properties.set_target(chrome_exe);
+    shortcut_properties.set_working_dir(chrome_exe.DirName());
+
+    string16 wide_switches(GetAppListCommandLine().GetArgumentsString());
+    shortcut_properties.set_arguments(wide_switches);
+    shortcut_properties.set_description(shortcut_name);
+
+    FilePath icon_path;
+    if (!PathService::Get(base::FILE_EXE, &icon_path))
+      return;

[gab, 2012/11/02 12:37:28]

Add NOTREACHED() before return;.

[benwells, 2012/11/03 00:54:18]

Done.

+
+    // Icons are added to the resources of the DLL using icon indexes. The
+    // icon index for the app list icon is in IDR_APP_LIST. Creating shortcuts
+    // needs to specify a resource index, which are different to icon indexes.
+    // They are 0 based and contiguous. As Google Chrome builds have an extra
+    // icon (SXS) the icon for Google Chrome builds need to be one higher.

[grt (UTC plus 2), 2012/11/02 19:39:38]

the "one higher" portion of this is out of date

[benwells, 2012/11/03 00:54:18]

Done.

+#if defined(GOOGLE_CHROME_BUILD)
+    const int kIconIndex = 5;
+#else
+    const int kIconIndex = 1;
+#endif
+    shortcut_properties.set_icon(icon_path, kIconIndex);

[grt (UTC plus 2), 2012/11/02 19:39:38]

icon_path -> chrome_exe and remove lines 396-399.

[benwells, 2012/11/03 00:54:18]

Done.

+    shortcut_properties.set_app_id(app_model_id);
+
+    base::win::CreateOrUpdateShortcutLink(shortcut_path, shortcut_properties,
+                                          base::win::SHORTCUT_CREATE_ALWAYS);
+    base::win::TaskbarPinShortcutLink(shortcut_path.value().c_str());
+    return;
+  }
+
+  if (!should_show && file_util::PathExists(shortcut_path)) {
+    base::win::TaskbarUnpinShortcutLink(shortcut_path.value().c_str());
+    file_util::Delete(shortcut_path, false);
+  }
+}
+
 }  // namespace
 
 namespace app_list_controller {
@@ -365,4 +430,12 @@ void ShowAppList() {
   g_app_list_controller.Get().ShowAppList();
 }
 
+void CheckAppListTaskbarShortcut() {
+  FilePath user_data_dir(g_browser_process->profile_manager()->user_data_dir());
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE, FROM_HERE,
+      base::Bind(&CheckAppListTaskbarShortcutOnFileThread, user_data_dir,
+                 GetAppModelId()));
+}
+
 }  // namespace app_list_controller