Add "noplaybackrate" to HTMLMediaElement controlsList attribute

This CL adds a new "noplaybackrate" token to the HTMLMediaElement controlsList attribute to let developers enable/disable playback speed control in native media controls. Intent to prototype and ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/u9jsiarDEOg Spec: WICG/controls-list#13 Bug: 1211336 Change-Id: I11240652b5000972398d2f5faa8267035aa9abb8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2905660 Reviewed-by: Yoav Weiss <[email protected]> Reviewed-by: Tommy Steimel <[email protected]> Reviewed-by: Frank Liberato <[email protected]> Commit-Queue: François Beaufort <[email protected]> Cr-Commit-Position: refs/heads/master@{#889324}
neptunesoft · Jun 4, 2021 · 81e12ff · 81e12ff
1 parent 090f80c
commit 81e12ff
Show file tree

Hide file tree

Showing 22 changed files with 98 additions and 58 deletions.
diff --git a/third_party/blink/public/mojom/web_feature/web_feature.mojom b/third_party/blink/public/mojom/web_feature/web_feature.mojom
@@ -3242,6 +3242,7 @@ enum WebFeature {
   kHiddenBackfaceWith3D = 3927,
   kMainFrameNonSecurePrivateAddressSpace = 3928,
   kCSSSelectorPseudoHas = 3929,
+  kHTMLMediaElementControlsListNoPlaybackRate = 3930,
 
   // Add new features immediately above this line. Don't change assigned
   // numbers of any item, and don't reuse removed slots.

diff --git a/third_party/blink/renderer/core/html/media/html_media_element_controls_list.cc b/third_party/blink/renderer/core/html/media/html_media_element_controls_list.cc
@@ -12,10 +12,11 @@ namespace {
 
 const char kNoDownload[] = "nodownload";
 const char kNoFullscreen[] = "nofullscreen";
+const char kNoPlaybackRate[] = "noplaybackrate";
 const char kNoRemotePlayback[] = "noremoteplayback";
 
 const char* const kSupportedTokens[] = {kNoDownload, kNoFullscreen,
-                                        kNoRemotePlayback};
+                                        kNoPlaybackRate, kNoRemotePlayback};
 
 }  // namespace
 
@@ -41,6 +42,10 @@ bool HTMLMediaElementControlsList::ShouldHideFullscreen() const {
   return contains(kNoFullscreen);
 }
 
+bool HTMLMediaElementControlsList::ShouldHidePlaybackRate() const {
+  return contains(kNoPlaybackRate);
+}
+
 bool HTMLMediaElementControlsList::ShouldHideRemotePlayback() const {
   return contains(kNoRemotePlayback);
 }

diff --git a/third_party/blink/renderer/core/html/media/html_media_element_controls_list.h b/third_party/blink/renderer/core/html/media/html_media_element_controls_list.h
@@ -20,6 +20,7 @@ class CORE_EXPORT HTMLMediaElementControlsList final : public DOMTokenList {
   // Whether the list dictates to hide a certain control.
   bool ShouldHideDownload() const;
   bool ShouldHideFullscreen() const;
+  bool ShouldHidePlaybackRate() const;
   bool ShouldHideRemotePlayback() const;
 
  private:

diff --git a/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc b/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc
@@ -152,6 +152,16 @@ void MaybeParserAppendChild(Element* parent, Element* child) {
     parent->ParserAppendChild(child);
 }
 
+bool ShouldShowPlaybackSpeedButton(HTMLMediaElement& media_element) {
+  // The page disabled the button via the controlsList attribute.
+  if (media_element.ControlsListInternal()->ShouldHidePlaybackRate()) {
+    UseCounter::Count(media_element.GetDocument(),
+                      WebFeature::kHTMLMediaElementControlsListNoPlaybackRate);
+    return false;
+  }
+  return true;
+}
+
 bool ShouldShowPictureInPictureButton(HTMLMediaElement& media_element) {
   return media_element.SupportsPictureInPicture();
 }
@@ -567,6 +577,8 @@ void MediaControlsImpl::InitializeControls() {
   if (base::FeatureList::IsEnabled(media::kPlaybackSpeedButton)) {
     playback_speed_button_ =
         MakeGarbageCollected<MediaControlPlaybackSpeedButtonElement>(*this);
+    playback_speed_button_->SetIsWanted(
+        ShouldShowPlaybackSpeedButton(MediaElement()));
   }
   overflow_menu_ =
       MakeGarbageCollected<MediaControlOverflowMenuButtonElement>(*this);
@@ -927,6 +939,11 @@ void MediaControlsImpl::OnControlsListUpdated() {
 
   download_button_->SetIsWanted(
       download_button_->ShouldDisplayDownloadButton());
+
+  if (playback_speed_button_) {
+    playback_speed_button_->SetIsWanted(
+        ShouldShowPlaybackSpeedButton(MediaElement()));
+  }
 }
 
 LayoutObject* MediaControlsImpl::PanelLayoutObject() {

diff --git a/third_party/blink/web_tests/VirtualTestSuites b/third_party/blink/web_tests/VirtualTestSuites
@@ -1081,7 +1081,7 @@
   },
   {
     "prefix": "playback_speed_button",
-    "bases": ["media/controls"],
+    "bases": ["http/tests/media/controls", "media/controls"],
     "args": [ "--enable-features=PlaybackSpeedButton" ]
   },
   {

diff --git a/third_party/blink/web_tests/http/tests/media/controls/controls-list-add-hide-expected.txt b/third_party/blink/web_tests/http/tests/media/controls/controls-list-add-hide-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+FAIL Test disabling controls on the video element with all controls. Failed to find overflow item: -internal-media-controls-playback-speed-button
+Harness: the test ran to completion.
+
diff --git a/third_party/blink/web_tests/http/tests/media/controls/controls-list-add-hide.html b/third_party/blink/web_tests/http/tests/media/controls/controls-list-add-hide.html
@@ -10,19 +10,31 @@
   var v = document.getElementById('enabled-controls');
 
   v.addEventListener('canplaythrough', t.step_func(e => {
-    assert_true(isFullscreenButtonEnabled(v));
     assert_true(isDownloadsButtonEnabled(v));
+    assert_true(isFullscreenButtonEnabled(v));
+    assert_true(isPlaybackSpeedButtonEnabled(v));
 
     v.controlsList.add('nodownload');
 
     runAfterLayoutAndPaint(t.step_func(() => {
-      assert_true(isFullscreenButtonEnabled(v));
       assert_false(isDownloadsButtonEnabled(v));
+      assert_true(isFullscreenButtonEnabled(v));
+      assert_true(isPlaybackSpeedButtonEnabled(v));
+
       v.controlsList.add('nofullscreen');
 
-      runAfterLayoutAndPaint(t.step_func_done(() => {
-        assert_false(isFullscreenButtonEnabled(v));
+      runAfterLayoutAndPaint(t.step_func(() => {
         assert_false(isDownloadsButtonEnabled(v));
+        assert_false(isFullscreenButtonEnabled(v));
+        assert_true(isPlaybackSpeedButtonEnabled(v));
+
+        v.controlsList.add('noplaybackrate');
+
+        runAfterLayoutAndPaint(t.step_func_done(() => {
+          assert_false(isDownloadsButtonEnabled(v));
+          assert_false(isFullscreenButtonEnabled(v));
+          assert_false(isPlaybackSpeedButtonEnabled(v));
+        }));
       }));
     }));
   }));

diff --git a/third_party/blink/web_tests/http/tests/media/controls/controls-list-remove-show-expected.txt b/third_party/blink/web_tests/http/tests/media/controls/controls-list-remove-show-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+FAIL Test enabling controls on the video element with them enabled. Failed to find overflow item: -internal-media-controls-playback-speed-button
+Harness: the test ran to completion.
+
diff --git a/third_party/blink/web_tests/http/tests/media/controls/controls-list-remove-show.html b/third_party/blink/web_tests/http/tests/media/controls/controls-list-remove-show.html
@@ -4,26 +4,37 @@
 <script src="../../resources/testharnessreport.js"></script>
 <script src="../../resources/run-after-layout-and-paint.js"></script>
 <script src="../../media-resources/media-controls.js"></script>
-<video controlslist="nodownload nofullscreen" id="disabled-controls" width="500px"></video>
+<video controlslist="nodownload nofullscreen noplaybackrate" id="disabled-controls" width="500px"></video>
 <script>
 async_test(t => {
   var v = document.getElementById('disabled-controls');
 
   v.addEventListener('canplaythrough', t.step_func(e => {
-    assert_false(isFullscreenButtonEnabled(v));
     assert_false(isDownloadsButtonEnabled(v));
+    assert_false(isFullscreenButtonEnabled(v));
+    assert_false(isPlaybackSpeedButtonEnabled(v));
 
     v.controlsList.remove('nodownload');
 
     runAfterLayoutAndPaint(t.step_func(() => {
-      assert_false(isFullscreenButtonEnabled(v));
       assert_true(isDownloadsButtonEnabled(v));
+      assert_false(isFullscreenButtonEnabled(v));
+      assert_false(isPlaybackSpeedButtonEnabled(v));
 
       v.controlsList.remove('nofullscreen');
 
-      runAfterLayoutAndPaint(t.step_func_done(() => {
-        assert_true(isFullscreenButtonEnabled(v));
+      runAfterLayoutAndPaint(t.step_func(() => {
         assert_true(isDownloadsButtonEnabled(v));
+        assert_true(isFullscreenButtonEnabled(v));
+        assert_false(isPlaybackSpeedButtonEnabled(v));
+
+        v.controlsList.remove('noplaybackrate');
+
+        runAfterLayoutAndPaint(t.step_func_done(() => {
+          assert_true(isFullscreenButtonEnabled(v));
+          assert_true(isDownloadsButtonEnabled(v));
+          assert_true(isPlaybackSpeedButtonEnabled(v));
+        }));
       }));
     }));
   }));

diff --git a/...ests/http/tests/media/controls/video-controls-overflow-menu-correct-ordering-expected.txt b/...ests/http/tests/media/controls/video-controls-overflow-menu-correct-ordering-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+FAIL Overflow menu children appear in correct order. assert_equals: expected 8 but got 7
+Harness: the test ran to completion.
+
diff --git a/...nk/web_tests/http/tests/media/controls/video-controls-overflow-menu-correct-ordering.html b/...nk/web_tests/http/tests/media/controls/video-controls-overflow-menu-correct-ordering.html
@@ -25,7 +25,7 @@
 
     // The overflow menu should always have the same number of elements.
     // Their visibility will change over time based on the size of the video.
-    assert_equals(children.length, 7);
+    assert_equals(children.length, 8);
 
     // Ensure that all of the buttons are visible in the right order
     for (var i = 0; i < children.length; i++) {

diff --git a/third_party/blink/web_tests/media/controls/video-controls-overflow-menu-text-expected.txt b/third_party/blink/web_tests/media/controls/video-controls-overflow-menu-text-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+FAIL Overflow menu displays the correct text. assert_equals: expected "playback speed" but got "picture in picture"
+Harness: the test ran to completion.
+
diff --git a/third_party/blink/web_tests/media/overflow-menu.js b/third_party/blink/web_tests/media/overflow-menu.js
@@ -6,6 +6,7 @@ var overflowButtonsCSS = [
     "-webkit-media-controls-mute-button",
     "-internal-media-controls-cast-button",
     "-webkit-media-controls-toggle-closed-captions-button",
+    "-internal-media-controls-playback-speed-button",
     "-internal-media-controls-picture-in-picture-button",
 ];
 //  PseudoID for the overflow button
@@ -31,10 +32,11 @@ var OverflowMenuButtons = {
   MUTE: 3,
   CAST: 4,
   CLOSED_CAPTIONS: 5,
+  PLAYBACK_SPEED: 6,
 };
 
 // Default text within the overflow menu
-var overflowMenuText = ["Play", "Fullscreen", "Download", "Mute", "Cast", "CaptionsOff"];
+var overflowMenuText = ["Play", "Fullscreen", "Download", "Mute", "Cast", "CaptionsOff", "Playback speed"];
 
 if (document.pictureInPictureEnabled)
   overflowMenuText.push('Picture in Picture');
diff --git a/...rtual/playback_speed_button/http/tests/media/controls/controls-list-add-hide-expected.txt b/...rtual/playback_speed_button/http/tests/media/controls/controls-list-add-hide-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+PASS Test disabling controls on the video element with all controls.
+Harness: the test ran to completion.
+
diff --git a/...al/playback_speed_button/http/tests/media/controls/controls-list-remove-show-expected.txt b/...al/playback_speed_button/http/tests/media/controls/controls-list-remove-show-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+PASS Test enabling controls on the video element with them enabled.
+Harness: the test ran to completion.
+
diff --git a/...tton/http/tests/media/controls/video-controls-overflow-menu-correct-ordering-expected.txt b/...tton/http/tests/media/controls/video-controls-overflow-menu-correct-ordering-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+PASS Overflow menu children appear in correct order.
+Harness: the test ran to completion.
+
diff --git a/...y/blink/web_tests/virtual/playback_speed_button/media/controls/playback-speed-button.html b/...y/blink/web_tests/virtual/playback_speed_button/media/controls/playback-speed-button.html
@@ -3,7 +3,7 @@
 <script src="../../../../resources/testharness.js"></script>
 <script src="../../../../resources/testharnessreport.js"></script>
 <script src="../../../../media/media-controls.js"></script>
-<script src="../overflow-menu.js"></script>
+<script src="../../../../media/overflow-menu.js"></script>
 
 <!--Padding ensures the overflow menu is visible for the tests. -->
 <body style="padding-top: 200px; padding-left: 100px">

diff --git a/...rtual/playback_speed_button/media/controls/playback-speed-list-hide-on-click-outside.html b/...rtual/playback_speed_button/media/controls/playback-speed-list-hide-on-click-outside.html
@@ -3,7 +3,7 @@
 <script src="../../../../resources/testharness.js"></script>
 <script src="../../../../resources/testharnessreport.js"></script>
 <script src="../../../../media/media-controls.js"></script>
-<script src="../overflow-menu.js"></script>
+<script src="../../../../media/overflow-menu.js"></script>
 
 <!--Padding ensures the overflow menu is visible for the tests. -->
 <body style="padding-top: 200px; padding-left: 100px">

diff --git a/...rtual/playback_speed_button/media/controls/video-controls-overflow-menu-text-expected.txt b/...rtual/playback_speed_button/media/controls/video-controls-overflow-menu-text-expected.txt
@@ -0,0 +1,4 @@
+This is a testharness.js-based test.
+PASS Overflow menu displays the correct text.
+Harness: the test ran to completion.
+
diff --git a/...tests/virtual/playback_speed_button/media/controls/video-controls-overflow-menu-text.html b/...tests/virtual/playback_speed_button/media/controls/video-controls-overflow-menu-text.html
@@ -3,7 +3,7 @@
 <script src="../../../../resources/testharness.js"></script>
 <script src="../../../../resources/testharnessreport.js"></script>
 <script src="../../../../media/media-controls.js"></script>
-<script src="../overflow-menu.js"></script>
+<script src="../../../../media/overflow-menu.js"></script>
 
 <!--Padding ensures the overflow menu is visible for the tests. -->
 <body style="padding-top: 200px; padding-left: 100px">

diff --git a/third_party/blink/web_tests/virtual/playback_speed_button/media/overflow-menu.js b/third_party/blink/web_tests/virtual/playback_speed_button/media/overflow-menu.js
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
@@ -32834,6 +32834,7 @@ Called by update_use_counter_feature_enum.py.-->
   <int value="3927" label="HiddenBackfaceWith3D"/>
   <int value="3928" label="MainFrameNonSecurePrivateAddressSpace"/>
   <int value="3929" label="CSSSelectorPseudoHas"/>
+  <int value="3930" label="HTMLMediaElementControlsListNoPlaybackRate"/>
 </enum>
 
 <enum name="FeaturePolicyAllowlistType">