From 872f25dcc6d2225e14ee5d80bef3c8da60129bd9 Mon Sep 17 00:00:00 2001
From: Marshall Greenblatt <magreenblatt@gmail.com>
Date: Thu, 14 Mar 2019 19:49:46 -0400
Subject: [PATCH] cefbuilds: Add support for new version number format (see
 issue #2596)

---
 tools/cefbuilds/cef_html_builder.py      |   8 +-
 tools/cefbuilds/cef_json_builder.py      |  84 ++++--
 tools/cefbuilds/cef_json_builder_test.py | 338 ++++++++++++++---------
 3 files changed, 266 insertions(+), 164 deletions(-)

diff --git a/tools/cefbuilds/cef_html_builder.py b/tools/cefbuilds/cef_html_builder.py
index c6b0687b3..19167019f 100644
--- a/tools/cefbuilds/cef_html_builder.py
+++ b/tools/cefbuilds/cef_html_builder.py
@@ -154,7 +154,13 @@ class cef_html_builder:
 
   @staticmethod
   def _get_cef_source_url(cef_version):
-    branch = cef_version.split('.')[1]
+    if cef_version.find('+chromium') > 0:
+      # New-style CEF version numbers include the Chromium version number.
+      # Example: 74.0.1+g62d140e+chromium-74.0.3729.6
+      chromium_version = cef_version[cef_version.rfind('-') + 1:]
+      branch = chromium_version.split('.')[2]
+    else:
+      branch = cef_version.split('.')[1]
     return 'https://bitbucket.org/chromiumembedded/cef/get/%s.tar.bz2' % branch
 
   @staticmethod
diff --git a/tools/cefbuilds/cef_json_builder.py b/tools/cefbuilds/cef_json_builder.py
index 0b8a1682c..292d46502 100644
--- a/tools/cefbuilds/cef_json_builder.py
+++ b/tools/cefbuilds/cef_json_builder.py
@@ -67,6 +67,18 @@ class cef_json_encoder(json.JSONEncoder):
     return o
 
 
+_chromium_version_regex = '[1-9]{1}[0-9]{1,2}\.0\.[1-9]{1}[0-9]{2,4}\.(0|[1-9]{1}[0-9]{0,2})'
+_cef_hash_regex = 'g[0-9a-f]{7}'
+_cef_number_regex = '[0-9]{1,5}\.[0-9]{1,5}\.[0-9]{1,5}'
+
+# Example: 3.2704.1414.g185cd6c
+_cef_old_version_regex = _cef_number_regex + '\.' + _cef_hash_regex
+# Example: 74.0.1+g62d140e+chromium-74.0.3729.6
+_cef_version_release_regex = _cef_number_regex + '\+' + _cef_hash_regex + '\+chromium\-' + _chromium_version_regex
+# Example: 74.0.0-master.1920+g725ed88+chromium-74.0.3729.0
+_cef_version_dev_regex = _cef_number_regex + '\-\w+\.[0-9]{1,7}\+' + _cef_hash_regex + '\+chromium\-' + _chromium_version_regex
+
+
 class cef_json_builder:
   """ Class used to build the cefbuilds JSON file. """
 
@@ -91,8 +103,11 @@ class cef_json_builder:
   @staticmethod
   def is_valid_version(version):
     """ Returns true if the specified CEF version is fully qualified and valid. """
-    return bool(
-        re.compile('^3.[0-9]{4,5}.[0-9]{4,5}.g[0-9a-f]{7}$').match(version))
+    if version is None:
+      return False
+    return bool(re.compile('^' + _cef_old_version_regex + '$').match(version)) \
+        or bool(re.compile('^' + _cef_version_release_regex + '$').match(version)) \
+        or bool(re.compile('^' + _cef_version_dev_regex + '$').match(version))
 
   @staticmethod
   def is_valid_chromium_version(version):
@@ -100,7 +115,7 @@ class cef_json_builder:
     if version is None:
       return False
     return version == 'master' or \
-        re.compile('^[1-9]{1}[0-9]{1}\.0\.[0-9]{4,5}.[0-9]{1,3}$').match(version)
+        bool(re.compile('^' + _chromium_version_regex + '$').match(version))
 
   @staticmethod
   def get_file_name(version, platform, type):
@@ -143,6 +158,36 @@ class cef_json_builder:
     """ Returns the number of queries sent while building. """
     return self._queryct
 
+  def _query_chromium_version(self, cef_version):
+    """ Try to remotely query the Chromium version for old-style CEF version
+        numbers. """
+    chromium_version = 'master'
+    git_hash = cef_version[-7:]
+    query_url = 'https://bitbucket.org/chromiumembedded/cef/raw/%s/CHROMIUM_BUILD_COMPATIBILITY.txt' % git_hash
+    self._queryct = self._queryct + 1
+    if not self._silent:
+      print 'Reading %s' % query_url
+
+    try:
+      # Read the remote URL contents.
+      handle = urllib.urlopen(query_url)
+      compat_value = handle.read().strip()
+      handle.close()
+
+      # Parse the contents.
+      config = eval(compat_value, {'__builtins__': None}, None)
+      if not 'chromium_checkout' in config:
+        raise Exception('Unexpected contents')
+
+      val = config['chromium_checkout']
+      if val.find('refs/tags/') == 0:
+        chromium_version = val[10:]
+    except Exception, e:
+      print 'Failed to read Chromium version information'
+      raise
+
+    return chromium_version
+
   def set_chromium_version(self, cef_version, chromium_version=None):
     """ Set the matching Chromium version. If the specified Chromium version is
         invalid then it will be queried remotely. """
@@ -154,31 +199,12 @@ class cef_json_builder:
         # Keep the Chromium version that we already know about.
         return self._versions[cef_version]
 
-      # Try to identify the Chromium version.
-      chromium_version = 'master'
-      git_hash = cef_version[-7:]
-      query_url = 'https://bitbucket.org/chromiumembedded/cef/raw/%s/CHROMIUM_BUILD_COMPATIBILITY.txt' % git_hash
-      self._queryct = self._queryct + 1
-      if not self._silent:
-        print 'Reading %s' % query_url
-
-      try:
-        # Read the remote URL contents.
-        handle = urllib.urlopen(query_url)
-        compat_value = handle.read().strip()
-        handle.close()
-
-        # Parse the contents.
-        config = eval(compat_value, {'__builtins__': None}, None)
-        if not 'chromium_checkout' in config:
-          raise Exception('Unexpected contents')
-
-        val = config['chromium_checkout']
-        if val.find('refs/tags/') == 0:
-          chromium_version = val[10:]
-      except Exception, e:
-        print 'Failed to read Chromium version information'
-        raise
+      if cef_version.find('+chromium') > 0:
+        # New-style CEF version numbers include the Chromium version number.
+        # Example: 74.0.1+g62d140e+chromium-74.0.3729.6
+        chromium_version = cef_version[cef_version.rfind('-') + 1:]
+      else:
+        chromium_version = self._query_chromium_version(cef_version)
 
     if not self.is_valid_chromium_version(chromium_version):
       raise Exception('Invalid Chromium version: %s' % chromium_version)
@@ -188,7 +214,7 @@ class cef_json_builder:
 
   def get_chromium_version(self, cef_version):
     """ Return the matching Chromium version. If not currently known it will
-        be queried remotely. """
+        be parsed from the CEF version or queried remotely. """
     if cef_version in self._versions:
       return self._versions[cef_version]
     # Identify the Chromium version.
diff --git a/tools/cefbuilds/cef_json_builder_test.py b/tools/cefbuilds/cef_json_builder_test.py
index e01dc15e9..87a9b2cfb 100644
--- a/tools/cefbuilds/cef_json_builder_test.py
+++ b/tools/cefbuilds/cef_json_builder_test.py
@@ -9,6 +9,59 @@ import unittest
 
 class TestCefJSONBuilder(unittest.TestCase):
 
+  # Test CEF version number matching.
+  def test_valid_version(self):
+    # Old-style version numbers.
+    self.assertTrue(cef_json_builder.is_valid_version('3.2704.1414.g185cd6c'))
+    # New-style version numbers.
+    self.assertTrue(
+        cef_json_builder.is_valid_version(
+            '74.0.1+g62d140e+chromium-74.0.3729.6'))
+    self.assertTrue(
+        cef_json_builder.is_valid_version(
+            '74.0.0-master.1920+g725ed88+chromium-74.0.3729.0'))
+    self.assertTrue(
+        cef_json_builder.is_valid_version(
+            '74.0.0-my_branch.1920+g725ed88+chromium-74.0.3729.0'))
+
+    # Must be a completely qualified version number.
+    self.assertFalse(cef_json_builder.is_valid_version(None))
+    self.assertFalse(cef_json_builder.is_valid_version('foobar'))
+    self.assertFalse(cef_json_builder.is_valid_version('3.2704.1414'))
+    self.assertFalse(cef_json_builder.is_valid_version('74.0.1+g62d140e'))
+    self.assertFalse(
+        cef_json_builder.is_valid_version('74.0.0-master.1920+g725ed88'))
+    self.assertFalse(
+        cef_json_builder.is_valid_version('74.0.0+chromium-74.0.3729.0'))
+
+  # Test Chromium version number matching.
+  def test_valid_chromium_version(self):
+    self.assertTrue(cef_json_builder.is_valid_chromium_version('74.0.3729.0'))
+    self.assertTrue(cef_json_builder.is_valid_chromium_version('74.0.3729.6'))
+    self.assertTrue(
+        cef_json_builder.is_valid_chromium_version('100.0.59200.602'))
+
+    # self.assertFalse(cef_json_builder.is_valid_version(None))
+    self.assertFalse(cef_json_builder.is_valid_chromium_version('foobar'))
+    # Must be 4 numbers.
+    self.assertFalse(cef_json_builder.is_valid_chromium_version('74.0.3729'))
+    self.assertFalse(cef_json_builder.is_valid_chromium_version('74.0.3729.A'))
+    # 1st number is max 3 digits with no leading 0's.
+    self.assertFalse(
+        cef_json_builder.is_valid_chromium_version('7400.0.3729.6'))
+    self.assertFalse(cef_json_builder.is_valid_chromium_version('04.0.3729.6'))
+    # 2nd number is always 0.
+    self.assertFalse(cef_json_builder.is_valid_chromium_version('74.1.3729.6'))
+    # 3rd number is max 5 digits with no leading 0's.
+    self.assertFalse(cef_json_builder.is_valid_chromium_version('74.0.0372.6'))
+    self.assertFalse(
+        cef_json_builder.is_valid_chromium_version('74.0.372900.6'))
+    # 4rd number is max 3 digits with no leading 0's.
+    self.assertFalse(
+        cef_json_builder.is_valid_chromium_version('74.0.3729.6000'))
+    self.assertFalse(cef_json_builder.is_valid_chromium_version('74.0.3729.06'))
+    pass
+
   # Write builder contents to string and then read in.
   def _verify_write_read(self, builder):
     output = str(builder)
@@ -105,60 +158,65 @@ class TestCefJSONBuilder(unittest.TestCase):
 
   # Test get_files() behavior with a single file.
   def test_get_files_single(self):
-    builder = cef_json_builder()
+    # yapf: disable
+    versions = (
+        ('3.2704.1414.g185cd6c', '3.2704'),
+        ('74.0.1+g62d140e+chromium-74.0.3729.6', '74.0'),
+    )
+    # yapf: enable
 
-    # Specify all values just in case the defaults change.
-    expected = self._add_test_file(
-        builder,
-        platform='linux32',
-        version='3.2704.1414.g185cd6c',
-        type='standard')
+    for version, partial_version in versions:
+      builder = cef_json_builder()
 
-    # No filter.
-    files = builder.get_files()
-    self.assertEqual(len(files), 1)
-    self.assertEqual(expected, files[0])
+      # Specify all values just in case the defaults change.
+      expected = self._add_test_file(
+          builder, platform='linux32', version=version, type='standard')
 
-    # Platform filter.
-    files = builder.get_files(platform='linux32')
-    self.assertEqual(len(files), 1)
-    self.assertEqual(expected, files[0])
-    files = builder.get_files(platform='linux64')
-    self.assertEqual(len(files), 0)
+      # No filter.
+      files = builder.get_files()
+      self.assertEqual(len(files), 1)
+      self.assertEqual(expected, files[0])
 
-    # Version filter exact.
-    files = builder.get_files(version='3.2704.1414.g185cd6c')
-    self.assertEqual(len(files), 1)
-    self.assertEqual(expected, files[0])
+      # Platform filter.
+      files = builder.get_files(platform='linux32')
+      self.assertEqual(len(files), 1)
+      self.assertEqual(expected, files[0])
+      files = builder.get_files(platform='linux64')
+      self.assertEqual(len(files), 0)
 
-    # Version filter partial.
-    files = builder.get_files(version='3.2704')
-    self.assertEqual(len(files), 1)
-    self.assertEqual(expected, files[0])
-    files = builder.get_files(version='3.2623')
-    self.assertEqual(len(files), 0)
+      # Version filter exact.
+      files = builder.get_files(version=version)
+      self.assertEqual(len(files), 1)
+      self.assertEqual(expected, files[0])
 
-    # Type filter.
-    files = builder.get_files(type='standard')
-    self.assertEqual(len(files), 1)
-    self.assertEqual(expected, files[0])
-    files = builder.get_files(type='client')
-    self.assertEqual(len(files), 0)
+      # Version filter partial.
+      files = builder.get_files(version=partial_version)
+      self.assertEqual(len(files), 1)
+      self.assertEqual(expected, files[0])
+      files = builder.get_files(version='3.2623')
+      self.assertEqual(len(files), 0)
 
-    # All filters.
-    files = builder.get_files(
-        platform='linux32', version='3.2704', type='standard')
-    self.assertEqual(len(files), 1)
-    self.assertEqual(expected, files[0])
-    files = builder.get_files(
-        platform='linux32', version='3.2704', type='minimal')
-    self.assertEqual(len(files), 0)
-    files = builder.get_files(
-        platform='linux32', version='3.2623', type='standard')
-    self.assertEqual(len(files), 0)
-    files = builder.get_files(
-        platform='linux64', version='3.2704', type='standard')
-    self.assertEqual(len(files), 0)
+      # Type filter.
+      files = builder.get_files(type='standard')
+      self.assertEqual(len(files), 1)
+      self.assertEqual(expected, files[0])
+      files = builder.get_files(type='client')
+      self.assertEqual(len(files), 0)
+
+      # All filters.
+      files = builder.get_files(
+          platform='linux32', version=partial_version, type='standard')
+      self.assertEqual(len(files), 1)
+      self.assertEqual(expected, files[0])
+      files = builder.get_files(
+          platform='linux32', version=partial_version, type='minimal')
+      self.assertEqual(len(files), 0)
+      files = builder.get_files(
+          platform='linux32', version='3.2623', type='standard')
+      self.assertEqual(len(files), 0)
+      files = builder.get_files(
+          platform='linux64', version=partial_version, type='standard')
+      self.assertEqual(len(files), 0)
 
   # Test add/get of multiple files.
   def test_add_multiple_files(self):
@@ -167,7 +225,13 @@ class TestCefJSONBuilder(unittest.TestCase):
     expected = []
 
     platforms = cef_json_builder.get_platforms()
-    versions = ['3.2704.1414.g185cd6c', '3.2704.1400.gde36543']
+    versions = [
+        # Old-style version numbers.
+        '3.2704.1414.g185cd6c',
+        '3.2704.1400.gde36543',
+        # New-style version numbers.
+        '74.0.1+g62d140e+chromium-74.0.3729.6'
+    ]
     types = cef_json_builder.get_distrib_types()
     for platform in platforms:
       for version in versions:
@@ -183,8 +247,9 @@ class TestCefJSONBuilder(unittest.TestCase):
     self.assertEqual(len(files), len(platforms) * len(versions) * len(types))
 
     # Version filter.
-    files = builder.get_files(version=version)
-    self.assertEqual(len(files), len(platforms) * len(types))
+    for version in versions:
+      files = builder.get_files(version=version)
+      self.assertEqual(len(files), len(platforms) * len(types))
 
     # Type filter.
     files = builder.get_files(type='client')
@@ -203,115 +268,119 @@ class TestCefJSONBuilder(unittest.TestCase):
               platform=platform, type=type, version=version)
           self.assertEqual(len(files), 1)
           self.assertEqual(expected[idx], files[0])
-          idx = idx + 1
+          idx += 1
 
   # Test add/get/replace of multiple files.
   def test_replace_all_files(self):
-    builder = cef_json_builder()
-
-    version = '3.2704.1414.g185cd6c'
+    versions = ['3.2704.1414.g185cd6c', '74.0.1+g62d140e+chromium-74.0.3729.6']
     platforms = ['linux32', 'linux64']
     types = ['standard', 'minimal']
 
-    # Initial file versions.
-    for platform in platforms:
-      for type in types:
-        self._add_test_file(
-            builder, platform=platform, type=type, version=version)
+    for version in versions:
+      builder = cef_json_builder()
 
-    # No filter.
-    files = builder.get_files()
-    self.assertEqual(len(files), len(platforms) * len(types))
+      # Initial file versions.
+      for platform in platforms:
+        for type in types:
+          self._add_test_file(
+              builder, platform=platform, type=type, version=version)
 
-    expected = []
+      # No filter.
+      files = builder.get_files()
+      self.assertEqual(len(files), len(platforms) * len(types))
 
-    # Replace all file versions (due to new sha1).
-    for platform in platforms:
-      for type in types:
-        expected.append(
-            self._add_test_file(
-                builder,
-                platform=platform,
-                type=type,
-                version=version,
-                attrib_idx=1))
+      expected = []
 
-    # No filter.
-    files = builder.get_files()
-    self.assertEqual(len(files), len(platforms) * len(types))
+      # Replace all file versions (due to new sha1).
+      for platform in platforms:
+        for type in types:
+          expected.append(
+              self._add_test_file(
+                  builder,
+                  platform=platform,
+                  type=type,
+                  version=version,
+                  attrib_idx=1))
 
-    # All filters.
-    idx = 0
-    for platform in platforms:
-      for type in types:
-        files = builder.get_files(platform=platform, type=type, version=version)
-        self.assertEqual(len(files), 1)
-        self.assertEqual(expected[idx], files[0])
-        idx = idx + 1
+      # No filter.
+      files = builder.get_files()
+      self.assertEqual(len(files), len(platforms) * len(types))
+
+      # All filters.
+      idx = 0
+      for platform in platforms:
+        for type in types:
+          files = builder.get_files(
+              platform=platform, type=type, version=version)
+          self.assertEqual(len(files), 1)
+          self.assertEqual(expected[idx], files[0])
+          idx += 1
 
   # Test add/get/no replace of multiple files.
   def test_replace_no_files(self):
-    builder = cef_json_builder()
-
-    version = '3.2704.1414.g185cd6c'
+    versions = ['3.2704.1414.g185cd6c', '74.0.1+g62d140e+chromium-74.0.3729.6']
     platforms = ['linux32', 'linux64']
     types = ['standard', 'minimal']
 
-    # Initial file versions.
-    for platform in platforms:
-      for type in types:
-        self._add_test_file(
-            builder, platform=platform, type=type, version=version)
+    for version in versions:
+      builder = cef_json_builder()
 
-    # No filter.
-    files = builder.get_files()
-    self.assertEqual(len(files), len(platforms) * len(types))
+      # Initial file versions.
+      for platform in platforms:
+        for type in types:
+          self._add_test_file(
+              builder, platform=platform, type=type, version=version)
 
-    expected = []
+      # No filter.
+      files = builder.get_files()
+      self.assertEqual(len(files), len(platforms) * len(types))
 
-    # Replace no file versions (due to same sha1).
-    for platform in platforms:
-      for type in types:
-        expected.append(
-            self._add_test_file(
-                builder,
-                platform=platform,
-                type=type,
-                version=version,
-                shouldfail=True))
+      expected = []
 
-    # No filter.
-    files = builder.get_files()
-    self.assertEqual(len(files), len(platforms) * len(types))
+      # Replace no file versions (due to same sha1).
+      for platform in platforms:
+        for type in types:
+          expected.append(
+              self._add_test_file(
+                  builder,
+                  platform=platform,
+                  type=type,
+                  version=version,
+                  shouldfail=True))
 
-    # All filters.
-    idx = 0
-    for platform in platforms:
-      for type in types:
-        files = builder.get_files(platform=platform, type=type, version=version)
-        self.assertEqual(len(files), 1)
-        self.assertEqual(expected[idx], files[0])
-        idx = idx + 1
+      # No filter.
+      files = builder.get_files()
+      self.assertEqual(len(files), len(platforms) * len(types))
+
+      # All filters.
+      idx = 0
+      for platform in platforms:
+        for type in types:
+          files = builder.get_files(
+              platform=platform, type=type, version=version)
+          self.assertEqual(len(files), 1)
+          self.assertEqual(expected[idx], files[0])
+          idx += 1
 
   # Test Chromium version.
   def test_chromium_version(self):
     builder = cef_json_builder()
 
-    self.assertTrue(builder.is_valid_chromium_version('master'))
-    self.assertTrue(builder.is_valid_chromium_version('49.0.2704.0'))
-    self.assertTrue(builder.is_valid_chromium_version('49.0.2704.50'))
-    self.assertTrue(builder.is_valid_chromium_version('49.0.2704.100'))
-
-    self.assertFalse(builder.is_valid_chromium_version(None))
-    self.assertFalse(builder.is_valid_chromium_version('09.0.2704.50'))
-    self.assertFalse(builder.is_valid_chromium_version('00.0.0000.00'))
-    self.assertFalse(builder.is_valid_chromium_version('foobar'))
-
-    # The Git hashes must exist but the rest of the CEF version can be fake.
-    versions = (('3.2704.1414.g185cd6c',
-                 '51.0.2704.47'), ('3.2623.9999.gb90a3be', '49.0.2623.110'),
-                ('3.2623.9999.g2a6491b',
-                 '49.0.2623.87'), ('3.9999.9999.gab2636b', 'master'),)
+    # For old-style CEF version numbers the Git hashes must exist but the rest
+    # of the CEF version can be fake.
+    # yapf: disable
+    versions = (
+        # Old-style version numbers.
+        ('3.2704.1414.g185cd6c', '51.0.2704.47'),
+        ('3.2623.9999.gb90a3be', '49.0.2623.110'),
+        ('3.2623.9999.g2a6491b', '49.0.2623.87'),
+        ('3.9999.9999.gab2636b', 'master'),
+        # New-style version numbers.
+        ('74.0.1+g62d140e+chromium-74.0.3729.6', '74.0.3729.6'),
+        ('74.0.0-master.1920+g725ed88+chromium-74.0.3729.0', '74.0.3729.0'),
+        ('74.0.0-my_branch.1920+g725ed88+chromium-74.0.3729.0', '74.0.3729.0'),
+    )
+    # yapf: enable
 
     # Test with no query.
     for (cef, chromium) in versions:
@@ -329,11 +398,12 @@ class TestCefJSONBuilder(unittest.TestCase):
     # Test with query.
     for (cef, chromium) in versions:
       self.assertFalse(builder.has_chromium_version(cef))
-      # No version known, so query sent.
+      # No version known, so query sent for old-style version numbers.
       self.assertEqual(chromium, builder.get_chromium_version(cef))
       self.assertTrue(builder.has_chromium_version(cef))
 
-    self.assertEqual(len(versions), builder.get_query_count())
+    # Only old-style version numbers.
+    self.assertEqual(4, builder.get_query_count())
 
 
 # Program entry point.