Remove the Pagination struct, clean up viewport scroll policy propagation.

Source/core/dom/Document.cpp

Index: Source/core/dom/Document.cpp
diff --git a/Source/core/dom/Document.cpp b/Source/core/dom/Document.cpp
index 3f9fe5d48f1c414e4c990c06de69b89036359ead..04ffab170ce6b2e808c7d0fdeb11a71001916c8c 100644
--- a/Source/core/dom/Document.cpp
+++ b/Source/core/dom/Document.cpp
@@ -342,6 +342,29 @@ static void printNavigationErrorMessage(const Frame& frame, const KURL& activeUR
     frame.domWindow()->printErrorMessage(message);
 }
 
+// Decide which element that is to define the viewport's overflow policy. Any parameter may be
+// 0. Avoid calling Element::renderStyle() here; require style as separate parameters instead. The
+// reason for this is that style may not have been associated with the elements yet - in which case
+// it may have been calculated on the fly (without associating it with the actual element)
+// somewhere. This isn't very pretty, but if we want to avoid duplicating the code that chooses
+// between root and body for overflow propagation, this is about as pretty as it gets.

[rune, 2014/02/07 16:44:27]

You could consolidate this code with the viewportDefiningElement method making
it a member of Document, have rootStyle and bodyStyle as parameters (potentially
default to 0), and retrieve rootStyle and bodyStyle from documentElement() and
body() when passed as 0.

That gives an extra cost of retrieving body() an extra time when called from
inheritHtmlAndBodyElementStyles, but prettier I think.

[mstensho (USE GERRIT), 2014/02/11 10:44:14]

Done.

+// The |bodyElement| passed must either be 0 or set to the document's primary body element.
+static Element* pickViewportDefiningElement(Element* rootElement, RenderStyle* rootStyle, Element* bodyElement, RenderStyle* bodyStyle)

[ojan, 2014/02/08 01:40:19]

Do we need to pass the RenderStyles? Can't we just grab them off
rootElement/bodyElement?

[mstensho (USE GERRIT), 2014/02/11 10:44:14]

That's not always going to work. The comment tries to explain why.

+{
+    // If a BODY element sets non-visible overflow, it is to be propagated to the viewport, as long
+    // as the following conditions are all met:
+    // (1) The root element is HTML.
+    // (2) It is the primary BODY element (we only assert for this, expecting callers to behave).
+    // (3) The root element has visible overflow.
+    // Otherwise it's the root element's properties that are to be propagated.
+    if (!rootElement || !rootStyle)
+        return 0;
+    ASSERT(!bodyElement || bodyElement == bodyElement->document().body());
+    if (bodyElement && bodyStyle && rootStyle->isOverflowVisible() && rootElement->hasTagName(htmlTag))
+        return bodyElement;
+    return rootElement;
+}
+
 uint64_t Document::s_globalTreeVersion = 0;
 
 // This class doesn't work with non-Document ExecutionContext.
@@ -1607,14 +1630,6 @@ void Document::updateDistributionForNodeIfNeeded(Node* node)
 
 void Document::setStyleDependentState(RenderStyle* documentStyle)

[ojan, 2014/02/08 01:40:19]

Nit: maybe rename this method to setupFontBuilder or something since that's all
it does now.

[mstensho (USE GERRIT), 2014/02/11 10:44:14]

Done.

 {
-    const Pagination& pagination = view()->pagination();
-    if (pagination.mode != Pagination::Unpaginated) {
-        Pagination::setStylesForPaginationMode(pagination.mode, documentStyle);
-        documentStyle->setColumnGap(pagination.gap);
-        if (renderView()->hasColumns())
-            renderView()->updateColumnInfoFromStyle(documentStyle);
-    }
-
     FontBuilder fontBuilder;
     fontBuilder.initForStyleResolve(*this, documentStyle, isSVGDocument());
     RefPtr<CSSFontSelector> selector = m_styleEngine->fontSelector();
@@ -1632,10 +1647,11 @@ void Document::inheritHtmlAndBodyElementStyles(StyleRecalcChange change)
 
     WritingMode rootWritingMode = documentElementStyle->writingMode();
     TextDirection rootDirection = documentElementStyle->direction();
-    HTMLElement* body = this->body();
 
+    HTMLElement* body = this->body();
+    RefPtr<RenderStyle> bodyStyle;
     if (body) {
-        RefPtr<RenderStyle> bodyStyle = body->renderStyle();
+        bodyStyle = body->renderStyle();
         if (!bodyStyle || body->needsStyleRecalc() || documentElement()->needsStyleRecalc() || change == Force)
             bodyStyle = ensureStyleResolver().styleForElement(body, documentElementStyle.get());
         if (!writingModeSetOnDocumentElement())
@@ -1644,6 +1660,16 @@ void Document::inheritHtmlAndBodyElementStyles(StyleRecalcChange change)
             rootDirection = bodyStyle->direction();
     }
 
+    RefPtr<RenderStyle> overflowStyle;
+    if (Element* element = pickViewportDefiningElement(documentElement(), documentElementStyle.get(), body, bodyStyle.get())) {
+        if (element == body) {
+            overflowStyle = bodyStyle;
+        } else {
+            ASSERT(element == documentElement());
+            overflowStyle = documentElementStyle;
+        }
+    }
+
     // Resolved rem units are stored in the matched properties cache so we need to make sure to
     // invalidate the cache if the documentElement needed to reattach or the font size changed
     // and then trigger a full document recalc. We also need to clear it here since the
@@ -1657,10 +1683,29 @@ void Document::inheritHtmlAndBodyElementStyles(StyleRecalcChange change)
     }
 
     RefPtr<RenderStyle> documentStyle = renderView()->style();
-    if (documentStyle->writingMode() != rootWritingMode || documentStyle->direction() != rootDirection) {
+    if (documentStyle->writingMode() != rootWritingMode
+        || documentStyle->direction() != rootDirection
+        || (overflowStyle && (documentStyle->overflowX() != overflowStyle->overflowX() || documentStyle->overflowY() != overflowStyle->overflowY()))) {
         RefPtr<RenderStyle> newStyle = RenderStyle::clone(documentStyle.get());
         newStyle->setWritingMode(rootWritingMode);
         newStyle->setDirection(rootDirection);
+        EOverflow overflowX = OAUTO;
+        EOverflow overflowY = OAUTO;
+        if (overflowStyle) {
+            overflowX = overflowStyle->overflowX();
+            overflowY = overflowStyle->overflowY();
+            // Visible overflow on the viewport is meaningless.
+            if (overflowX == OVISIBLE)
+                overflowX = OAUTO;

[ojan, 2014/02/08 01:40:19]

We need to do this because pagination doesn't work with overflow:visible? A
comment explaining why we need to do this might help.

[mstensho (USE GERRIT), 2014/02/11 10:44:14]

overflow:visible is meaningless on the viewport, regardless of pagination. The
spec [1] has this to say: "The 'visible' value when used for the viewport must
be interpreted as 'auto'.". But I'll improve the comment.

[1] http://www.w3.org/TR/CSS2/visufx.html#overflow

+            if (overflowY == OVISIBLE)
+                overflowY = OAUTO;
+
+            // Column-gap is (ab)used by the current paged overflow implementation (in lack of other
+            // ways to specify gaps between pages), so we have to propagate it too.
+            newStyle->setColumnGap(overflowStyle->columnGap());
+        }
+        newStyle->setOverflowX(overflowX);
+        newStyle->setOverflowY(overflowY);
         renderView()->setStyle(newStyle);
         setStyleDependentState(newStyle.get());
     }
@@ -2340,6 +2385,15 @@ HTMLHeadElement* Document::head()
     return 0;
 }
 
+Element* Document::viewportDefiningElement() const
+{
+    Element* rootElement = documentElement();
+    Element* bodyElement = body();
+    return pickViewportDefiningElement(
+        rootElement, rootElement ? rootElement->renderStyle() : 0,
+        bodyElement, bodyElement ? bodyElement->renderStyle() : 0);
+}
+
 void Document::close()
 {
     // FIXME: We should follow the specification more closely: