Refactor of BrowserDistribution.

chrome/installer/util/browser_distribution.h

 // Copyright (c) 2012 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.
 //
 // This file declares a class that contains various method related to branding.
 
 #ifndef CHROME_INSTALLER_UTIL_BROWSER_DISTRIBUTION_H_
 #define CHROME_INSTALLER_UTIL_BROWSER_DISTRIBUTION_H_
 
 #include <string>
@@ skipping 11 matching lines @@
 class BrowserDistribution {
  public:
   enum Type {
     CHROME_BROWSER,
     CHROME_FRAME,
     CHROME_BINARIES,
     CHROME_APP_HOST,
     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();
 
   static BrowserDistribution* GetSpecificDistribution(Type type);
 
   Type GetType() const { return type_; }
 
   virtual void DoPostUninstallOperations(const Version& version,
                                          const base::FilePath& local_data_path,
                                          const string16& distribution_data);
 
   // Returns the GUID to be used when registering for Active Setup.
   virtual string16 GetActiveSetupGuid();
 
   virtual string16 GetAppGuid();
 
   // Returns the unsuffixed application name of this program.
   // This is the base of the name registered with Default Programs on Windows.
   // IMPORTANT: This should only be called by the installer which needs to make
   // decisions on the suffixing of the upcoming install, not by external callers
   // 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.
   // IMPORTANT: This should only be called by ShellUtil::GetAppId as the appid
   // should be suffixed in all scenarios.
   virtual string16 GetBaseAppId();
 
   virtual string16 GetInstallSubDir();
 
   virtual string16 GetPublisherName();
@@ skipping 19 matching lines @@
   virtual string16 GetUninstallLinkName();
 
   virtual string16 GetUninstallRegPath();
 
   virtual string16 GetVersionKey();
 
   virtual bool CanSetAsDefault();
 
   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,
   // and provides the CommandExecuteImpl class UUID if |handler_class_uuid| is
   // non-NULL.
   virtual bool GetCommandExecuteImplClsid(string16* handler_class_uuid);
 
   // Returns true if this distribution uses app_host.exe to run platform apps.
   virtual bool AppHostIsSupported();
 
   virtual void UpdateInstallStatus(bool system_install,
       installer::ArchiveType archive_type,
       installer::InstallStatus install_status);
 
   // Returns true if this distribution should set the Omaha experiment_labels
   // registry value.
   virtual bool ShouldSetExperimentLabels();
 
   virtual bool HasUserExperiments();
 
  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);
 
   const Type type_;
 
  private:
   BrowserDistribution();
 
   DISALLOW_COPY_AND_ASSIGN(BrowserDistribution);
 };
 
 #endif  // CHROME_INSTALLER_UTIL_BROWSER_DISTRIBUTION_H_