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 ca67c0bc1b5d40d7b382c0dd8280cb43a92276b0..5fbe5f55937fd1fd29f2888dd92667ebd129b3e9 100644
--- a/chrome/installer/util/browser_distribution.h
+++ b/chrome/installer/util/browser_distribution.h
@@ -29,6 +29,17 @@ class BrowserDistribution {
     NUM_TYPES
   };
 
+  enum ShortcutType {
+    SHORTCUT_CHROME,
+    SHORTCUT_ALTERNATE_CHROME,

[grt (UTC plus 2), 2013/07/05 17:27:37]

I prefer SHORTCUT_CHROME_ALTERNATE for this since it's the alternate shortcut to
Chrome rather than the shortcut to an alternate Chrome. I'm okay with this
suggesting being vetoed if the current name makes more sense to everyone else.

[gab, 2013/07/08 12:59:02]

Suggestion sgtm, please use SHORTCUT_CHROME_ALTERNATE.

[huangs, 2013/07/08 19:43:57]

(Optional) please consider changing SHORTCUT_CHROME to SHORTCUT_CHROME_DEFAULT
to facilitate code search; otherwise searching for SHORTCUT_CHROME would return
matches for both.

[gab, 2013/07/08 20:20:32]

This is easily mitigated by a simple regex though "SHORTCUT_CHROME\b".

I'd keep the shorter name as it is used most and the callsites already result in
long wrap calls.

[huangs, 2013/07/09 15:02:53]

Sounds good.

[calamity, 2013/07/16 04:05:17]

Done.

+    SHORTCUT_APP_LAUNCHER
+  };
+
+  enum Subfolder {
+    SUBFOLDER_CHROME,
+    // TODO(calamity): add SUBFOLDER_APPS when refactoring chrome app dir code.
+  };
+
   virtual ~BrowserDistribution() {}
 
   static BrowserDistribution* GetDistribution();
@@ -53,10 +64,23 @@ class BrowserDistribution {
   // at run-time.
   virtual string16 GetBaseAppName();
 
-  // Returns the localized name of the program.
-  virtual string16 GetAppShortCutName();
+  // Returns the display name of this distribution.

[gab, 2013/07/05 14:23:37]

s/display name/localized display name

to be explicit about the fact that this is localized and shouldn't be used for
static registry entry names, etc.
(I know some implementations are not localized, but the point is for callers to
be aware to it should be treated as a localized string).

[calamity, 2013/07/16 04:05:17]

Done.

+  virtual string16 GetDisplayName();
+
+  // Returns the name of the shortcut identified by |shortcut_type| for this

[gab, 2013/07/05 14:23:37]

s/name/localized name

(same idea as above)

[calamity, 2013/07/16 04:05:17]

Done.

+  // distribution.
+  virtual string16 GetShortcutName(ShortcutType shortcut_type);
+
+  // Returns the index of the icon for the product identified by
+  // |shortcut_type|, inside the file specified by GetIconFilename().
+  virtual int GetIconIndex(ShortcutType shortcut_type);
+
+  // Returns the executable filename (not path) that contains the product icon.
+  virtual string16 GetIconFilename();
 
-  virtual string16 GetAlternateApplicationName();
+  // Returns the name of the subfolder in the Start Menu identified by

[gab, 2013/07/05 14:23:37]

s/name/localized name

(same idea as above)

[calamity, 2013/07/16 04:05:17]

Done.

+  // |subfolder_type| that this distribution should create shortcuts in.

[grt (UTC plus 2), 2013/07/05 17:27:37]

Please add to this comment that this returns GetShortcutName(SHORTCUT_CHROME)
for SUBFOLDER_CHROME. The subclasses of BD assume this, so it must be part of
the contract for this method.

[gab, 2013/07/08 12:59:02]

I don't see why you say that subclasses of BD assume this? None of them even
call into this method? Or do you mean they rely on this by not overriding it?

[grt (UTC plus 2), 2013/07/08 15:15:50]

Exactly. GoogleChromeDistribution and GoogleChromeSxSDistribution implicitly
assume that they don't need to provide their own GetStartMenuShortcutSubfolder
methods since their GetShortcutName overrides will be used to get the
SUBFOLDER_CHROME shortcut folder.

[calamity, 2013/07/16 04:05:17]

Done.

+  virtual string16 GetStartMenuShortcutSubfolder(Subfolder subfolder_type);
 
   // Returns the unsuffixed appid of this program.
   // The AppUserModelId is a property of Windows programs.
@@ -96,13 +120,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,
@@ -130,6 +147,8 @@ class BrowserDistribution {
   static BrowserDistribution* GetOrCreateBrowserDistribution(
       BrowserDistribution** dist);
 
+  static const char* kDefaultAlternateChromeShortcutName;

[grt (UTC plus 2), 2013/07/05 17:27:37]

This appears to be unused. Remove it?

[calamity, 2013/07/16 04:05:17]

Done.

+
   const Type type_;
 
  private: