New issue
Advanced search Search tips

Issue 799125 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-XR



Sign in to add a comment

WebVrVsyncAlign no longer being disabled in Telemetry tests

Project Member Reported by bsheedy@chromium.org, Jan 4 2018

Issue description

--disable-features=WebVrVsyncAlign is no longer being properly applied during VR Telemetry tests, which is causing performance regressions to be reported.

The cause is https://chromium.googlesource.com/catapult.git/+/ed32dd50b3276262cb0ce22691b8f263955b0577, although all that did was expose an existing bug in Telemetry.

Chrome only works with one instance each of --disable-features, --enable-features, and --force-field-trials - if multiple instances are in the command line, then Chrome uses whatever it parses last.

Telemetry blindly appends extra browser arguments from a Page to the list, so if one of the above switches is used in a Page's extra args, then there will be multiple usage instances.

Before being passed to Chrome, Telemetry puts all the command line flags into a set to remove duplicates. Prior to the above CL, an additional feature was appended to the end of all --disable-features switches, which caused the --disable-features instance with WebVrVsyncAlign to appear last in the set. With the above CL, WebVrVsyncAlign now appears before another --disable-features instance, so it gets overridden.

The solution for this is to have Telemetry manually merge multiple instances of the offending switches before passing them to Chrome.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/b3d2f85cd3ba76ba8a4b4d28f06d3e38cc860f9d

commit b3d2f85cd3ba76ba8a4b4d28f06d3e38cc860f9d
Author: bsheedy <bsheedy@chromium.org>
Date: Mon Jan 08 19:12:52 2018

Merge duplicate command line switches that use lists

Merges multiple instances of --disable-features, --enable-features,
and --force-field-trials in the command line args passed to Chrome.
Without this, use of these switches from a Page's extra_browser_args
property would either not be applied in Chrome or clobber other
instances of the switch.

Bug:  chromium:799125 
Change-Id: Ib25923c0c8bf0b6772d06a6307e7a22ebe6710ed
Reviewed-on: https://chromium-review.googlesource.com/850437
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/b3d2f85cd3ba76ba8a4b4d28f06d3e38cc860f9d/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend.py
[modify] https://crrev.com/b3d2f85cd3ba76ba8a4b4d28f06d3e38cc860f9d/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend_unittest.py

Status: Fixed (was: Started)
The feature is being disabled properly during perf tests now, and the performance regression has un-regressed.
Labels: Test-Complete
Components: Internals>XR

Sign in to add a comment