Attachment 357763 Details for Bug 192905 – Patch

[patch] Patch

bug-192905-20181219171603.patch (text/plain), 14.95 KB, created by Simon Fraser (smfr) on 2018-12-19 17:16:04 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2018-12-19 17:16:04 PST

Size: 14.95 KB

patch

obsolete

>Subversion Revision: 239397
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 29fb744c6d9cea64afaac21ff95a910e33bd7f4f..59cdd5ba076ee03768b68671022a3055906046ea 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,39 @@
>+2018-12-19  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Add support to run-benchmark to use non-default copies of the browser apps
>+        https://bugs.webkit.org/show_bug.cgi?id=192905
>+        rdar://problem/46845840
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Add support for a --browser-path argument to run-benchmark, which allows you to use
>+        a custom app bundle for a given browser (only implemented for Safari at present).
>+
>+        * Scripts/webkitpy/benchmark_runner/benchmark_runner.py:
>+        (BenchmarkRunner.__init__):
>+        * Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py:
>+        (BrowserDriver.__init__):
>+        * Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver_factory.py:
>+        (BrowserDriverFactory.create):
>+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py:
>+        (OSXBrowserDriver._launch_process):
>+        (OSXBrowserDriver._launch_process_with_caffeinate): Fix spelling.
>+        (OSXBrowserDriver._launch_process_with_caffinate): Deleted.
>+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:
>+        (OSXChromeCanaryDriver.launch_url):
>+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py:
>+        (OSXFirefoxNightlyDriver.launch_url):
>+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:
>+        (OSXSafariDriver.__init__):
>+        (OSXSafariDriver.launch_url):
>+        * Scripts/webkitpy/benchmark_runner/run_benchmark.py:
>+        (parse_args):
>+        (run_benchmark_plan):
>+        * Scripts/webkitpy/benchmark_runner/webserver_benchmark_runner.py:
>+        (WebServerBenchmarkRunner.__init__):
>+        * Scripts/webkitpy/browserperfdash/browserperfdash_unittest.py:
>+        (FakeBenchmarkRunner.__init__):
>+
> 2018-12-19  Megan Gardner  <megan_gardner@apple.com>
> 
>         Allow clients to set the navigator platform
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py b/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py
>old mode 100644
>new mode 100755
>index 75e1c64c742a00404a77d6b6dde500583b9a888d..5876d6545215271812b0133d82f308f00a193ebb
>--- a/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py
>@@ -23,7 +23,7 @@ _log = logging.getLogger(__name__)
> class BenchmarkRunner(object):
>     name = 'benchmark_runner'
> 
>-    def __init__(self, plan_file, local_copy, count_override, build_dir, output_file, platform, browser, scale_unit=True, show_iteration_values=False, device_id=None):
>+    def __init__(self, plan_file, local_copy, count_override, build_dir, output_file, platform, browser, browser_path, scale_unit=True, show_iteration_values=False, device_id=None):
>         try:
>             plan_file = self._find_plan_file(plan_file)
>             with open(plan_file, 'r') as fp:
>@@ -35,7 +35,7 @@ class BenchmarkRunner(object):
>                     self._plan['local_copy'] = local_copy
>                 if count_override:
>                     self._plan['count'] = count_override
>-                self._browser_driver = BrowserDriverFactory.create(platform, browser)
>+                self._browser_driver = BrowserDriverFactory.create(platform, browser, browser_path)
>                 self._build_dir = os.path.abspath(build_dir) if build_dir else None
>                 self._output_file = output_file
>                 self._scale_unit = scale_unit
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py
>old mode 100644
>new mode 100755
>index ffa2c688cb83e256c69aaf7d665d3fe6317ba827..5db4768a31481cb57732b86dd3af8c187d94ea8b
>--- a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py
>@@ -10,6 +10,9 @@ class BrowserDriver(object):
> 
>     ___metaclass___ = ABCMeta
> 
>+    def __init__(self, browser_path):
>+        self._browser_path = browser_path
>+
>     @abstractmethod
>     def prepare_env(self, config):
>         pass
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver_factory.py b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver_factory.py
>old mode 100644
>new mode 100755
>index b27fd7e29f8dd7125d7935128c854d7930d32e7e..215759eea0b44dd246d62efb2043ff4c91ec898e
>--- a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver_factory.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver_factory.py
>@@ -28,7 +28,7 @@ class BrowserDriverFactory(object):
>         cls.browser_drivers[platform][browser_name] = browser_driver_class
> 
>     @classmethod
>-    def create(cls, platform, browser_name):
>+    def create(cls, platform, browser_name, browser_path):
>         if browser_name not in cls.browser_drivers[platform]:
>             raise ValueError("Browser \"%s\" is not available on platform \"%s\"" % (browser_name, platform))
>-        return cls.browser_drivers[platform][browser_name]()
>+        return cls.browser_drivers[platform][browser_name](browser_path)
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py
>index 505c2210327cce9dd73da1b825a04385f9015743..9095931c3465b23b331673feecf97c82b2d662aa 100644
>--- a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py
>@@ -40,7 +40,7 @@ class OSXBrowserDriver(BrowserDriver):
> 
>         # FIXME: May need to be modified for a local build such as setting up DYLD libraries
>         args = ['open', '-a', app_path] + args
>-        cls._launch_process_with_caffinate(args)
>+        cls._launch_process_with_caffeinate(args)
> 
>     @classmethod
>     def _launch_webdriver(cls, url, driver):
>@@ -57,8 +57,13 @@ class OSXBrowserDriver(BrowserDriver):
>         subprocess.call(['/usr/bin/killall', process_name])
> 
>     @classmethod
>-    def _launch_process_with_caffinate(cls, args, env=None):
>-        process = subprocess.Popen(args, env=env)
>+    def _launch_process_with_caffeinate(cls, args, env=None):
>+        try:
>+            process = subprocess.Popen(args, env=env)
>+        except Exception as error:
>+            _log.error('Popen failed: {error}'.format(error=error))
>+            return
>+
>         subprocess.Popen(["/usr/bin/caffeinate", "-disw", str(process.pid)])
>         return process
> 
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py
>old mode 100644
>new mode 100755
>index 43b3c7ee20145bf0aaa1052eb0e92f913ce36577..0c94192c1f66bd712c2c484cc0652a7c98860f36
>--- a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py
>@@ -38,6 +38,7 @@ class OSXChromeCanaryDriver(OSXBrowserDriver):
>     app_name = 'Google Chrome Canary.app'
> 
>     def launch_url(self, url, options, browser_build_path):
>+        # FIXME: handle self._browser_path.
>         args_with_url = self._insert_url(create_args(), 2, url)
>         self._launch_process(build_dir=browser_build_path, app_name=self.app_name, url=url, args=args_with_url)
> 
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py
>old mode 100644
>new mode 100755
>index a23d252210d71c1dc6c5d46991876597c40e257c..9907ced5ab62940668005a068ff2ba64f77b1d89
>--- a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py
>@@ -39,6 +39,7 @@ class OSXFirefoxNightlyDriver(OSXBrowserDriver):
>     app_name = 'FirefoxNightly.app'
> 
>     def launch_url(self, url, options, browser_build_path):
>+        # FIXME: handle self._browser_path.
>         args_with_url = self._insert_url(create_args(), 0, url)
>         self._launch_process(build_dir=browser_build_path, app_name=self.app_name, url=url, args=args_with_url)
> 
>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
>old mode 100644
>new mode 100755
>index a20ada44c90bc8d704eef160c1508213a4bcc3a8..aff307ed07dbcd2d61d4b2580b39b577803d79dd
>--- a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py
>@@ -16,6 +16,9 @@ class OSXSafariDriver(OSXBrowserDriver):
>     process_name = 'Safari'
>     browser_name = 'safari'
> 
>+    def __init__(self, browser_path):
>+        super(OSXSafariDriver, self).__init__(browser_path)
>+
>     def prepare_env(self, config):
>         self._safari_process = None
>         super(OSXSafariDriver, self).prepare_env(config)
>@@ -34,10 +37,16 @@ class OSXSafariDriver(OSXBrowserDriver):
>                 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)
>+        elif self._browser_path:
>+            safari_app_in_browser_path = os.path.join(self._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)
> 
>         args.extend(self._safari_preferences)
>         _log.info('Launching safari: %s with url: %s' % (args[0], url))
>-        self._safari_process = OSXSafariDriver._launch_process_with_caffinate(args, env)
>+        self._safari_process = OSXSafariDriver._launch_process_with_caffeinate(args, env)
> 
>         # Stop for initialization of the safari process, otherwise, open
>         # command may use the system safari.
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/run_benchmark.py b/Tools/Scripts/webkitpy/benchmark_runner/run_benchmark.py
>index 8752f2b6fc76b4410bb4bc64650c7e8a6925f9bd..52d2fe896ac1d4878508ae8be4c4a14ad1b60626 100755
>--- a/Tools/Scripts/webkitpy/benchmark_runner/run_benchmark.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/run_benchmark.py
>@@ -44,6 +44,7 @@ def parse_args():
>     parser.add_argument('--count', type=int, help='Number of times to run the benchmark (e.g. 5).')
>     parser.add_argument('--driver', default=WebServerBenchmarkRunner.name, choices=benchmark_runner_subclasses.keys(), help='Use the specified benchmark driver. Defaults to %s.' % WebServerBenchmarkRunner.name)
>     parser.add_argument('--browser', default=default_browser(), choices=BrowserDriverFactory.available_browsers(), help='Browser to run the nechmark in. Defaults to %s.' % default_browser())
>+    parser.add_argument('--browser-path', help='Specify the path to a non-default copy of the target browser as a path to the .app.')
>     parser.add_argument('--platform', default=default_platform(), choices=BrowserDriverFactory.available_platforms(), help='Platform that this script is running on. Defaults to %s.' % default_platform())
>     parser.add_argument('--local-copy', help='Path to a local copy of the benchmark (e.g. PerformanceTests/SunSpider/).')
>     parser.add_argument('--device-id', default=None, help='Undocumented option for mobile device testing.')
>@@ -65,7 +66,7 @@ def parse_args():
> 
> def run_benchmark_plan(args, plan):
>     benchmark_runner_class = benchmark_runner_subclasses[args.driver]
>-    runner = benchmark_runner_class(plan, args.local_copy, args.count, args.build_dir, args.output_file, args.platform, args.browser, args.scale_unit, args.show_iteration_values, args.device_id)
>+    runner = benchmark_runner_class(plan, args.local_copy, args.count, args.build_dir, args.output_file, args.platform, args.browser, args.browser_path, args.scale_unit, args.show_iteration_values, args.device_id)
>     runner.execute()
> 
> 
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/webserver_benchmark_runner.py b/Tools/Scripts/webkitpy/benchmark_runner/webserver_benchmark_runner.py
>old mode 100644
>new mode 100755
>index b0cb88d917373f93684689e48b19d91a4b0f85f5..ec23f22b9ea8bf4d55498687f40ec62cf5e4763d
>--- a/Tools/Scripts/webkitpy/benchmark_runner/webserver_benchmark_runner.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/webserver_benchmark_runner.py
>@@ -15,10 +15,10 @@ _log = logging.getLogger(__name__)
> class WebServerBenchmarkRunner(BenchmarkRunner):
>     name = 'webserver'
> 
>-    def __init__(self, plan_file, local_copy, count_override, build_dir, output_file, platform, browser, scale_unit=True, show_iteration_values=False, device_id=None):
>+    def __init__(self, plan_file, local_copy, count_override, build_dir, output_file, platform, browser, browser_path, scale_unit=True, show_iteration_values=False, device_id=None):
>         self._http_server_driver = HTTPServerDriverFactory.create(platform)
>         self._http_server_driver.set_device_id(device_id)
>-        super(WebServerBenchmarkRunner, self).__init__(plan_file, local_copy, count_override, build_dir, output_file, platform, browser, scale_unit, show_iteration_values, device_id)
>+        super(WebServerBenchmarkRunner, self).__init__(plan_file, local_copy, count_override, build_dir, output_file, platform, browser, browser_path, scale_unit, show_iteration_values, device_id)
> 
>     def _get_result(self, test_url):
>         result = self._browser_driver.add_additional_results(test_url, self._http_server_driver.fetch_result())
>diff --git a/Tools/Scripts/webkitpy/browserperfdash/browserperfdash_unittest.py b/Tools/Scripts/webkitpy/browserperfdash/browserperfdash_unittest.py
>old mode 100644
>new mode 100755
>index 34615186414ddd71c0a9c02ad7fb95a46353f81e..325f8a6ad42f1feac5c591c8fc3261af6701ce1b
>--- a/Tools/Scripts/webkitpy/browserperfdash/browserperfdash_unittest.py
>+++ b/Tools/Scripts/webkitpy/browserperfdash/browserperfdash_unittest.py
>@@ -40,8 +40,8 @@ _log = logging.getLogger(__name__)
> class FakeBenchmarkRunner(BenchmarkRunner):
>     name = 'fake'
> 
>-    def __init__(self, plan_file, local_copy, count_override, build_dir, output_file, platform, browser):
>-        super(FakeBenchmarkRunner, self).__init__(plan_file, local_copy, count_override, build_dir, output_file, platform, browser)
>+    def __init__(self, plan_file, local_copy, count_override, build_dir, output_file, platform, browser, browser_path):
>+        super(FakeBenchmarkRunner, self).__init__(plan_file, local_copy, count_override, build_dir, output_file, platform, browser, browser_path)
> 
>     def execute(self):
>         return True

Actions: View | Formatted Diff | Diff

Attachments on bug 192905: 357763 | 357879 | 357883