New issue
Advanced search Search tips

Issue 870454 link

Starred by 2 users
Status: Fixed
Owner:
mlamouri@chromium.org
Closed: Aug 3
Cc:
liber...@chromium.org
lethalantidote@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

VideoSurfaceLayer: disable feature by default on beta branch

Project Member Reported by mlamouri@chromium.org, Aug 2 
The feature is still suffering from one bug and has a perf regression with the first Beta coming out. We will still have it enabled 50/50 via Finch as none of these are Beta blockers but the feature is at risk for Stable and we shouldn't take the risk of merging this late.

Note that the change will only be sent to the Beta branch as we are already running 50/50 in Dev/Canary so both code paths are exercised already.
Project Member

Comment 1 by sheriffbot@chromium.org, Aug 2

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 2 by mlamouri@chromium.org, Aug 2 

The CL is basically going to be https://chromium-review.googlesource.com/c/chromium/src/+/1161255 (+tests changes).

It's safe to merge directly in Beta because this test path is already exercised in Canary/Dev via Finch and is also going to be exercised in Beta 50/50. This is only changing the default value for users that do not have Finch. It's a safeguard in case of we have to turn off the feature as these users would otherwise have the feature on by default.

Comment 3 by gov...@chromium.org, Aug 2

Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #2. Please test it locally before merging to M69. Thank you.

Pls mark bug as fixed after the merge if nothing else is pending.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 3

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d88f64944fcaff079e1d806a5baabfaccc07100b

commit d88f64944fcaff079e1d806a5baabfaccc07100b
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Fri Aug 03 18:05:07 2018

VideoSurfaceLayer: disable feature by default on beta branch.

The feature will instead rely only on Finch to be enabled.
This CL also disables a unittest that required the feature to be
enabled.

Bug:  870454 
Change-Id: Ib6e7b54cea7a6d258103157b9448b7ea03dc2209
Reviewed-on: https://chromium-review.googlesource.com/1161574
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#385}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/d88f64944fcaff079e1d806a5baabfaccc07100b/media/base/media_switches.cc
[modify] https://crrev.com/d88f64944fcaff079e1d806a5baabfaccc07100b/media/blink/webmediaplayer_impl_unittest.cc

Comment 5 by mlamouri@chromium.org, Aug 3

Status: Fixed (was: Started)
►

Sign in to add a comment