Attachment 371020 Details for Bug 198316 – Patch

[patch] Patch

bug-198316-20190530192502.patch (text/plain), 4.91 KB, created by dewei_zhu on 2019-05-30 19:25:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: dewei_zhu

Created: 2019-05-30 19:25:02 PDT

Size: 4.91 KB

patch

obsolete

>Subversion Revision: 245909
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 5fb61730cbc9ce7ef0c0c73f67bf1ca51017481c..d7b5f75fc6dcd1f090544a4411a8abaf03bcdffc 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-05-28  Dewei Zhu  <dewei_zhu@apple.com>
>+
>+        run-benchmark should report an error if the argument to --build-directory is bogus
>+        https://bugs.webkit.org/show_bug.cgi?id=198316
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        'run-benchmark' should not fallback to system safari when browser or browser build path is
>+        specified but not valid.
>+        Add a run-time check to ensure at least one of the resource from build directory is used when
>+        build directory is specified.
>+
>+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:
>+        (OSXSafariDriver.launch_url): Raise an exception when browser or browser build path is
>+        specified but not valid.
>+
> 2019-05-30  Keith Miller  <keith_miller@apple.com>
> 
>         IsoHeaps don't notice uncommitted VA becoming the first eligible.
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py
>index 7b26e693a9a11b519ac8e37b08bdafbba1406681..dbf594e3defa558fe00260bd3a67d6b1295aeb5c 100755
>--- a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py
>@@ -1,5 +1,6 @@
> #!/usr/bin/env python
> 
>+import itertools
> import logging
> import os
> import subprocess
>@@ -27,18 +28,26 @@ class OSXSafariDriver(OSXBrowserDriver):
>         args = ['/Applications/Safari.app/Contents/MacOS/Safari']
>         env = {}
>         if browser_build_path:
>-            safari_app_in_build_path = os.path.join(browser_build_path, 'Safari.app/Contents/MacOS/Safari')
>-            if os.path.exists(safari_app_in_build_path):
>+            browser_build_absolute_path = os.path.abspath(browser_build_path)
>+            safari_app_in_build_path = os.path.join(browser_build_absolute_path, 'Safari.app/Contents/MacOS/Safari')
>+            has_safari_app = os.path.exists(safari_app_in_build_path)
>+            content_in_path = os.listdir(browser_build_absolute_path)
>+            contains_frameworks = any(itertools.imap(lambda entry: entry.endswith('.framework'), os.listdir(browser_build_absolute_path)))
>+
>+            if has_safari_app:
>                 args = [safari_app_in_build_path]
>-                env = {'DYLD_FRAMEWORK_PATH': browser_build_path, 'DYLD_LIBRARY_PATH': browser_build_path, '__XPC_DYLD_FRAMEWORK_PATH': browser_build_path, '__XPC_DYLD_LIBRARY_PATH': browser_build_path}
>-            else:
>-                _log.info('Could not find Safari.app at %s, using the system Safari instead' % safari_app_in_build_path)
>+
>+            if contains_frameworks:
>+                env = {'DYLD_FRAMEWORK_PATH': browser_build_absolute_path, 'DYLD_LIBRARY_PATH': browser_build_absolute_path, '__XPC_DYLD_FRAMEWORK_PATH': browser_build_absolute_path, '__XPC_DYLD_LIBRARY_PATH': browser_build_absolute_path}
>+            elif not has_safari_app:
>+                raise Exception('Could not find any framework "{}"'.format(browser_build_path))
>+
>         elif browser_path:
>             safari_app_in_browser_path = os.path.join(browser_path, 'Contents/MacOS/Safari')
>             if os.path.exists(safari_app_in_browser_path):
>                 args = [safari_app_in_browser_path]
>             else:
>-                _log.info('Could not find application at %s, using the system Safari instead' % safari_app_in_browser_path)
>+                raise Exception('Could not find Safari.app at {}'.format(safari_app_in_browser_path))
> 
>         args.extend(self._safari_preferences)
>         _log.info('Launching safari: %s with url: %s' % (args[0], url))
>@@ -47,6 +56,18 @@ class OSXSafariDriver(OSXBrowserDriver):
>         # Stop for initialization of the safari process, otherwise, open
>         # command may use the system safari.
>         time.sleep(3)
>+
>+        if browser_build_path:
>+            _log.info('Checking if any open file is from "{}".'.format(browser_build_path))
>+            # Cannot use 'check_call' here as '+D' option will have non-zero return code when not all files under
>+            # specified folder are used.
>+            process = subprocess.Popen(['/usr/sbin/lsof', '-a', '-p', str(self._safari_process.pid), '+D', browser_build_absolute_path], stdout=subprocess.PIPE)
>+            output = process.communicate()[0]
>+            if has_safari_app:
>+                assert 'Safari.app/Contents/MacOS/Safari' in output, 'Safari.app is not launched from "{}"'.format(browser_build_path)
>+            if contains_frameworks:
>+                assert '.framework' in output, 'No framework is loaded from "{}"'.format(browser_build_path)
>+
>         subprocess.Popen(['open', '-a', args[0], url])
> 
>     def launch_driver(self, url, options, browser_build_path):

Flags:

rniwa: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 198316: 370810 | 370991 | 371020 | 371038