[wpt] Check ChromeDriver version before accepting found binary (#25657)

This fixes a long standing annoyance where we will blindly accept any
'chromedriver' binary, even if it isn't compatible with the Chrome
version under test. Instead, check that the versions of Chrome and
ChromeDriver mostly match. ChromeDriver is allowed to lag behind by a
release version, to allow for using a local build of Chromium.

See #10451

tools/wpt/browser.py

@@ -656,8 +656,36 @@ def find_binary(self, venv_path=None, channel=None):
         self.logger.warning("Unable to find the browser binary.")
         return None
 
-    def find_webdriver(self, channel=None):
-        return find_executable("chromedriver")
+    def find_webdriver(self, channel=None, browser_binary=None):
+        chromedriver_binary = find_executable("chromedriver")
+        if not chromedriver_binary:
+            return chromedriver_binary
+
+        chromedriver_version = self.webdriver_version(chromedriver_binary)
+        if not chromedriver_version:
+            self.logger.warning(
+                "Unable to get version for ChromeDriver %s, rejecting it" %
+                chromedriver_binary)
+            return None
+
+        if not browser_binary:
+            browser_binary = self.find_binary(channel)
+        browser_version = self.version(browser_binary)
+        if not browser_version:
+            # If we can't get the browser version, we just have to assume the
+            # ChromeDriver is good.
+            return chromedriver_binary
+
+        # Check that the ChromeDriver version matches the Chrome version.
+        chromedriver_major = chromedriver_version.split('.')[0]
+        browser_major = browser_version.split('.')[0]
+        if chromedriver_major != browser_major:
+            self.logger.warning(
+                "Found ChromeDriver %s; does not match Chrome/Chromium %s" %
+                (chromedriver_version, browser_version))
+            return None
+
+        return chromedriver_binary
 
     def _official_chromedriver_url(self, chrome_version):
         # http://chromedriver.chromium.org/downloads/version-selection
@@ -700,6 +728,17 @@ def _latest_chromedriver_url(self, chrome_version):
     def install_webdriver_by_version(self, version, dest=None):
         if dest is None:
             dest = os.pwd
+
+        # There may be an existing chromedriver binary from a previous install.
+        # To provide a clean install experience, remove the old binary - this
+        # avoids tricky issues like unzipping over a read-only file.
+        existing_binary_path = find_executable("chromedriver", dest)
+        if existing_binary_path:
+            self.logger.info("Removing existing ChromeDriver binary: %s" %
+                existing_binary_path)
+            os.chmod(existing_binary_path, stat.S_IWUSR)
+            os.remove(existing_binary_path)
+
         url = self._latest_chromedriver_url(version) if version \
             else self._chromium_chromedriver_url(None)
         self.logger.info("Downloading ChromeDriver from %s" % url)
@@ -749,6 +788,21 @@ def version(self, binary=None, webdriver_binary=None):
             return None
         return m.group(1)
 
+    def webdriver_version(self, webdriver_binary):
+        if uname[0] == "Windows":
+            return _get_fileversion(webdriver_binary, self.logger)
+
+        try:
+            version_string = call(webdriver_binary, "--version").strip()
+        except subprocess.CalledProcessError:
+            self.logger.warning("Failed to call %s" % webdriver_binary)
+            return None
+        m = re.match(r"ChromeDriver ([0-9][0-9.]*)", version_string)
+        if not m:
+            self.logger.warning("Failed to extract version from: %s" % version_string)
+            return None
+        return m.group(1)
+
 
 class ChromeAndroidBase(Browser):
     """A base class for ChromeAndroid and AndroidWebView.

tools/wpt/run.py

@@ -334,16 +334,19 @@ def setup_kwargs(self, kwargs):
                 logger.info("MojoJS enabled")
             except Exception as e:
                 logger.error("Cannot enable MojoJS: %s" % e)
+
         if kwargs["webdriver_binary"] is None:
             webdriver_binary = None
             if not kwargs["install_webdriver"]:
-                webdriver_binary = self.browser.find_webdriver()
+                webdriver_binary = self.browser.find_webdriver(
+                    channel=kwargs["browser_channel"],
+                    browser_binary=kwargs["binary"]
+                )
 
             if webdriver_binary is None:
                 install = self.prompt_install("chromedriver")
 
                 if install:
-                    logger.info("Downloading chromedriver")
                     webdriver_binary = self.browser.install_webdriver(
                         dest=self.venv.bin_path,
                         channel=browser_channel,
@@ -355,7 +358,7 @@ def setup_kwargs(self, kwargs):
             if webdriver_binary:
                 kwargs["webdriver_binary"] = webdriver_binary
             else:
-                raise WptrunError("Unable to locate or install chromedriver binary")
+                raise WptrunError("Unable to locate or install matching ChromeDriver binary")
         if browser_channel in self.experimental_channels:
             logger.info("Automatically turning on experimental features for Chrome Dev/Canary or Chromium trunk")
             kwargs["binary_args"].append("--enable-experimental-web-platform-features")

tools/wpt/tests/test_browser.py

@@ -24,6 +24,68 @@ def test_all_browser_abc():
             assert not inspect.isabstract(cls), "%s is abstract" % name
 
 
+@mock.patch('tools.wpt.browser.find_executable')
+def test_chrome_find_webdriver(mocked_find_executable):
+    # Cannot find ChromeDriver
+    chrome = browser.Chrome(logger)
+    mocked_find_executable.return_value = None
+    assert chrome.find_webdriver() is None
+
+    # ChromeDriver binary cannot be called.
+    chrome = browser.Chrome(logger)
+    mocked_find_executable.return_value = '/usr/bin/chromedriver'
+    chrome.webdriver_version = mock.MagicMock(return_value=None)
+    assert chrome.find_webdriver() is None
+
+    # Browser binary cannot be called.
+    chrome = browser.Chrome(logger)
+    mocked_find_executable.return_value = '/usr/bin/chromedriver'
+    chrome.webdriver_version = mock.MagicMock(return_value='70.0.1')
+    chrome.version = mock.MagicMock(return_value=None)
+    assert chrome.find_webdriver(browser_binary='/usr/bin/chrome') == '/usr/bin/chromedriver'
+
+    # Browser version matches.
+    chrome = browser.Chrome(logger)
+    mocked_find_executable.return_value = '/usr/bin/chromedriver'
+    chrome.webdriver_version = mock.MagicMock(return_value='70.0.1')
+    chrome.version = mock.MagicMock(return_value='70.1.5')
+    assert chrome.find_webdriver(browser_binary='/usr/bin/chrome') == '/usr/bin/chromedriver'
+
+    # Browser version doesn't match.
+    chrome = browser.Chrome(logger)
+    mocked_find_executable.return_value = '/usr/bin/chromedriver'
+    chrome.webdriver_version = mock.MagicMock(return_value='70.0.1')
+    chrome.version = mock.MagicMock(return_value='69.0.1')
+    assert chrome.find_webdriver(browser_binary='/usr/bin/chrome') is None
+
+
+# On Windows, webdriver_version directly calls _get_fileversion, so there is no
+# logic to test there.
+@pytest.mark.skipif(sys.platform.startswith('win'), reason='just uses _get_fileversion on Windows')
+@mock.patch('tools.wpt.browser.call')
+def test_chrome_webdriver_version(mocked_call):
+    chrome = browser.Chrome(logger)
+    webdriver_binary = '/usr/bin/chromedriver'
+
+    # Working cases.
+    mocked_call.return_value = 'ChromeDriver 84.0.4147.30'
+    assert chrome.webdriver_version(webdriver_binary) == '84.0.4147.30'
+    mocked_call.return_value = 'ChromeDriver 87.0.1 (abcd1234-refs/branch-heads/4147@{#310})'
+    assert chrome.webdriver_version(webdriver_binary) == '87.0.1'
+
+    # Various invalid version strings
+    mocked_call.return_value = 'Chrome 84.0.4147.30 (dev)'
+    assert chrome.webdriver_version(webdriver_binary) is None
+    mocked_call.return_value = 'ChromeDriver New 84.0.4147.30'
+    assert chrome.webdriver_version(webdriver_binary) is None
+    mocked_call.return_value = ''
+    assert chrome.webdriver_version(webdriver_binary) is None
+
+    # The underlying subprocess call throws.
+    mocked_call.side_effect = subprocess.CalledProcessError(5, 'cmd', output='Call failed')
+    assert chrome.webdriver_version(webdriver_binary) is None
+
+
 @mock.patch('subprocess.check_output')
 def test_safari_version(mocked_check_output):
     safari = browser.Safari(logger)