Bug 1565384 part 1: When an embedded SVG document has an aspect-ratio change, trigger a reflow if 'object-fit' is set on the embedding element. r=emilio
authorDaniel Holbert <dholbert@cs.stanford.edu>
Wed, 17 Jul 2019 00:58:57 +0000
changeset 483037 0c49c78a3216187dc2836768311f5c5039caae32
parent 483036 b290783122001a9299a6578c11d5ea501a8fd0f4
child 483038 8231ec90138863906788b5e73f8781012be30531
push id36303
push userdvarga@mozilla.com
push dateWed, 17 Jul 2019 09:36:40 +0000
treeherdermozilla-central@29e9dde37bd2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1565384
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1565384 part 1: When an embedded SVG document has an aspect-ratio change, trigger a reflow if 'object-fit' is set on the embedding element. r=emilio If the embedding element uses `object-fit`, then that indicates it's precisely positioning and/or sizing the embedded SVG document's viewport to fit inside the embedding element's content area. So, when the internal SVG viewBox changes, then the embedding element needs to redo that positioning/sizing. For now, this specifically requires a reflow (and in particular, the nsViewManager adjustments at the end of nsSubDocumentFrame::Reflow). Differential Revision: https://phabricator.services.mozilla.com/D37910
layout/reftests/w3c-css/submitted/images3/object-fit-dyn-aspect-ratio-001-ref.html
layout/reftests/w3c-css/submitted/images3/object-fit-dyn-aspect-ratio-001.html
layout/reftests/w3c-css/submitted/images3/object-fit-dyn-aspect-ratio-002-ref.html
layout/reftests/w3c-css/submitted/images3/object-fit-dyn-aspect-ratio-002.html
layout/reftests/w3c-css/submitted/images3/reftest.list
layout/svg/nsSVGOuterSVGFrame.cpp
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/images3/object-fit-dyn-aspect-ratio-001-ref.html
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>CSS Reftest Reference</title>
+    <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
+    <style>
+      div {
+        margin: 1px;
+        background: lime;
+        float: left;
+      }
+
+      .square {
+        width: 24px;
+        height: 24px;
+      }
+      .bigWide {
+        width: 48px;
+        height: 32px;
+      }
+      .bigTall {
+        width: 32px;
+        height: 48px;
+      }
+      .small {
+        width: 8px;
+        height: 8px;
+      }
+
+      br { clear: both; }
+
+    </style>
+  </head>
+  <body>
+    <!-- Note: the specified heights here are just 1/2 the widths. -->
+    <div class="square"></div>
+    <div class="square" style="height: 12px"></div>
+    <br>
+    <div class="bigWide"></div>
+    <div class="bigWide" style="height: 24px"></div>
+    <br>
+    <div class="bigTall"></div>
+    <div class="bigTall" style="height: 16px"></div>
+    <br>
+    <div class="small"></div>
+    <div class="small" style="height: 4px"></div>
+    <br>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/images3/object-fit-dyn-aspect-ratio-001.html
@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>CSS Test: 'object-fit: contain' and 'cover' on object element whose aspect ratio dynamically changes</title>
+    <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
+    <link rel="help" href="http://www.w3.org/TR/css3-images/#sizing">
+    <link rel="help" href="http://www.w3.org/TR/css3-images/#the-object-fit">
+    <link rel="match" href="object-fit-dyn-aspect-ratio-001-ref.html">
+    <style>
+      object {
+        margin: 1px;
+        float: left;
+        /* I'm just using 'object-position' for cosmetic reasons, so that the
+           painted areas are all snapped to top-left which makes reference case
+           more trivial. */
+        object-position: top left;
+      }
+      .cov { object-fit: cover;   }
+      .con { object-fit: contain; }
+
+      .square {
+        width: 24px;
+        height: 24px;
+      }
+      .bigWide {
+        width: 48px;
+        height: 32px;
+      }
+      .bigTall {
+        width: 32px;
+        height: 48px;
+      }
+      .small {
+        width: 8px;
+        height: 8px;
+      }
+
+      br { clear: both; }
+
+    </style>
+    <script>
+      function go() {
+        for (let elem of document.getElementsByTagName("object")) {
+          var doc = elem.contentDocument;
+          /* These should all should select out a piece of the
+             bottom-right quadrant (the lime chunk): */
+          doc.documentElement.setAttribute("viewBox", "8 4 8 4");
+        }
+      }
+    </script>
+  </head>
+  <body onload="go()">
+    <object data="support/colors-16x8-parDefault.svg" class="square cov"></object>
+    <object data="support/colors-16x8-parDefault.svg" class="square con"></object>
+    <br>
+    <object data="support/colors-16x8-parDefault.svg" class="bigWide cov"></object>
+    <object data="support/colors-16x8-parDefault.svg" class="bigWide con"></object>
+    <br>
+    <object data="support/colors-16x8-parDefault.svg" class="bigTall cov"></object>
+    <object data="support/colors-16x8-parDefault.svg" class="bigTall con"></object>
+    <br>
+    <object data="support/colors-16x8-parDefault.svg" class="small cov"></object>
+    <object data="support/colors-16x8-parDefault.svg" class="small con"></object>
+    <br>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/images3/object-fit-dyn-aspect-ratio-002-ref.html
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>CSS Reftest Reference</title>
+    <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
+    <style>
+      div {
+        margin: 1px;
+        background: lime;
+        float: left;
+      }
+
+      .square {
+        width: 24px;
+        height: 24px;
+      }
+      .bigWide {
+        width: 48px;
+        height: 32px;
+      }
+      .bigTall {
+        width: 32px;
+        height: 48px;
+      }
+      .small {
+        width: 8px;
+        height: 8px;
+      }
+
+      br { clear: both; }
+
+    </style>
+  </head>
+  <body>
+    <!-- Note: each inline-style specified width here is just using the final
+         viewBox aspect-ratio (1/2) times the element's specified height. This
+         is how wide the concrete object size[1] should end up, for the
+         testcase's "object-fit:contain" elements.
+        [1] https://drafts.csswg.org/css-images-3/#concrete-object-size -->
+    <div class="square"></div>
+    <div class="square" style="width: 12px"></div>
+    <br>
+    <div class="bigWide"></div>
+    <div class="bigWide" style="width: 16px"></div>
+    <br>
+    <div class="bigTall"></div>
+    <div class="bigTall" style="width: 24px"></div>
+    <br>
+    <div class="small"></div>
+    <div class="small" style="width: 4px"></div>
+    <br>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/images3/object-fit-dyn-aspect-ratio-002.html
@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>CSS Test: 'object-fit: contain' and 'cover' on object element whose aspect ratio dynamically changes</title>
+    <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
+    <link rel="help" href="http://www.w3.org/TR/css3-images/#sizing">
+    <link rel="help" href="http://www.w3.org/TR/css3-images/#the-object-fit">
+    <link rel="match" href="object-fit-dyn-aspect-ratio-002-ref.html">
+    <style>
+      object {
+        margin: 1px;
+        float: left;
+        /* I'm just using 'object-position' for cosmetic reasons, so that the
+           painted areas are all snapped to top-left which makes reference case
+           more trivial. */
+        object-position: top left;
+      }
+      .cov { object-fit: cover;   }
+      .con { object-fit: contain; }
+
+      .square {
+        width: 24px;
+        height: 24px;
+      }
+      .bigWide {
+        width: 48px;
+        height: 32px;
+      }
+      .bigTall {
+        width: 32px;
+        height: 48px;
+      }
+      .small {
+        width: 8px;
+        height: 8px;
+      }
+
+      br { clear: both; }
+
+    </style>
+    <script>
+      function go() {
+        for (let elem of document.getElementsByTagName("object")) {
+          var doc = elem.contentDocument;
+          /* These should all should select out a piece of the
+             bottom-right quadrant (the lime chunk): */
+          doc.documentElement.setAttribute("viewBox", "4 8 4 8");
+        }
+      }
+    </script>
+  </head>
+  <body onload="go()">
+    <object data="support/colors-8x16-parDefault.svg" class="square cov"></object>
+    <object data="support/colors-8x16-parDefault.svg" class="square con"></object>
+    <br>
+    <object data="support/colors-8x16-parDefault.svg" class="bigWide cov"></object>
+    <object data="support/colors-8x16-parDefault.svg" class="bigWide con"></object>
+    <br>
+    <object data="support/colors-8x16-parDefault.svg" class="bigTall cov"></object>
+    <object data="support/colors-8x16-parDefault.svg" class="bigTall con"></object>
+    <br>
+    <object data="support/colors-8x16-parDefault.svg" class="small cov"></object>
+    <object data="support/colors-8x16-parDefault.svg" class="small con"></object>
+    <br>
+  </body>
+</html>
--- a/layout/reftests/w3c-css/submitted/images3/reftest.list
+++ b/layout/reftests/w3c-css/submitted/images3/reftest.list
@@ -1,8 +1,12 @@
+# Tests for dynamic change to aspect ratio on element with 'object-fit' set
+== object-fit-dyn-aspect-ratio-001.html object-fit-dyn-aspect-ratio-001-ref.html
+== object-fit-dyn-aspect-ratio-002.html object-fit-dyn-aspect-ratio-002-ref.html
+
 # Tests for 'object-fit' / 'object-position' with a PNG image
 == object-fit-fill-png-001c.html object-fit-fill-png-001-ref.html
 == object-fit-fill-png-001e.html object-fit-fill-png-001-ref.html
 == object-fit-fill-png-001i.html object-fit-fill-png-001-ref.html
 == object-fit-fill-png-001o.html object-fit-fill-png-001-ref.html
 == object-fit-fill-png-001p.html object-fit-fill-png-001-ref.html
 == object-fit-fill-png-002c.html object-fit-fill-png-002-ref.html
 == object-fit-fill-png-002e.html object-fit-fill-png-002-ref.html
--- a/layout/svg/nsSVGOuterSVGFrame.cpp
+++ b/layout/svg/nsSVGOuterSVGFrame.cpp
@@ -125,24 +125,32 @@ void nsSVGOuterSVGFrame::Init(nsIContent
   if (doc) {
     // we only care about our content's zoom and pan values if it's the root
     // element
     if (doc->GetRootElement() == mContent) {
       mIsRootContent = true;
 
       nsIFrame* embeddingFrame;
       if (IsRootOfReplacedElementSubDoc(&embeddingFrame) && embeddingFrame) {
-        if (MOZ_UNLIKELY(!embeddingFrame->HasAllStateBits(NS_FRAME_IS_DIRTY)) &&
-            DependsOnIntrinsicSize(embeddingFrame)) {
-          // Looks like this document is loading after the embedding element
-          // has had its first reflow, and that its size depends on our
-          // intrinsic size.  We need it to resize itself to use our (now
-          // available) intrinsic size:
-          embeddingFrame->PresShell()->FrameNeedsReflow(
-              embeddingFrame, IntrinsicDirty::StyleChange, NS_FRAME_IS_DIRTY);
+        if (MOZ_UNLIKELY(!embeddingFrame->HasAllStateBits(NS_FRAME_IS_DIRTY))) {
+          bool dependsOnIntrinsicSize = DependsOnIntrinsicSize(embeddingFrame);
+          if (dependsOnIntrinsicSize ||
+              embeddingFrame->StylePosition()->mObjectFit !=
+                  NS_STYLE_OBJECT_FIT_FILL) {
+            // Looks like this document is loading after the embedding element
+            // has had its first reflow, and it cares about our intrinsic size
+            // (either for determining its own size, or for sizing/positioning
+            // its view to honor "object-fit").  We need it to reflow itself to
+            // use our (now-available) intrinsic size:
+            auto dirtyHint = dependsOnIntrinsicSize
+                                 ? IntrinsicDirty::StyleChange
+                                 : IntrinsicDirty::Resize;
+            embeddingFrame->PresShell()->FrameNeedsReflow(
+                embeddingFrame, dirtyHint, NS_FRAME_IS_DIRTY);
+          }
         }
       }
     }
   }
 }
 
 //----------------------------------------------------------------------
 // nsQueryFrame methods
@@ -692,23 +700,29 @@ nsresult nsSVGOuterSVGFrame::AttributeCh
     }
     if (aAttribute == nsGkAtoms::width || aAttribute == nsGkAtoms::height ||
         aAttribute == nsGkAtoms::viewBox) {
       // Don't call ChildrenOnlyTransformChanged() here, since we call it
       // under Reflow if the width/height/viewBox actually changed.
 
       nsIFrame* embeddingFrame;
       if (IsRootOfReplacedElementSubDoc(&embeddingFrame) && embeddingFrame) {
-        if (DependsOnIntrinsicSize(embeddingFrame)) {
+        bool dependsOnIntrinsicSize = DependsOnIntrinsicSize(embeddingFrame);
+        if (dependsOnIntrinsicSize ||
+            embeddingFrame->StylePosition()->mObjectFit !=
+                NS_STYLE_OBJECT_FIT_FILL) {
           // Tell embeddingFrame's presShell it needs to be reflowed (which
-          // takes care of reflowing us too).
+          // takes care of reflowing us too). And if it depends on our
+          // intrinsic size, then we need to invalidate its intrinsic sizes
+          // (via the IntrinsicDirty::StyleChange hint.)
+          auto dirtyHint = dependsOnIntrinsicSize ? IntrinsicDirty::StyleChange
+                                                  : IntrinsicDirty::Resize;
           embeddingFrame->PresShell()->FrameNeedsReflow(
-              embeddingFrame, IntrinsicDirty::StyleChange, NS_FRAME_IS_DIRTY);
-        }
-        // else our width and height is overridden - don't reflow anything
+              embeddingFrame, dirtyHint, NS_FRAME_IS_DIRTY);
+        }  // else our width/height/viewBox are irrelevant to the outer doc.
       } else {
         // We are not embedded by reference, so our 'width' and 'height'
         // attributes are not overridden (and viewBox may influence our
         // intrinsic aspect ratio).  We need to reflow.
         PresShell()->FrameNeedsReflow(this, IntrinsicDirty::StyleChange,
                                       NS_FRAME_IS_DIRTY);
       }
     }