mac-arm64: Put arm64 into the tag visible to Keystone and Omaha

In order for the update service to distinguish between
single-architecture x86_64 and arm64 builds as installed, the
architecture should be present in the tag that the update client
presents.

On its own, Keystone reports the architecture of the system it’s running
on, but for cases where the installed version may be native to the
system (an arm64 system running x86_64 Chrome under Rosetta
translation), where it may be preferable to retain the existing build
type for a period of time under our control, the architecture of the
installed product must also be reported. This information can also be
used to detect when a non-native version of Chrome is installed so that,
again under our control, we can direct an update to a more suitable
native version as appropriate.

This makes no changes to the tags used for single-architecture
mac-x86_64 builds. Whether this is advisable long-term is another
question, but for now, the prudent thing to do is to maintain the status
quo for the vast existing installed base. Any changes to that strategy
must be coordinated with the server-side configuration.

This changes the tags used for single-architecture mac-arm64 builds to
contain the “arm64” string, like “arm64”, “arm64-dev”, and “arm64-full”.

Bug: 1116601
Change-Id: I8e9dbc41285ea37018ebfd4e50b41bee985d9c8c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2496200Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820850}

build/apple/tweak_info_plist.py

@@ -189,7 +189,7 @@ def _TagSuffixes():
   return tag_suffixes
 
 
-def _AddKeystoneKeys(plist, bundle_identifier):
+def _AddKeystoneKeys(plist, bundle_identifier, base_tag):
   """Adds the Keystone keys. This must be called AFTER _AddVersionKeys() and
   also requires the |bundle_identifier| argument (com.example.product)."""
   plist['KSVersion'] = plist['CFBundleShortVersionString']
@@ -197,16 +197,18 @@ def _AddKeystoneKeys(plist, bundle_identifier):
   plist['KSUpdateURL'] = 'https://tools.google.com/service/update2'
 
   _RemoveKeys(plist, 'KSChannelID')
+  if base_tag != '':
+    plist['KSChannelID'] = base_tag
   for tag_suffix in _TagSuffixes():
     if tag_suffix:
-      plist['KSChannelID' + tag_suffix] = tag_suffix
+      plist['KSChannelID' + tag_suffix] = base_tag + tag_suffix
 
 
 def _RemoveKeystoneKeys(plist):
   """Removes any set Keystone keys."""
   _RemoveKeys(plist, 'KSVersion', 'KSProductID', 'KSUpdateURL')
 
-  tag_keys = []
+  tag_keys = ['KSChannelID']
   for tag_suffix in _TagSuffixes():
     tag_keys.append('KSChannelID' + tag_suffix)
   _RemoveKeys(plist, *tag_keys)
@@ -241,6 +243,9 @@ def Main(argv):
                     type='int',
                     default=False,
                     help='Enable Keystone [1 or 0]')
+  parser.add_option('--keystone-base-tag',
+                    default='',
+                    help='Base Keystone tag to set')
   parser.add_option('--scm',
                     dest='add_scm_info',
                     action='store',
@@ -378,7 +383,8 @@ def Main(argv):
     if options.bundle_identifier is None:
       print('Use of Keystone requires the bundle id.', file=sys.stderr)
       return 1
-    _AddKeystoneKeys(plist, options.bundle_identifier)
+    _AddKeystoneKeys(plist, options.bundle_identifier,
+                     options.keystone_base_tag)
   else:
     _RemoveKeystoneKeys(plist)
 

chrome/BUILD.gn

@@ -404,16 +404,19 @@ if (is_win) {
 
   tweak_info_plist("chrome_app_plist") {
     info_plist = "app/app-Info.plist"
-    _keystone_arg = "0"
-    if (is_chrome_branded) {
-      _keystone_arg = "1"
-    }
     args = [
       "--breakpad=0",
-      "--keystone=$_keystone_arg",
       "--scm=1",
       "--bundle_id=$chrome_mac_bundle_id",
     ]
+    if (is_chrome_branded) {
+      args += [ "--keystone=1" ]
+      if (current_cpu == "arm64") {
+        args += [ "--keystone-base-tag=arm64" ]
+      }
+    } else {
+      args += [ "--keystone=0" ]
+    }
   }
 
   mac_app_bundle("chrome_app") {

chrome/installer/mac/signing/modification.py

@@ -59,8 +59,15 @@ def _modify_plists(paths, dist, config):
         elif _KS_BRAND_ID in app_plist:
             del app_plist[_KS_BRAND_ID]
 
+        base_tag = app_plist.get(_KS_CHANNEL_ID)
+        base_channel_tag_components = []
+        if base_tag:
+            base_channel_tag_components.append(base_tag)
         if dist.channel:
-            app_plist[_KS_CHANNEL_ID] = dist.channel
+            base_channel_tag_components.append(dist.channel)
+        base_channel_tag = '-'.join(base_channel_tag_components)
+        if base_channel_tag:
+            app_plist[_KS_CHANNEL_ID] = base_channel_tag
         elif _KS_CHANNEL_ID in app_plist:
             del app_plist[_KS_CHANNEL_ID]
 
@@ -75,9 +82,8 @@ def _modify_plists(paths, dist, config):
         for key in app_plist.keys():
             if not key.startswith(_KS_CHANNEL_ID + '-'):
                 continue
-            orig_channel, tag = key.split('-')
-            channel_str = dist.channel if dist.channel else ''
-            app_plist[key] = '{}-{}'.format(channel_str, tag)
+            ignore, extra = key.split('-')
+            app_plist[key] = '{}-{}'.format(base_channel_tag, extra)
 
 
 def _replace_icons(paths, dist, config):

chrome/installer/mac/signing/modification_test.py

@@ -39,6 +39,12 @@ def plist_read(*args):
     return plists[args[0]]
 
 
+def plist_read_with_architecture(*args):
+    plist = plist_read(*args)
+    plist.update({'KSChannelID': 'arm64', 'KSChannelID-full': 'arm64-full'})
+    return plist
+
+
 @mock.patch('signing.commands.plistlib',
             **{'readPlist.side_effect': plist_read})
 @mock.patch.multiple(
@@ -63,9 +69,9 @@ class TestModification(unittest.TestCase):
                 for tup in call:
                     for arg in tup:
                         # Don't anchor this substring in a particular directory
-                        # because it may appear in any of /$I, /$O, or /$W. Don't
-                        # anchor it with App Product.app either, because it may
-                        # be renamed (to App Product Canary.app).
+                        # because it may appear in any of /$I, /$O, or /$W.
+                        # Don't anchor it with App Product.app either, because
+                        # it may be renamed (to App Product Canary.app).
                         if 'Contents/Frameworks/Product Framework.framework' in arg:
                             return True
 
@@ -114,6 +120,43 @@ class TestModification(unittest.TestCase):
 
         self.assertTrue(self._is_framework_unchanged(plistlib, kwargs))
 
+    def test_distribution_with_architecture(self, plistlib, **kwargs):
+        plistlib.readPlist.side_effect = plist_read_with_architecture
+
+        dist = model.Distribution()
+        config = dist.to_config(self.config)
+
+        modification.customize_distribution(self.paths, dist, config)
+
+        self.assertEqual(1, plistlib.writePlist.call_count)
+        plistlib.writePlist.assert_called_with(
+            {
+                'CFBundleIdentifier': config.base_bundle_id,
+                'CFBundleName': 'Product',
+                'KSProductID': 'test.ksproduct',
+                'KSChannelID': 'arm64',
+                'KSChannelID-full': 'arm64-full'
+            },
+            '/$W/App Product.app/Contents/Info.plist',
+        )
+
+        self.assertEqual(4, kwargs['copy_files'].call_count)
+        kwargs['copy_files'].assert_has_calls([
+            mock.call('/$I/Product Packaging/app-entitlements.plist',
+                      '/$W/app-entitlements.plist'),
+            mock.call('/$I/Product Packaging/helper-gpu-entitlements.plist',
+                      '/$W/helper-gpu-entitlements.plist'),
+            mock.call('/$I/Product Packaging/helper-plugin-entitlements.plist',
+                      '/$W/helper-plugin-entitlements.plist'),
+            mock.call(
+                '/$I/Product Packaging/helper-renderer-entitlements.plist',
+                '/$W/helper-renderer-entitlements.plist'),
+        ])
+        self.assertEqual(0, kwargs['move_file'].call_count)
+        self.assertEqual(0, kwargs['write_file'].call_count)
+
+        self.assertTrue(self._is_framework_unchanged(plistlib, kwargs))
+
     def test_distribution_with_brand(self, plistlib, **kwargs):
         dist = model.Distribution(branding_code='MOO')
         config = dist.to_config(self.config)
@@ -183,6 +226,44 @@ class TestModification(unittest.TestCase):
 
         self.assertTrue(self._is_framework_unchanged(plistlib, kwargs))
 
+    def test_distribution_with_architecture_and_channel(self, plistlib,
+                                                        **kwargs):
+        plistlib.readPlist.side_effect = plist_read_with_architecture
+
+        dist = model.Distribution(channel='dev')
+        config = dist.to_config(self.config)
+
+        modification.customize_distribution(self.paths, dist, config)
+
+        self.assertEqual(1, plistlib.writePlist.call_count)
+        plistlib.writePlist.assert_called_with(
+            {
+                'CFBundleIdentifier': config.base_bundle_id,
+                'CFBundleName': 'Product',
+                'KSProductID': 'test.ksproduct',
+                'KSChannelID': 'arm64-dev',
+                'KSChannelID-full': 'arm64-dev-full'
+            },
+            '/$W/App Product.app/Contents/Info.plist',
+        )
+
+        self.assertEqual(4, kwargs['copy_files'].call_count)
+        kwargs['copy_files'].assert_has_calls([
+            mock.call('/$I/Product Packaging/app-entitlements.plist',
+                      '/$W/app-entitlements.plist'),
+            mock.call('/$I/Product Packaging/helper-gpu-entitlements.plist',
+                      '/$W/helper-gpu-entitlements.plist'),
+            mock.call('/$I/Product Packaging/helper-plugin-entitlements.plist',
+                      '/$W/helper-plugin-entitlements.plist'),
+            mock.call(
+                '/$I/Product Packaging/helper-renderer-entitlements.plist',
+                '/$W/helper-renderer-entitlements.plist'),
+        ])
+        self.assertEqual(0, kwargs['move_file'].call_count)
+        self.assertEqual(0, kwargs['write_file'].call_count)
+
+        self.assertTrue(self._is_framework_unchanged(plistlib, kwargs))
+
     def test_distribution_with_product_dirname(self, plistlib, **kwargs):
         dist = model.Distribution(product_dirname='Farmland/Cows')
         config = dist.to_config(self.config)