streamline benchmark-runner browser

[Stephanie Lewis]

Created attachment 254127 [details]
patch

Some minor changes to benchmark-runner browser.

Add Caffeinated
Don't block on getting output.
Use system safari as default.
Add xpc dyld path.

[WebKit Commit Bot]

Attachment 254127 [details] did not pass style-queue:


ERROR: Tools/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Comment on attachment 254127 [details]
patch

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

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py:32
> +        process = subprocess.Popen(['open', '-a', appPath] + args)
> +        cls.launchCaffeinateForProcess(process.pid)

Instead of having a helper function to launch caffinate, why don't we add a method e.g. launchProcessWithCaffinate that launches both the target process and caffeinate?

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py:43
> +        subprocess.Popen(["/usr/bin/caffeinate", "-disw", str(pid)])

Does caffeinate automatically terminate?

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:39
> +        self.safariProcess = subprocess.Popen(args, env=env)
> +
> +        OSXSafariDriver.launchCaffeinateForProcess(self.safariProcess.pid)

That way, we can just call that function instead of Popen'ing and calling a separate helper function.

[Stephanie Lewis]

(In reply to comment #2)
> Comment on attachment 254127 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=254127&action=review
> 
> > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py:32
> > +        process = subprocess.Popen(['open', '-a', appPath] + args)
> > +        cls.launchCaffeinateForProcess(process.pid)
> 
> Instead of having a helper function to launch caffinate, why don't we add a
> method e.g. launchProcessWithCaffinate that launches both the target process
> and caffeinate?
> 
> > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py:43
> > +        subprocess.Popen(["/usr/bin/caffeinate", "-disw", str(pid)])
> 
> Does caffeinate automatically terminate?

Yeah Caffeinate takes the pid of a process to watch and terminates when the process terminates.

> 
> > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:39
> > +        self.safariProcess = subprocess.Popen(args, env=env)
> > +
> > +        OSXSafariDriver.launchCaffeinateForProcess(self.safariProcess.pid)
> 
> That way, we can just call that function instead of Popen'ing and calling a
> separate helper function.

[Stephanie Lewis]

Committed http://trac.webkit.org/changeset/185242