Refactor of BrowserDistribution.

chrome/installer/util/browser_distribution.h

Index: chrome/installer/util/browser_distribution.h
diff --git a/chrome/installer/util/browser_distribution.h b/chrome/installer/util/browser_distribution.h
index d1459ff04fcb6ec0b7e3c8e6d8dd33c44a90701e..778dae1f7b725e9afdfeee338cd5cda4ac3dce42 100644
--- a/chrome/installer/util/browser_distribution.h
+++ b/chrome/installer/util/browser_distribution.h
@@ -29,6 +29,23 @@ class BrowserDistribution {
     NUM_TYPES
   };
 
+  enum ShortcutEnum {
+    SHORTCUT_CHROME,
+    SHORTCUT_ALTERNATE_CHROME,
+    SHORTCUT_APP_LAUNCHER

[gab, 2013/05/18 23:57:22]

I wouldn't add the SHORTCUT_APP_LAUNCHER entry just yet if you don't implement
it in this CL.

Simply add a TODO.

[calamity, 2013/05/24 02:03:11]

Implemented it because chrome_app_host_operations.cc needed the icon index of
the app launcher. Strings are hardcoded with TODOs to change them to a localized
string.

+  };
+
+  enum SubfolderEnum {
+    SUBFOLDER_CHROME,
+    SUBFOLDER_APPS

[gab, 2013/05/18 23:57:22]

Same for SUBFOLDER_APPS... don't add it yet if not implementing it.

[calamity, 2013/05/24 02:03:11]

Done.

+  };
+
+  struct ShortcutInfo {
+    string16 name;
+    string16 icon_file;
+    int icon_index;
+  };
+
   virtual ~BrowserDistribution() {}
 
   static BrowserDistribution* GetDistribution();
@@ -53,10 +70,15 @@ class BrowserDistribution {
   // at run-time.
   virtual string16 GetBaseAppName();
 
-  // Returns the localized name of the program.
-  virtual string16 GetAppShortCutName();
+  // Returns the name this distribution should use in logs.
+  virtual string16 GetDisplayName();

[gab, 2013/05/18 23:57:22]

I hadn't realized the need sometimes to only get the Chrome shortcut name (for
logs, for registry entries, and for profile shortcuts mostly); GetDisplayName()
is a good name for these cases, but I'd change the comment to mention this is
the same as the Chrome shortcut name (don't mention logs) and use it in
profile_shortcut_manager_win.

[huangs, 2013/05/21 21:35:41]

The old code uses GetAppShortCutName() for non-shortcut contexts, which I think
should be cleaned up. Currently I can think of the following contexts:
- Shortcuts: Localized, handled in this CL.
- Directories: Localized, handled in this CL.
- Logging: Fixed, for ease of processing?
- shell_util.h: GetProgIdEntries(): localized?
- shell_util.h: GetShellIntegrationEntries(): localized?

We'll have to bug grt@ for this.

[gab, 2013/05/21 21:46:45]

I think GetDisplayName() is appropriate for these. (registry entries need to be
distribution specific to allow side-by-side installations of Chrome, Chromium,
and Canary -- I don't like that some use the full localized Google Chrome name
though, but we can't change that).

[calamity, 2013/05/24 02:03:11]

Changed the comment. Is there anything else to do here? It doesn't seem to have
any use in profile_shortcut_manager_win..?

 
-  virtual string16 GetAlternateApplicationName();
+  // Returns the ShortcutInfo for this distribution.
+  virtual ShortcutInfo GetShortcutInfo(ShortcutEnum shortcut_enum);
+
+  // Returns the name of the subfolder in the Start Menu that this distribution
+  // should create shortcuts in.
+  virtual string16 GetStartMenuShortcutSubfolder(SubfolderEnum subfolder_enum);
 
   // Returns the unsuffixed appid of this program.
   // The AppUserModelId is a property of Windows programs.
@@ -96,13 +118,6 @@ class BrowserDistribution {
 
   virtual bool CanCreateDesktopShortcuts();
 
-  // Returns the executable filename (not path) that contains the product icon.
-  virtual string16 GetIconFilename();
-
-  // Returns the index of the icon for the product, inside the file specified by
-  // GetIconFilename().
-  virtual int GetIconIndex();
-
   virtual bool GetChromeChannel(string16* channel);
 
   // Returns true if this distribution includes a DelegateExecute verb handler,
@@ -126,6 +141,20 @@ class BrowserDistribution {
  protected:
   explicit BrowserDistribution(Type type);
 
+  virtual string16 GetAppShortCutName();

[gab, 2013/05/18 23:57:22]

I think we should get rid of GetAppShortCutName(), GetIconIndex(),
GetIconFilename(), etc. and simply hardcode values in GetShortcutInfo().
Otherwise we will still need to add 3 more overridable calls to implement
SHORTCUT_APP_LAUNCHER.

I'd prefer overriding GetShortcutInfo() directly in the other distributions.

[calamity, 2013/05/24 02:03:11]

Done.

+  virtual int GetIconIndex();
+  virtual string16 GetIconFilename();
+  virtual string16 GetAlternateApplicationName();
+
+  // Icons are added to the resources of the DLL using icon names. The icon
+  // index for the app list icon is named IDR_X_APP_LIST or (for official
+  // builds) IDR_X_APP_LIST_SXS for Chrome Canary. Creating shortcuts needs to
+  // specify a resource index, which are different to icon names.  They are 0
+  // based and contiguous. As Google Chrome builds have extra icons the icon for
+  // Google Chrome builds need to be higher. Unfortunately these indexes are not
+  // in any generated header file.
+  virtual int GetAppListIconIndex();

[gab, 2013/05/18 23:57:22]

Why implement this in this CL if it not implementing SHORTCUT_APPLAUNCHER yet?

[calamity, 2013/05/24 02:03:11]

Done.

[calamity, 2013/05/24 02:03:11]

Done.

+
   template<class DistributionClass>
   static BrowserDistribution* GetOrCreateBrowserDistribution(
       BrowserDistribution** dist);