...
Created attachment 454787 [details] [fast-cq] patch
Committed r291325 (248464@main): <https://commits.webkit.org/248464@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454787 [details].
<rdar://problem/90345926>
Comment on attachment 454787 [details] [fast-cq] patch View in context: https://bugs.webkit.org/attachment.cgi?id=454787&action=review > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:50 > +def set_binary_location_impl(options, browser_build_path, app_name, process_name): > + if not browser_build_path: > + return > + app_path = os.path.join(browser_build_path, app_name) > + binary_path = os.path.join(app_path, "Contents/MacOS", process_name) > + options.binary_location = binary_path > + It looks like we can just move this to `OSXChromeDriverBase._set_chrome_binary_location`. So that we don't need to repeat it in the derived classes. > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:-67 > - if not browser_build_path: > - browser_build_path = '/Applications/' Could you explain why we want to stop loading it from '/Applications'? If we move set_binary_location_impl to `OSXChromeDriverBase._set_chrome_binary_location` & if we want to keep this behavior, we just do if not browser_build_path: browser_build_path = '/Applications/' super(OSXChromeCanaryDriver, self)._set_chrome_binary_location(options, browser_build_path)