Add option to install an ephemeral app to the Windows jump list

chrome/browser/ui/views/apps/jumplist_updater_win.h

+// 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_APPS_JUMPLIST_UPDATER_WIN_H_
+#define CHROME_BROWSER_UI_VIEWS_APPS_JUMPLIST_UPDATER_WIN_H_
+
+#include <vector>
+
+#include "base/files/file_path.h"
+#include "base/strings/string16.h"
+#include "ui/gfx/image/image.h"

[tapted, 2014/02/12 10:16:28]

nit: don't think this is used

+
+class Profile;
+
+// Updates the jump list of an application. Jump lists are available in
+// Windows 7 and later only.
+// Note that all list items are added to the predefined "Tasks" category.
+// Support for other categories or custom categories will be added when
+// required.
+class JumpListUpdater {
+ public:
+  // A single item in the jump list. "chrome.exe" will be invoked for each
+  // list item. Each item can specify additional command line switches.
+  struct ListItem {
+    // List item caption.
+    base::string16 title;
+
+    // The path of the list item's icon.
+    base::FilePath icon_path;
+
+    // Command line args for the list item.
+    base::string16 command_line_args;
+
+    ListItem(const base::string16& title,
+             const base::FilePath& icon_path,
+             const base::string16& args)
+        : title(title), icon_path(icon_path), command_line_args(args) {}

[tapted, 2014/02/12 10:16:28]

I don't think anything's calling this constructor. Remove?

+
+    ListItem(const base::string16& title, const base::FilePath& icon_path)

[tapted, 2014/02/12 10:16:28]

nit: non-trivial constructors shouldn't be defined in headers (I think clang
emits errors, but clang doesn't (yet!) usually run on windows-only files).

This class also has a non-trivial destructor which will be auto-generated (and
also inlined), so it needs a ~ListItem(); declared here and given an empty body
in the .cc

+        : title(title), icon_path(icon_path) {}
+  };
+
+  // Append a switch and value to command line args.

[tapted, 2014/02/12 10:16:28]

This feels a bit out of place here (e.g. it's not clear why this is special to
JumpListUpdater). And dealing with strings means that some of the "append stuff"
logic is repeated from CommandLine::GetCommandLineString() - I think you can
lean on CommandLine more.

I'm thinking that it might be neater to have data members just: profile_,
app_user_model_. Then construct the full CommandLine each time in Update(), like

PathService::Get(base::FILE_EXE, &chrome_path);
CommandLine command_line(chrome_path);
AppendCommonSwitches(&command_line);
AddTasks(jumplist, command_line, items);  // Note different signature.

Then, in ListItem, replace `command_line_args` with a `CommandLine args`
(initialized with CommandLine::NO_PROGRAM in the constructor).

+  static void AppendArgs(const std::string& switch_string,
+                         const base::string16& value,
+                         base::string16* command_line_args);
+
+  JumpListUpdater(Profile* profile,
+                  const base::string16& app_user_model_id);
+  virtual ~JumpListUpdater();
+
+  // Update the jump list.
+  void Update(const std::vector<ListItem>& items);

[tapted, 2014/02/12 10:16:28]

Actually, partway through the review, I just found `ShellLinkItemList` from
`src/chrome/browser/jumplist_win.h`. I think this function could potentially
take a `const ShellLinkItemList& items`.  Is the ShellLinkItem class usable in
place of `ListItem` (maybe after some refactoring to make it more flexible)?

Then... I think we need to consider exposing `AddShellLink` form the anonymous
namespace in jumplist_win.cc. It really is too similar to justify maintaining
them both (unless there's a good reason :). It might make my ~CommandLine
comments obsolete (unless jumplist_win.cc gets refactored as well), but I think
there are strong code-hygiene benefits if we share code with jumplist_win.cc.

+
+ private:
+  Profile* profile_;
+
+  // The Windows AppUserModelID.
+  base::string16 app_user_model_id_;
+
+  // Absolute path to chrome.exe.
+  base::FilePath chrome_path_;
+
+  // Common command line switches for each list item.
+  base::string16 default_chrome_switches_;
+
+  DISALLOW_COPY_AND_ASSIGN(JumpListUpdater);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_APPS_JUMPLIST_UPDATER_WIN_H_