Refactor chrome_launcher_support::GetAnyChromePath.

chrome/installer/launcher_support/chrome_launcher_support.cc

 // 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.
 
 #include "chrome/installer/launcher_support/chrome_launcher_support.h"
 
 #include <windows.h>
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ skipping 114 matching lines @@
 #endif
 }
 
 }  // namespace
 
 base::FilePath GetChromePathForInstallationLevel(InstallationLevel level) {
   return FindExeRelativeToSetupExe(
       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;
 }
 
 }  // namespace chrome_launcher_support