Bug 234107

Summary: 'run-benchmark' should launch browsers in a relative clean state.
Product: WebKit Reporter: dewei_zhu
Component: New BugsAssignee: dewei_zhu
Status: RESOLVED FIXED    
Severity: Normal CC: dewei_zhu, ews-watchlist, glenn, gsnedders, jbedard, slewis
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch slewis: review+

Description dewei_zhu 2021-12-09 15:31:26 PST
'run-benchmark' should launch browsers in a relative clean state.
Comment 1 dewei_zhu 2021-12-09 15:46:37 PST
Created attachment 446624 [details]
Patch
Comment 2 dewei_zhu 2021-12-09 16:51:50 PST
<rdar://85831161>
Comment 3 Stephanie Lewis 2021-12-10 13:58:04 PST
Comment on attachment 446624 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=446624&action=review

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:34
> +        chrome_options.add_argument("--disable-extensions")

did you want disable-extensions in two places?

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:41
> +        return '--window-size={width},{height}'.format(width=int(screen_size.width), height=int(screen_size.height))

does setting window size set content size or window size?

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py:33
> +        args_with_url = ['--args', '-width', str(int(screen_size.width)), '-height', str(int(screen_size.height))]

Do we really run  fullscreen?
Same question does this set content size or window size because Chrome, Firefox, and Safari often have different sized chromes and we want the content size to match
Comment 4 dewei_zhu 2021-12-12 17:52:50 PST
Comment on attachment 446624 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=446624&action=review

>> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:34
>> +        chrome_options.add_argument("--disable-extensions")
> 
> did you want disable-extensions in two places?

Those are different, one for webdriver, one for launch with 'open' command.

>> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:41
>> +        return '--window-size={width},{height}'.format(width=int(screen_size.width), height=int(screen_size.height))
> 
> does setting window size set content size or window size?

It will fill the screen, that's what we want.

>> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_firefox_driver.py:33
>> +        args_with_url = ['--args', '-width', str(int(screen_size.width)), '-height', str(int(screen_size.height))]
> 
> Do we really run  fullscreen?
> Same question does this set content size or window size because Chrome, Firefox, and Safari often have different sized chromes and we want the content size to match

This will not be full screen, it will fill screen without overlapping with docker.
Comment 5 dewei_zhu 2021-12-16 16:06:09 PST
Comment on attachment 446624 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=446624&action=review

>>> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:41
>>> +        return '--window-size={width},{height}'.format(width=int(screen_size.width), height=int(screen_size.height))
>> 
>> does setting window size set content size or window size?
> 
> It will fill the screen, that's what we want.

Discussed in person. It looks like MotionMark is immune from slightly window height change and speedometer2 doesn't show significant change (100 iteration) with slightly different window size.
Comment 6 dewei_zhu 2021-12-20 17:20:24 PST
Landed in r287289.