[Media Controls] Populate fullscreen option properly.

This CL add a check to properly populate fullscreen option.

Bug: 942156
Change-Id: I8227a91cac36c57ff1f63065af8cea72f34ee344
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610585Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Jazz Xu <jazzhsu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659253}

third_party/blink/renderer/modules/media_controls/media_controls_impl.cc

@@ -87,6 +87,7 @@
 #include "third_party/blink/renderer/modules/media_controls/media_controls_orientation_lock_delegate.h"
 #include "third_party/blink/renderer/modules/media_controls/media_controls_resource_loader.h"
 #include "third_party/blink/renderer/modules/media_controls/media_controls_rotate_to_fullscreen_delegate.h"
+#include "third_party/blink/renderer/modules/media_controls/media_controls_shared_helper.h"
 #include "third_party/blink/renderer/modules/media_controls/media_controls_text_track_manager.h"
 #include "third_party/blink/renderer/modules/remoteplayback/remote_playback.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
@@ -153,33 +154,6 @@ constexpr WTF::TimeDelta kTimeToShowVolumeSliderTest =
 // The number of seconds to jump when double tapping.
 constexpr int kNumberOfSecondsToJump = 10;
 
-bool ShouldShowFullscreenButton(const HTMLMediaElement& media_element) {
-  // Unconditionally allow the user to exit fullscreen if we are in it
-  // now.  Especially on android, when we might not yet know if
-  // fullscreen is supported, we sometimes guess incorrectly and show
-  // the button earlier, and we don't want to remove it here if the
-  // user chose to enter fullscreen.  crbug.com/500732 .
-  if (media_element.IsFullscreen())
-    return true;
-
-  if (!media_element.IsHTMLVideoElement())
-    return false;
-
-  if (!media_element.HasVideo())
-    return false;
-
-  if (!Fullscreen::FullscreenEnabled(media_element.GetDocument()))
-    return false;
-
-  if (media_element.ControlsListInternal()->ShouldHideFullscreen()) {
-    UseCounter::Count(media_element.GetDocument(),
-                      WebFeature::kHTMLMediaElementControlsListNoFullscreen);
-    return false;
-  }
-
-  return true;
-}
-
 void MaybeParserAppendChild(Element* parent, Element* child) {
   DCHECK(parent);
   if (child)
@@ -966,12 +940,14 @@ void MediaControlsImpl::OnControlsListUpdated() {
 
   if (IsModern() && ShouldShowVideoControls()) {
     fullscreen_button_->SetIsWanted(true);
-    fullscreen_button_->setAttribute(html_names::kDisabledAttr,
-                                     ShouldShowFullscreenButton(MediaElement())
-                                         ? AtomicString()
-                                         : AtomicString(""));
+    fullscreen_button_->setAttribute(
+        html_names::kDisabledAttr,
+        MediaControlsSharedHelpers::ShouldShowFullscreenButton(MediaElement())
+            ? AtomicString()
+            : AtomicString(""));
   } else {
-    fullscreen_button_->SetIsWanted(ShouldShowFullscreenButton(MediaElement()));
+    fullscreen_button_->SetIsWanted(
+        MediaControlsSharedHelpers::ShouldShowFullscreenButton(MediaElement()));
     fullscreen_button_->removeAttribute(html_names::kDisabledAttr);
   }
 

third_party/blink/renderer/modules/media_controls/media_controls_shared_helper.cc

@@ -5,7 +5,11 @@
 #include "third_party/blink/renderer/modules/media_controls/media_controls_shared_helper.h"
 
 #include <cmath>
+#include "third_party/blink/public/mojom/web_feature/web_feature.mojom-shared.h"
+#include "third_party/blink/renderer/core/frame/use_counter.h"
+#include "third_party/blink/renderer/core/fullscreen/fullscreen.h"
 #include "third_party/blink/renderer/core/html/media/html_media_element.h"
+#include "third_party/blink/renderer/core/html/media/html_media_element_controls_list.h"
 #include "third_party/blink/renderer/core/html/time_ranges.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 
@@ -80,4 +84,32 @@ String MediaControlsSharedHelpers::FormatTime(double time) {
   return String::Format("%s%d:%02d", negative_sign, minutes, seconds);
 }
 
+bool MediaControlsSharedHelpers::ShouldShowFullscreenButton(
+    const HTMLMediaElement& media_element) {
+  // Unconditionally allow the user to exit fullscreen if we are in it
+  // now.  Especially on android, when we might not yet know if
+  // fullscreen is supported, we sometimes guess incorrectly and show
+  // the button earlier, and we don't want to remove it here if the
+  // user chose to enter fullscreen.  crbug.com/500732 .
+  if (media_element.IsFullscreen())
+    return true;
+
+  if (!media_element.IsHTMLVideoElement())
+    return false;
+
+  if (!media_element.HasVideo())
+    return false;
+
+  if (!Fullscreen::FullscreenEnabled(media_element.GetDocument()))
+    return false;
+
+  if (media_element.ControlsListInternal()->ShouldHideFullscreen()) {
+    UseCounter::Count(media_element.GetDocument(),
+                      WebFeature::kHTMLMediaElementControlsListNoFullscreen);
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/media_controls/media_controls_shared_helper.h

@@ -21,6 +21,7 @@ class MediaControlsSharedHelpers final {
       HTMLMediaElement& media_element);
 
   static String FormatTime(double time);
+  static bool ShouldShowFullscreenButton(const HTMLMediaElement& media_element);
 };
 
 }  // namespace blink

third_party/blink/renderer/modules/media_controls/touchless/media_controls_touchless_impl.cc

@@ -23,6 +23,7 @@
 #include "third_party/blink/renderer/core/page/chrome_client.h"
 #include "third_party/blink/renderer/modules/media_controls/elements/media_control_elements_helper.h"
 #include "third_party/blink/renderer/modules/media_controls/media_controls_orientation_lock_delegate.h"
+#include "third_party/blink/renderer/modules/media_controls/media_controls_shared_helper.h"
 #include "third_party/blink/renderer/modules/media_controls/media_controls_text_track_manager.h"
 #include "third_party/blink/renderer/modules/media_controls/touchless/elements/media_controls_touchless_bottom_container_element.h"
 #include "third_party/blink/renderer/modules/media_controls/touchless/elements/media_controls_touchless_overlay_element.h"
@@ -270,9 +271,8 @@ void MediaControlsTouchlessImpl::ShowContextMenu() {
 
   WTF::Vector<mojom::blink::MenuItem> menu_items;
 
-  // TODO(jazzhsu, https://crbug.com/942577): Populate fullscreen list entry
-  // properly.
-  menu_items.push_back(mojom::blink::MenuItem::FULLSCREEN);
+  if (MediaControlsSharedHelpers::ShouldShowFullscreenButton(MediaElement()))
+    menu_items.push_back(mojom::blink::MenuItem::FULLSCREEN);
 
   if (MediaElement().HasAudio())
     menu_items.push_back(mojom::blink::MenuItem::MUTE);

third_party/blink/renderer/modules/media_controls/touchless/media_controls_touchless_impl_test.cc

@@ -18,6 +18,7 @@
 #include "third_party/blink/renderer/core/fullscreen/fullscreen.h"
 #include "third_party/blink/renderer/core/geometry/dom_rect.h"
 #include "third_party/blink/renderer/core/html/media/html_media_element.h"
+#include "third_party/blink/renderer/core/html/media/html_media_element_controls_list.h"
 #include "third_party/blink/renderer/core/html/media/html_media_test_helper.h"
 #include "third_party/blink/renderer/core/html/media/html_video_element.h"
 #include "third_party/blink/renderer/core/html/time_ranges.h"
@@ -406,14 +407,15 @@ TEST_F(MediaControlsTouchlessImplTest, ContextMenuMojomTest) {
   EXPECT_TRUE(arg_list.video_state->is_fullscreen);
   EXPECT_FALSE(MediaElement().IsFullscreen());
 
-  // Disable download and show mute option.
+  // Disable download and fullscreen, and show mute option.
+  MediaElement().ControlsListInternal()->Add("nofullscreen");
   MediaElement().GetDocument().GetSettings()->SetHideDownloadUI(true);
   SetHasAudio(true);
 
   SetMenuResponseAndShowMenu(mojom::blink::MenuItem::MUTE);
 
-  EXPECT_EQ((int)arg_list.menu_items.size(), 2);
-  EXPECT_EQ(arg_list.menu_items[1], mojom::blink::MenuItem::MUTE);
+  EXPECT_EQ((int)arg_list.menu_items.size(), 1);
+  EXPECT_EQ(arg_list.menu_items[0], mojom::blink::MenuItem::MUTE);
   EXPECT_FALSE(arg_list.video_state->is_muted);
   EXPECT_TRUE(MediaElement().muted());
 
@@ -428,8 +430,8 @@ TEST_F(MediaControlsTouchlessImplTest, ContextMenuMojomTest) {
                                                  ASSERT_NO_EXCEPTION);
   SetMenuResponseAndShowMenu(mojom::blink::MenuItem::CAPTIONS, 0);
 
-  EXPECT_EQ((int)arg_list.menu_items.size(), 2);
-  EXPECT_EQ(arg_list.menu_items[1], mojom::blink::MenuItem::CAPTIONS);
+  EXPECT_EQ((int)arg_list.menu_items.size(), 1);
+  EXPECT_EQ(arg_list.menu_items[0], mojom::blink::MenuItem::CAPTIONS);
   EXPECT_EQ(arg_list.text_tracks[1]->label, "english");
   EXPECT_EQ(track->mode(), TextTrack::ShowingKeyword());