Bug 1853455 - Fix nsIFrame::ComputePageValue to check for page style values, rather than checking the start page value. r=dholbert

Because we use NULL to indicate the auto value, once a page contributes a
non-auto page value then that will be all that is seen by ComputePageValue
when a later page doesn't have a page-name at all.

Actually checking for the page-name value will fix page-name values when
an empty/auto page-name follows pages with a non-auto value. It is also
slightly more efficient, as it avoids a frame property lookup and instead
just looks at the computed style.

While we are here, remove an assert that is clearly not possible to occur
anymore, and add a short explanation of the code in ComputePageValue.

Differential Revision: https://phabricator.services.mozilla.com/D188544

layout/base/nsCSSFrameConstructor.cpp

@@ -1350,16 +1350,6 @@ static void MoveChildrenTo(nsIFrame* aOldParent, nsContainerFrame* aNewParent,
   }
 }
 
-static bool MaybeApplyPageName(nsFrameConstructorState& aState,
-                               const StylePageName& aPageName) {
-  if (aPageName.IsPageName()) {
-    aState.mAutoPageNameValue = aPageName.AsPageName().AsAtom();
-    return true;
-  }
-  MOZ_ASSERT(aPageName.IsAuto(), "Impossible page name");
-  return false;
-}
-
 static void EnsureAutoPageName(nsFrameConstructorState& aState,
                                const nsContainerFrame* const aFrame) {
   // Check if we need to figure out our used page name.
@@ -1372,8 +1362,8 @@ static void EnsureAutoPageName(nsFrameConstructorState& aState,
 
   for (const nsContainerFrame* frame = aFrame; frame;
        frame = frame->GetParent()) {
-    const StylePageName& pageName = frame->StylePage()->mPage;
-    if (MaybeApplyPageName(aState, pageName)) {
+    if (const nsAtom* maybePageName = frame->GetStylePageName()) {
+      aState.mAutoPageNameValue = maybePageName;
       return;
     }
   }
@@ -1402,7 +1392,9 @@ nsCSSFrameConstructor::AutoFrameConstructionPageName::
 
   MOZ_ASSERT(mNameToRestore,
              "Page name should have been found by EnsureAutoPageName");
-  MaybeApplyPageName(aState, aFrame->StylePage()->mPage);
+  if (const nsAtom* maybePageName = aFrame->GetStylePageName()) {
+    aState.mAutoPageNameValue = maybePageName;
+  }
   aFrame->SetAutoPageValue(aState.mAutoPageNameValue);
 }
 

layout/generic/nsIFrame.cpp

@@ -2135,12 +2135,17 @@ nsIFrame::CaretBlockAxisMetrics nsIFrame::GetCaretBlockAxisMetrics(
 const nsAtom* nsIFrame::ComputePageValue() const {
   const nsAtom* value = nsGkAtoms::_empty;
   const nsIFrame* frame = this;
+  // Find what CSS page name value this frame's subtree has, if any.
+  // Starting with this frame, check if a page name other than auto is present,
+  // and record it if so. Then, if the current frame is a container frame, find
+  // the first non-placeholder child and repeat.
+  // This will find the most deeply nested first in-flow child of this frame's
+  // subtree, and return its page name (with auto resolved if applicable, and
+  // subtrees with no page-names returning the empty atom rather than null).
   do {
-    // If this has a non-auto start value, track that instead.
-    if (const nsAtom* const startValue = frame->GetStartPageValue()) {
-      value = startValue;
+    if (const nsAtom* maybePageName = frame->GetStylePageName()) {
+      value = maybePageName;
     }
-    MOZ_ASSERT(value, "Should not have a NULL page value.");
     // Get the next frame to read from.
     const nsIFrame* firstNonPlaceholderFrame = nullptr;
     // If this is a container frame, inspect its in-flow children.

layout/generic/nsIFrame.h

@@ -1322,6 +1322,17 @@ class nsIFrame : public nsQueryFrame {
     return nullptr;
   }
 
+  // Returns the page name based on style information for this frame, or null
+  // if the value is auto.
+  const nsAtom* GetStylePageName() const {
+    const mozilla::StylePageName& pageName = StylePage()->mPage;
+    if (pageName.IsPageName()) {
+      return pageName.AsPageName().AsAtom();
+    }
+    MOZ_ASSERT(pageName.IsAuto(), "Impossible page name");
+    return nullptr;
+  }
+
  private:
   // The value that the CSS page-name "auto" keyword resolves to for children
   // of this frame.
@@ -1576,6 +1587,16 @@ class nsIFrame : public nsQueryFrame {
                                                  const nsFontMetrics&) const;
   // Gets the page-name value to be used for the page that contains this frame
   // during paginated reflow.
+  // This only inspects the first in-flow child of this frame, and if that
+  // is a container frame then its first in-flow child, until it reaches the
+  // deepest child of the tree. This will correctly find the page-name only for
+  // the first page that this frame is on.
+  // This will resolve auto values, including the case where no frame has a
+  // page-name set in which case it will return the empty atom. It will never
+  // return null.
+  // This is intended to be used either on the root frame to find the first
+  // page's page-name, or on a newly created continuation to find what the new
+  // page's page-name will be.
   const nsAtom* ComputePageValue() const MOZ_NONNULL_RETURN;
 
   ///////////////////////////////////////////////////////////////////////////////

testing/web-platform/meta/css/printing/page-size-008-print.html.ini

@@ -1,2 +0,0 @@
-[page-size-008-print.html]
-  expected: FAIL