Revert "Update Keystone glue for SxS"

This reverts commit c72dbb1c.

Reason for revert: Causing compile fail on mac-builder-perf
https://ci.chromium.org/p/chrome/builders/ci/mac-builder-perf/166934

Original change's description:
> Update Keystone glue for SxS
> 
> Each SxS-capable Chrome channel can have a different brand code, so
> update the Keystone glue to know that.
> 
> This can be tricky because the old, non-SxS-capable Chromes are still
> built from this code, so be sure to handle that case.
> 
> Overview plan at https://docs.google.com/document/d/1MVANxDLPPeN1x-NsfgsLSjbX8twzQi69Wb1iPK-owaU/edit
> 
> Bug: 1061486
> Change-Id: I2cbc2d6ba698b085ee00536a1fbfffa3336ddecd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2102642
> Commit-Queue: Avi Drissman <avi@chromium.org>
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#752637}

TBR=avi@chromium.org,mark@chromium.org

Change-Id: Id1c49dc01812441abbcd288358472d89fb159d65
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1061486
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2116154Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752667}

chrome/browser/mac/keystone_glue.mm

@@ -39,49 +39,41 @@ namespace {
 
 namespace ksr = keystone_registration;
 
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
+// Constants for the brand file (uses an external file so it can survive
+// updates to Chrome.)
 
-// Functions to handle the brand file.
-//
-// Note that an external file is used so it can survive updates to Chrome.
-//
-// Note that these directories are hard-coded in Keystone scripts, so
-// NSSearchPathForDirectoriesInDomains isn't used since the scripts couldn't use
-// anything like that.
-
-NSString* BrandFileName(version_info::Channel channel) {
-  NSString* fragment;
-
-  switch (channel) {
-    case version_info::Channel::CANARY:
-      fragment = @" Canary";
-      break;
-    case version_info::Channel::DEV:
-      fragment = @" Dev";
-      break;
-    case version_info::Channel::BETA:
-      fragment = @" Beta";
-      break;
-    default:
-      fragment = @"";
-      break;
-  }
+#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
+#define kStableBrandFileName @"Google Chrome Brand.plist"
+#define kCanaryBrandFileName @"Google Chrome Canary Brand.plist"
+#elif BUILDFLAG(CHROMIUM_BRANDING)
+#define kStableBrandFileName @"Chromium Brand.plist"
+#define kCanaryBrandFileName @"Chromium Canary Brand.plist"
+#else
+#error Unknown branding
+#endif
 
-  return [NSString stringWithFormat:@"Google Chrome%@ Brand.plist", fragment];
-}
+// These directories are hardcoded in Keystone promotion preflight and the
+// Keystone install script, so NSSearchPathForDirectoriesInDomains isn't used
+// since the scripts couldn't use anything like that.
+NSString* kStableBrandUserFile = @"~/Library/Google/" kStableBrandFileName;
+NSString* kStableBrandSystemFile = @"/Library/Google/" kStableBrandFileName;
+NSString* kCanaryBrandUserFile = @"~/Library/Google/" kCanaryBrandFileName;
+NSString* kCanaryBrandSystemFile = @"/Library/Google/" kCanaryBrandFileName;
 
 NSString* UserBrandFilePath(version_info::Channel channel) {
-  return [[@"~/Library/Google/" stringByAppendingString:BrandFileName(channel)]
-      stringByStandardizingPath];
+  NSString* file = (channel == version_info::Channel::CANARY)
+                       ? kCanaryBrandUserFile
+                       : kStableBrandUserFile;
+  return [file stringByStandardizingPath];
 }
 
 NSString* SystemBrandFilePath(version_info::Channel channel) {
-  return [[@"/Library/Google/" stringByAppendingString:BrandFileName(channel)]
-      stringByStandardizingPath];
+  NSString* file = (channel == version_info::Channel::CANARY)
+                       ? kCanaryBrandSystemFile
+                       : kStableBrandSystemFile;
+  return [file stringByStandardizingPath];
 }
 
-#endif
-
 // Adaptor for scheduling an Objective-C method call in ThreadPool.
 class PerformBridge : public base::RefCountedThreadSafe<PerformBridge> {
  public:
@@ -206,10 +198,8 @@ class PerformBridge : public base::RefCountedThreadSafe<PerformBridge> {
 - (void)changePermissionsForPromotionWithTool:(NSString*)toolPath;
 - (void)changePermissionsForPromotionComplete;
 
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
 // Returns the brand file path to use for Keystone.
 - (NSString*)brandFilePath;
-#endif
 
 // YES if no update installation has succeeded since a binary diff patch
 // installation failed. This signals the need to attempt a full installer
@@ -324,8 +314,10 @@ NSString* const kVersionKey = @"KSVersion";
   version_info::Channel channelType = chrome::GetChannelByName(channel);
   if (channelType == version_info::Channel::STABLE) {
     channel = base::SysNSStringToUTF8(ksr::KSRegistrationRemoveExistingTag);
+#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
     DCHECK(chrome::GetChannelByName(channel) == version_info::Channel::STABLE)
         << "-channel name modification has side effect";
+#endif
   }
 
   _productID.reset([productID copy]);
@@ -335,8 +327,6 @@ NSString* const kVersionKey = @"KSVersion";
   _channel = channel;
 }
 
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-
 - (NSString*)brandFilePath {
   DCHECK(_version != nil) << "-loadParameters must be called first";
 
@@ -351,22 +341,29 @@ NSString* const kVersionKey = @"KSVersion";
   // Default to none.
   _brandFile.reset(@"", base::scoped_policy::RETAIN);
 
-  // Only a side-by-side capable Chromium can have an independent brand code.
-
-  if (!chrome::IsSideBySideCapable()) {
-    // If on the older dev or beta channels that were not side-by-side capable,
-    // this installation may have replaced an older system-level installation.
-    // Check for a user brand file and nuke it if present. Don't try to remove
-    // the system brand file, there wouldn't be any permission to do so.
+  // Only the stable and canary channel can have independent brand codes.
 
-    // Don't do this on a side-by-side capable channel. Those can run
-    // side-by-side with another Google Chrome installation whose brand code, if
-    // any, should remain intact.
+  if (channel == version_info::Channel::DEV ||
+      channel == version_info::Channel::BETA) {
+    // If on the dev or beta channel, this installation may have replaced
+    // an older system-level installation. Check for a user brand file and
+    // nuke it if present. Don't try to remove the system brand file, there
+    // wouldn't be any permission to do so.
+    //
+    // Don't do this on the canary channel. The canary can run side-by-side
+    // with another Google Chrome installation whose brand code, if any,
+    // should remain intact.
 
     if ([fm fileExistsAtPath:userBrandFile]) {
       [fm removeItemAtPath:userBrandFile error:NULL];
     }
-  } else {
+
+  } else if (channel == version_info::Channel::STABLE ||
+             channel == version_info::Channel::CANARY) {
+    // Stable and Canary use different app ids, so they can both have brand
+    // codes. Even if Canary does not actively use brand codes, we want to
+    // exercise the same logic, so that we can detect perf regressions early.
+
     // If there is a system brand file, use it.
     if ([fm fileExistsAtPath:systemBrandFile]) {
       // System
@@ -423,8 +420,6 @@ NSString* const kVersionKey = @"KSVersion";
   return _brandFile;
 }
 
-#endif  // BUILDFLAG(GOOGLE_CHROME_BRANDING)
-
 - (BOOL)loadKeystoneRegistration {
   if (!_productID || !_appPath || !_url || !_version)
     return NO;
@@ -461,10 +456,7 @@ NSString* const kVersionKey = @"KSVersion";
   NSNumber* preserveTTToken = @YES;
   NSString* appInfoPlistPath = [self appInfoPlistPath];
   NSString* brandKey = kBrandKey;
-  NSString* brandPath = @"";
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-  brandPath = [self brandFilePath];
-#endif
+  NSString* brandPath = [self brandFilePath];
 
   if ([brandPath length] == 0) {
     // Brand path and brand key must be cleared together or ksadmin seems
@@ -472,7 +464,7 @@ NSString* const kVersionKey = @"KSVersion";
     brandKey = @"";
   }
 
-  // Note that _channel is permitted to be an empty string, but it must not be
+  // Note that channel_ is permitted to be an empty string, but it must not be
   // nil.
   NSString* tagSuffix = [self tagSuffix];
   NSString* tagValue =
@@ -932,7 +924,6 @@ NSString* const kVersionKey = @"KSVersion";
 
   [self updateStatus:kAutoupdatePromoting version:nil error:nil];
 
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
   // TODO(mark): Remove when able!
   //
   // keystone_promote_preflight will copy the current brand information out to
@@ -1030,7 +1021,6 @@ NSString* const kVersionKey = @"KSVersion";
   if (synchronous) {
     CFRunLoopRunInMode(kCFRunLoopDefaultMode, 10, false);
   }
-#endif  // BUILDFLAG(GOOGLE_CHROME_BRANDING)
 }
 
 - (void)promotionComplete:(NSNotification*)notification {
@@ -1154,8 +1144,6 @@ NSString* const kVersionKey = @"KSVersion";
 
 @end  // @implementation KeystoneGlue
 
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-
 namespace {
 
 std::string BrandCodeInternal() {
@@ -1176,17 +1164,12 @@ std::string BrandCodeInternal() {
 
 }  // namespace
 
-#endif  // BUILDFLAG(GOOGLE_CHROME_BRANDING)
-
 namespace keystone_glue {
 
 std::string BrandCode() {
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-  static std::string s_brand_code = std::move(BrandCodeInternal());
-  return s_brand_code;
-#else
-  return std::string();
-#endif  // BUILDFLAG(GOOGLE_CHROME_BRANDING)
+  // |s_brand_code| is leaked.
+  static std::string* s_brand_code = new std::string(BrandCodeInternal());
+  return *s_brand_code;
 }
 
 bool KeystoneEnabled() {

chrome/common/channel_info.h

@@ -46,12 +46,6 @@ version_info::Channel GetChannel();
 // Channel::STABLE, if channel is empty, else matches the name and returns
 // {STABLE,BETA,DEV,CANARY, UNKNOWN}.
 version_info::Channel GetChannelByName(const std::string& channel);
-
-// Returns whether this is a side-by-side capable copy of Chromium. For
-// unbranded builds, this is always true. For branded builds, this may not be
-// true for old copies of beta and dev channels that share the same user data
-// dir as the stable channel.
-bool IsSideBySideCapable();
 #endif
 
 #if defined(OS_POSIX)

chrome/common/channel_info_mac.mm

@@ -56,33 +56,6 @@ version_info::Channel GetChannelByName(const std::string& channel) {
   return version_info::Channel::UNKNOWN;
 }
 
-bool IsSideBySideCapable() {
-#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-  // Use the main Chrome application bundle and not the framework bundle.
-  // Keystone keys don't live in the framework.
-  NSBundle* bundle = base::mac::OuterBundle();
-  if (![bundle objectForInfoDictionaryKey:@"KSProductID"]) {
-    // This build is not Keystone-enabled, and without a channel assume it is
-    // side-by-side capable.
-    return true;
-  }
-
-  if (![bundle objectForInfoDictionaryKey:@"KSChannelID"]) {
-    // For the stable channel, KSChannelID is not set. Stable Chromes are what
-    // side-by-side capable Chromes are running side-by-side *to* and by
-    // definition are side-by-side capable.
-    return true;
-  }
-
-  // If there is a CrProductDirName key, then the user data dir of this
-  // beta/dev/canary Chrome is separate, and it can run side-by-side to the
-  // stable Chrome.
-  return [bundle objectForInfoDictionaryKey:@"CrProductDirName"];
-#else
-  return true;
-#endif
-}
-
 version_info::Channel GetChannel() {
   return GetChannelByName(GetChannelName());
 }