[Viewport Segments API] Fix fullscreen video playback when the device…

… is folded When the device is folded (and for PCs the keyboard is detached), the fullscreen video is laid across the fold which is not a great user experience. This patch fixes the issue by putting the video in one segment only. fullscreen.css is a user agent style sheet and do not support CSS environment variables and media queries. CSS environment variables are correctly parsed but not resolved. In order to make the patch simple only the Viewport Segments variables will be resolved. In regards to MQs we now enable parsing them and resolving them the first time. However in this patch the MQs will only be re-evaluated when the Viewport Segments changes. This patch will only impact non-foldable devices when parsing the fullscreen.css as we now parse MQs which we didn't before, but said MQs will be evaluated once (to none) and never re-evaluated. Bug: 1487699 Change-Id: I43adea152a273dce79dbecafff9327f11883e3ed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4903622 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Jonathan Ross <jonross@chromium.org> Commit-Queue: Alexis Menard <alexis.menard@intel.com> Cr-Commit-Position: refs/heads/main@{#1209449}
chromium · Oct 13, 2023 · 1746092 · 1746092
1 parent 3e2a8e2
commit 1746092
Show file tree

Hide file tree

Showing 11 changed files with 173 additions and 18 deletions.
diff --git a/content/browser/renderer_host/render_widget_host_browsertest.cc b/content/browser/renderer_host/render_widget_host_browsertest.cc
@@ -871,6 +871,92 @@ class RenderWidgetHostFoldableCSSTest : public RenderWidgetHostBrowserTest {
   }
 };
 
+// Tests that when a video element goes fullscreen and uses the default
+// fullscreen UA stylesheet (in blink/core/css/fullscreen.css) the viewport
+// segments MQs and env variables are correctly working.
+IN_PROC_BROWSER_TEST_F(RenderWidgetHostFoldableCSSTest,
+                       ViewportSegmentsWorksInUAFullscreenCSS) {
+  const char kTestPageURL[] =
+      R"HTML(data:text/html,<!DOCTYPE html>
+      <video id='video'></video>)HTML";
+  EXPECT_TRUE(NavigateToURL(shell(), GURL(kTestPageURL)));
+  // Check initial state before entering fullscreen.
+  ASSERT_FALSE(shell()->IsFullscreenForTabOrPending(web_contents()));
+  ASSERT_FALSE(web_contents()->IsFullscreen());
+  constexpr char kEnterFullscreenScript[] = R"JS(
+    document.querySelector('video').requestFullscreen().then(() => {
+        return !!document.fullscreenElement;
+    });
+  )JS";
+  MockDisplayFeature mock_display_feature(view());
+  // Initial state. This will ensure that no display feature/viewport segments
+  // are coming from the platform.
+  mock_display_feature.SetDisplayFeature(nullptr);
+  host()->SynchronizeVisualProperties();
+  ASSERT_TRUE(EvalJs(web_contents(), kEnterFullscreenScript).ExtractBool());
+
+  // Changing the display feature/viewport segments without leaving fullscreen
+  // should update the video element.
+  const gfx::Size root_view_size = view()->GetVisibleViewportSize();
+  const int kDisplayFeatureLength = 10;
+  int offset = root_view_size.width() / 2 - kDisplayFeatureLength / 2;
+  DisplayFeature emulated_display_feature{
+      DisplayFeature::Orientation::kVertical, offset,
+      /* mask_length */ kDisplayFeatureLength};
+  mock_display_feature.SetDisplayFeature(&emulated_display_feature);
+  host()->SynchronizeVisualProperties();
+  WaitForVisualPropertiesAck();
+  EXPECT_EQ(base::NumberToString(offset) + "px",
+            EvalJs(shell(), "getComputedStyle(video).width").ExtractString());
+  EXPECT_EQ(
+      root_view_size.height(),
+      EvalJs(shell(), "parseInt(getComputedStyle(video).height)").ExtractInt());
+
+  emulated_display_feature.orientation =
+      DisplayFeature::Orientation::kHorizontal;
+  offset = root_view_size.height() / 2 - kDisplayFeatureLength / 2;
+  emulated_display_feature.offset = offset;
+  mock_display_feature.SetDisplayFeature(&emulated_display_feature);
+  host()->SynchronizeVisualProperties();
+  WaitForVisualPropertiesAck();
+  EXPECT_EQ(base::NumberToString(offset) + "px",
+            EvalJs(shell(), "getComputedStyle(video).height").ExtractString());
+  EXPECT_EQ(
+      root_view_size.width(),
+      EvalJs(shell(), "parseInt(getComputedStyle(video).width)").ExtractInt());
+
+  // No display feature/viewport segments are set, the video should go
+  // fullscreen.
+  mock_display_feature.SetDisplayFeature(nullptr);
+  host()->SynchronizeVisualProperties();
+  WaitForVisualPropertiesAck();
+  EXPECT_EQ(
+      root_view_size.height(),
+      EvalJs(shell(), "parseInt(getComputedStyle(video).height)").ExtractInt());
+  EXPECT_EQ(
+      root_view_size.width(),
+      EvalJs(shell(), "parseInt(getComputedStyle(video).width)").ExtractInt());
+
+  constexpr char kExitFullscreenScript[] = R"JS(
+    document.exitFullscreen().then(() => {
+        return !document.fullscreenElement;
+    });
+  )JS";
+  ASSERT_TRUE(EvalJs(web_contents(), kExitFullscreenScript).ExtractBool());
+  ASSERT_FALSE(web_contents()->IsFullscreen());
+
+  // Change the viewport segments/display feature before entering fullscreen.
+  mock_display_feature.SetDisplayFeature(&emulated_display_feature);
+  ASSERT_TRUE(EvalJs(web_contents(), kEnterFullscreenScript).ExtractBool());
+  host()->SynchronizeVisualProperties();
+  WaitForVisualPropertiesAck();
+  EXPECT_EQ(base::NumberToString(offset) + "px",
+            EvalJs(shell(), "getComputedStyle(video).height").ExtractString());
+  EXPECT_EQ(
+      root_view_size.width(),
+      EvalJs(shell(), "parseInt(getComputedStyle(video).width)").ExtractInt());
+}
+
 // Tests that the renderer receives the root widget's window segments and
 // correctly exposes those via CSS.
 // TODO(crbug.com/1098549) Convert this to a WPT once emulation is available

diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -798,8 +798,10 @@ void RenderWidgetHostViewAura::ComputeDisplayFeature() {
 
   const display::Display display =
       display::Screen::GetScreen()->GetDisplayNearestWindow(window_);
-  // Set the display feature only if the browser window is maximized.
-  if (window_->GetRootWindow()->GetBoundsInScreen() != display.work_area()) {
+  // Set the display feature only if the browser window is maximized or
+  // fullscreen.
+  if (window_->GetRootWindow()->GetBoundsInScreen() != display.work_area() &&
+      window_->GetRootWindow()->GetBoundsInScreen() != display.bounds()) {
     return;
   }
 

diff --git a/third_party/blink/renderer/core/css/css_default_style_sheets.cc b/third_party/blink/renderer/core/css/css_default_style_sheets.cc
@@ -33,6 +33,7 @@
 #include "third_party/blink/public/resources/grit/blink_resources.h"
 #include "third_party/blink/renderer/core/css/media_query_evaluator.h"
 #include "third_party/blink/renderer/core/css/rule_set.h"
+#include "third_party/blink/renderer/core/css/style_engine.h"
 #include "third_party/blink/renderer/core/css/style_sheet_contents.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/execution_context/security_context.h"
@@ -169,7 +170,10 @@ void CSSDefaultStyleSheets::VerifyUniversalRuleCount() {
 
   if (fullscreen_style_sheet_) {
     default_fullscreen_style_->CompactRulesIfNeeded();
-    DCHECK_EQ(default_fullscreen_style_->UniversalRules().size(), 7u);
+    // There are 7 rules by default but if the viewport segments MQs are
+    // resolved then we have an additional rule.
+    DCHECK(default_fullscreen_style_->UniversalRules().size() == 7u ||
+           default_fullscreen_style_->UniversalRules().size() == 8u);
   }
 
   if (marker_style_sheet_) {
@@ -236,9 +240,6 @@ void CSSDefaultStyleSheets::AddRulesToDefaultStyleSheets(
     case NamespaceType::kMediaControls:
       default_media_controls_style_->AddRulesFromSheet(rules, ScreenEval());
       break;
-    case NamespaceType::kFullscreen:
-      default_fullscreen_style_->AddRulesFromSheet(rules, ScreenEval());
-      break;
   }
   // Add to print and forced color for all namespaces.
   default_print_style_->AddRulesFromSheet(rules, PrintEval());
@@ -390,17 +391,41 @@ void CSSDefaultStyleSheets::SetMediaControlsStyleSheetLoader(
   media_controls_style_sheet_loader_.swap(loader);
 }
 
-void CSSDefaultStyleSheets::EnsureDefaultStyleSheetForFullscreen() {
+void CSSDefaultStyleSheets::EnsureDefaultStyleSheetForFullscreen(
+    const Element& element) {
   if (fullscreen_style_sheet_) {
+    DCHECK(!default_fullscreen_style_->DidMediaQueryResultsChange(
+        MediaQueryEvaluator(element.GetDocument().GetFrame())));
     return;
   }
 
   String fullscreen_rules =
       UncompressResourceAsASCIIString(IDR_UASTYLE_FULLSCREEN_CSS) +
       LayoutTheme::GetTheme().ExtraFullscreenStyleSheet();
   fullscreen_style_sheet_ = ParseUASheet(fullscreen_rules);
-  AddRulesToDefaultStyleSheets(fullscreen_style_sheet_,
-                               NamespaceType::kFullscreen);
+
+  default_fullscreen_style_->AddRulesFromSheet(
+      fullscreen_style_sheet_,
+      MediaQueryEvaluator(element.GetDocument().GetFrame()));
+  VerifyUniversalRuleCount();
+}
+
+void CSSDefaultStyleSheets::RebuildFullscreenRuleSetIfMediaQueriesChanged(
+    const Element& element) {
+  if (!fullscreen_style_sheet_) {
+    return;
+  }
+
+  if (!default_fullscreen_style_->DidMediaQueryResultsChange(
+          MediaQueryEvaluator(element.GetDocument().GetFrame()))) {
+    return;
+  }
+
+  default_fullscreen_style_ = MakeGarbageCollected<RuleSet>();
+  default_fullscreen_style_->AddRulesFromSheet(
+      fullscreen_style_sheet_,
+      MediaQueryEvaluator(element.GetDocument().GetFrame()));
+  VerifyUniversalRuleCount();
 }
 
 bool CSSDefaultStyleSheets::EnsureDefaultStyleSheetForForcedColors() {

diff --git a/third_party/blink/renderer/core/css/css_default_style_sheets.h b/third_party/blink/renderer/core/css/css_default_style_sheets.h
@@ -57,7 +57,8 @@ class CSSDefaultStyleSheets final
 
   bool EnsureDefaultStyleSheetsForElement(const Element&);
   bool EnsureDefaultStyleSheetsForPseudoElement(PseudoId);
-  void EnsureDefaultStyleSheetForFullscreen();
+  void EnsureDefaultStyleSheetForFullscreen(const Element& element);
+  void RebuildFullscreenRuleSetIfMediaQueriesChanged(const Element& element);
   bool EnsureDefaultStyleSheetForForcedColors();
 
   RuleSet* DefaultHtmlStyle() { return default_html_style_.Get(); }
@@ -132,7 +133,6 @@ class CSSDefaultStyleSheets final
     kMathML,
     kSVG,
     kMediaControls,  // Not exactly a namespace
-    kFullscreen,
   };
   void AddRulesToDefaultStyleSheets(StyleSheetContents* rules,
                                     NamespaceType type);

diff --git a/third_party/blink/renderer/core/css/fullscreen.css b/third_party/blink/renderer/core/css/fullscreen.css
@@ -33,6 +33,20 @@ iframe:fullscreen {
   background: black;
 }
 
+@media (vertical-viewport-segments: 2) {
+  :not(:root):fullscreen {
+    height: env(viewport-segment-bottom 0 0, 100%) !important;
+    width: 100% !important;
+  }
+}
+
+@media (horizontal-viewport-segments: 2) {
+  :not(:root):fullscreen {
+    height: 100% !important;
+    width: env(viewport-segment-right 0 0, 100%) !important;
+  }
+}
+
 /* Anything below are extensions over what the Fullscreen API (29 May 2018) mandates. */
 
 /* This prevents video from overflowing the viewport in

diff --git a/third_party/blink/renderer/core/css/style_engine.cc b/third_party/blink/renderer/core/css/style_engine.cc
@@ -2288,12 +2288,13 @@ void StyleEngine::CollectFeaturesTo(RuleFeatureSet& features) {
   CollectScopedStyleFeaturesTo(features);
 }
 
-void StyleEngine::EnsureUAStyleForFullscreen() {
+void StyleEngine::EnsureUAStyleForFullscreen(const Element& element) {
   DCHECK(global_rule_set_);
   if (global_rule_set_->HasFullscreenUAStyle()) {
     return;
   }
-  CSSDefaultStyleSheets::Instance().EnsureDefaultStyleSheetForFullscreen();
+  CSSDefaultStyleSheets::Instance().EnsureDefaultStyleSheetForFullscreen(
+      element);
   global_rule_set_->MarkDirty();
   UpdateActiveStyle();
 }

diff --git a/third_party/blink/renderer/core/css/style_engine.h b/third_party/blink/renderer/core/css/style_engine.h
@@ -452,7 +452,7 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,
 
   void CollectFeaturesTo(RuleFeatureSet& features);
 
-  void EnsureUAStyleForFullscreen();
+  void EnsureUAStyleForFullscreen(const Element&);
   void EnsureUAStyleForElement(const Element&);
   void EnsureUAStyleForPseudoElement(PseudoId);
   void EnsureUAStyleForForcedColors();

diff --git a/third_party/blink/renderer/core/dom/element.cc b/third_party/blink/renderer/core/dom/element.cc
@@ -7669,7 +7669,7 @@ void Element::SetContainsFullScreenElement(bool flag) {
   // When exiting fullscreen, the element's document may not be active.
   if (flag) {
     DCHECK(GetDocument().IsActive());
-    GetDocument().GetStyleEngine().EnsureUAStyleForFullscreen();
+    GetDocument().GetStyleEngine().EnsureUAStyleForFullscreen(*this);
   }
   PseudoStateChanged(CSSSelector::kPseudoFullScreenAncestor);
 }

diff --git a/third_party/blink/renderer/core/frame/local_frame.cc b/third_party/blink/renderer/core/frame/local_frame.cc
@@ -86,6 +86,7 @@
 #include "third_party/blink/renderer/core/css/background_color_paint_image_generator.h"
 #include "third_party/blink/renderer/core/css/box_shadow_paint_image_generator.h"
 #include "third_party/blink/renderer/core/css/clip_path_paint_image_generator.h"
+#include "third_party/blink/renderer/core/css/css_default_style_sheets.h"
 #include "third_party/blink/renderer/core/css/document_style_environment_variables.h"
 #include "third_party/blink/renderer/core/css/style_change_reason.h"
 #include "third_party/blink/renderer/core/dom/child_frame_disconnector.h"
@@ -141,6 +142,7 @@
 #include "third_party/blink/renderer/core/frame/visual_viewport.h"
 #include "third_party/blink/renderer/core/frame/web_frame_widget_impl.h"
 #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
+#include "third_party/blink/renderer/core/fullscreen/fullscreen.h"
 #include "third_party/blink/renderer/core/fullscreen/scoped_allow_fullscreen.h"
 #include "third_party/blink/renderer/core/html/html_frame_element_base.h"
 #include "third_party/blink/renderer/core/html/html_link_element.h"
@@ -1481,6 +1483,15 @@ void LocalFrame::WindowSegmentsChanged(
   // "horizontal-viewport-segments" and "vertical-viewport-segments" features).
   MediaQueryAffectingValueChangedForLocalSubtree(MediaValueChange::kOther);
 
+  // Fullscreen element has its own document and uses the viewport media queries,
+  // so we need to make sure the media queries are re-evaluated.
+  if (Element* fullscreen = Fullscreen::FullscreenElementFrom(*GetDocument())) {
+    GetDocument()->GetStyleEngine().MarkAllElementsForStyleRecalc(
+        StyleChangeReasonForTracing::Create(style_change_reason::kFullscreen));
+    CSSDefaultStyleSheets::Instance()
+        .RebuildFullscreenRuleSetIfMediaQueriesChanged(*fullscreen);
+  }
+
   // Also need to update the environment variables related to window segments.
   UpdateViewportSegmentCSSEnvironmentVariables(window_segments);
 }
@@ -1491,9 +1502,21 @@ void LocalFrame::UpdateViewportSegmentCSSEnvironmentVariables(
 
   // Update the variable values on the root instance so that documents that
   // are created after the values change automatically have the right values.
-  StyleEnvironmentVariables& vars =
-      StyleEnvironmentVariables::GetRootInstance();
+  UpdateViewportSegmentCSSEnvironmentVariables(
+      StyleEnvironmentVariables::GetRootInstance(), window_segments);
 
+  if (Element* fullscreen = Fullscreen::FullscreenElementFrom(*GetDocument())) {
+    // Fullscreen has its own document so we need to update its variables as
+    // well.
+    UpdateViewportSegmentCSSEnvironmentVariables(
+        fullscreen->GetDocument().GetStyleEngine().EnsureEnvironmentVariables(),
+        window_segments);
+  }
+}
+
+void LocalFrame::UpdateViewportSegmentCSSEnvironmentVariables(
+    StyleEnvironmentVariables& vars,
+    const WebVector<gfx::Rect>& window_segments) {
   // Unset all variables, since they will be set as a whole by the code below.
   // Since the number and configurations of the segments can change, and
   // removing variables clears all values that have previously been set,

diff --git a/third_party/blink/renderer/core/frame/local_frame.h b/third_party/blink/renderer/core/frame/local_frame.h
@@ -70,6 +70,7 @@
 #include "third_party/blink/public/platform/web_vector.h"
 #include "third_party/blink/public/web/web_script_execution_callback.h"
 #include "third_party/blink/renderer/core/core_export.h"
+#include "third_party/blink/renderer/core/css/document_style_environment_variables.h"
 #include "third_party/blink/renderer/core/dom/weak_identifier_map.h"
 #include "third_party/blink/renderer/core/editing/forward.h"
 #include "third_party/blink/renderer/core/editing/iterators/text_iterator_behavior.h"
@@ -418,6 +419,9 @@ class CORE_EXPORT LocalFrame final
   void WindowSegmentsChanged(const WebVector<gfx::Rect>& window_segments);
   void UpdateViewportSegmentCSSEnvironmentVariables(
       const WebVector<gfx::Rect>& window_segments);
+  void UpdateViewportSegmentCSSEnvironmentVariables(
+      StyleEnvironmentVariables& vars,
+      const WebVector<gfx::Rect>& window_segments);
 
   device::mojom::blink::DevicePostureType GetDevicePosture();
 

diff --git a/third_party/blink/renderer/core/fullscreen/fullscreen.cc b/third_party/blink/renderer/core/fullscreen/fullscreen.cc
@@ -72,7 +72,7 @@ void FullscreenElementChanged(Document& document,
                               const FullscreenOptions* new_options) {
   DCHECK_NE(old_element, new_element);
 
-  document.GetStyleEngine().EnsureUAStyleForFullscreen();
+  document.GetStyleEngine().EnsureUAStyleForFullscreen(*new_element);
 
   if (old_element) {
     DCHECK_NE(old_element, Fullscreen::FullscreenElementFrom(document));