Make the requirements for activating VideoWakeLock stricter. [chromium/src : main]

Dale Curtis (Gerrit)

unread,

Mar 20, 2023, 3:21:19 PMMar 20

to blink-rev...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, mattreyno...@chromium.org, poscia...@chromium.org

Attention is currently required from: Dale Curtis, Frank Liberato, Yoav Weiss.

Dale Curtis uploaded patch set #10 to this change.

View Change

Make the requirements for activating VideoWakeLock stricter.

We've received several complaints that the video wake lock is being
held too liberally. E.g., for looping small emoji/videos and minor
video ads.

As such start enforcing stricter requirements:
- At least 75% of the video must be on screen.
- The video must take up at least 20% of the viewport.

The new behavior is behind a default on base::Feature flag per new
feature policy (so it can be turned off in the field if needed).

Fixed: 1340424
Change-Id: I70896baffb0b7b791f6dd29e0b99e8df4126bceb
---
M third_party/blink/renderer/core/html/media/video_wake_lock.cc
M third_party/blink/renderer/core/html/media/video_wake_lock.h
M third_party/blink/renderer/core/html/media/video_wake_lock_test.cc
3 files changed, 288 insertions(+), 80 deletions(-)

To view, visit change 3964046. To unsubscribe, or for help writing mail filters, visit settings.

Dale Curtis (Gerrit)

unread,

Mar 20, 2023, 3:21:29 PMMar 20

to blink-rev...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, mattreyno...@chromium.org, poscia...@chromium.org, Yoav Weiss, Chromium LUCI CQ, Frank Liberato, chromium...@chromium.org, Raphael Kubo Da Costa

Attention is currently required from: Frank Liberato, Yoav Weiss.

Patch set 10:Commit-Queue +1

View Change

2 comments:

File third_party/blink/renderer/core/html/media/video_wake_lock.h:
- Patch Set #7, Line 111: float visibility_threshold_ = 0.75f;
  you might be able to make this const since it's only modified in the ctor.
  Done
- Patch Set #7, Line 115: float size_threshold_ = 0.2f;
  i didn't see anything that modified this -- const / static constexpr?
  Done

To view, visit change 3964046. To unsubscribe, or for help writing mail filters, visit settings.

Frank Liberato (Gerrit)

unread,

Mar 20, 2023, 3:24:36 PMMar 20

to Dale Curtis, blink-rev...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, mattreyno...@chromium.org, poscia...@chromium.org, Yoav Weiss, Chromium LUCI CQ, chromium...@chromium.org, Raphael Kubo Da Costa

Attention is currently required from: Dale Curtis, Yoav Weiss.

Patch set 10:Code-Review +1

View Change

To view, visit change 3964046. To unsubscribe, or for help writing mail filters, visit settings.

Dale Curtis (Gerrit)

unread,

Mar 21, 2023, 1:08:36 PMMar 21

to blink-rev...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, mattreyno...@chromium.org, poscia...@chromium.org, Frank Liberato, Yoav Weiss, Chromium LUCI CQ, chromium...@chromium.org, Raphael Kubo Da Costa

Attention is currently required from: Yoav Weiss.

Patch set 11:Commit-Queue +2

View Change

To view, visit change 3964046. To unsubscribe, or for help writing mail filters, visit settings.

Chromium LUCI CQ (Gerrit)

unread,

Mar 21, 2023, 1:10:28 PMMar 21

to Dale Curtis, blink-rev...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, mattreyno...@chromium.org, poscia...@chromium.org, Frank Liberato, Yoav Weiss, chromium...@chromium.org, Raphael Kubo Da Costa

Chromium LUCI CQ submitted this change.

View Change



10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Approvals:
  Frank Liberato: Looks good to me
  Dale Curtis: Commit

Make the requirements for activating VideoWakeLock stricter.

We've received several complaints that the video wake lock is being
held too liberally. E.g., for looping small emoji/videos and minor
video ads.

As such start enforcing stricter requirements:
- At least 75% of the video must be on screen.
- The video must take up at least 20% of the viewport.

The new behavior is behind a default on base::Feature flag per new
feature policy (so it can be turned off in the field if needed).

Fixed: 1340424
Change-Id: I70896baffb0b7b791f6dd29e0b99e8df4126bceb

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3964046
Reviewed-by: Frank Liberato <libe...@chromium.org>
Commit-Queue: Dale Curtis <dalec...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1120011}


---
M third_party/blink/renderer/core/html/media/video_wake_lock.cc
M third_party/blink/renderer/core/html/media/video_wake_lock.h
M third_party/blink/renderer/core/html/media/video_wake_lock_test.cc
3 files changed, 288 insertions(+), 80 deletions(-)

To view, visit change 3964046. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward