Refactor chrome_launcher_support::GetAnyChromePath.

chrome/installer/launcher_support/chrome_launcher_support.cc

Index: chrome/installer/launcher_support/chrome_launcher_support.cc
diff --git a/chrome/installer/launcher_support/chrome_launcher_support.cc b/chrome/installer/launcher_support/chrome_launcher_support.cc
index 1c95519c6e9e7f771c7769fd8e03b54c96b76440..554d0b0cdd5cdd9f6eb0052ffe8b6df764a1c829 100644
--- a/chrome/installer/launcher_support/chrome_launcher_support.cc
+++ b/chrome/installer/launcher_support/chrome_launcher_support.cc
@@ -132,20 +132,17 @@ base::FilePath GetChromePathForInstallationLevel(InstallationLevel level) {
       GetSetupExeForInstallationLevel(level), kChromeExe);
 }
 
-base::FilePath GetAnyChromePath() {
-  base::FilePath chrome_path;
-  if (chrome_path.empty())
-    chrome_path = GetChromePathForInstallationLevel(SYSTEM_LEVEL_INSTALLATION);
-  if (chrome_path.empty())
-    chrome_path = GetChromePathForInstallationLevel(USER_LEVEL_INSTALLATION);
-  return chrome_path;
-}
-
-base::FilePath GetAnyChromeSxSPath() {
-  base::FilePath path =
-      GetChromeSxSPathForInstallationLevel(USER_LEVEL_INSTALLATION);
-  if (path.empty())
+base::FilePath GetAnyChromePath(bool is_canary) {
+  base::FilePath path;
+  if (is_canary) {

[grt (UTC plus 2), 2015/01/09 17:45:28]

i think you could simplify this by making GetSetupExeForInstallationLevel handle
chrome_sxs and doing away with GetChromeSxSPathForInstallationLevel altogether,
leaving you with something like
  path = GetChromePath(SYSTEM_LEVEL_INSTALLATION, chrome_sxs);
  if (path.empty())
    path = GetChromePath(USER_LEVEL_INSTALLATION, chrome_sxs);
and changing GetChromePathForInstallationLevel to:
  return GetChromePath(level, false /* chrome_sxs /*);

[Matt Giuca, 2015/01/12 06:54:26]

I didn't quite follow your example, but I take the point... made
GetChromePathForInstallationLevel take a Boolean rather than have two separate
functions, and then this simplified the implementation of GetAnyChromePath to a
bare minimum.

[grt (UTC plus 2), 2015/01/12 14:02:41]

Sorry if my proposal was confusing. I was suggesting a way that would leave
GetChromePathForInstallationLevel as-is so that its callers wouldn't need to
change. What you have is here is okay, though. I don't see a problem exposing
is_sxs there as well.

[Matt Giuca, 2015/01/13 08:22:00]

Acknowledged.

     path = GetChromeSxSPathForInstallationLevel(SYSTEM_LEVEL_INSTALLATION);
+    if (path.empty())
+      path = GetChromeSxSPathForInstallationLevel(USER_LEVEL_INSTALLATION);
+  } else {
+    path = GetChromePathForInstallationLevel(SYSTEM_LEVEL_INSTALLATION);
+    if (path.empty())
+      path = GetChromePathForInstallationLevel(USER_LEVEL_INSTALLATION);
+  }
   return path;
 }