Title: [219121] trunk
Revision
219121
Author
an...@apple.com
Date
2017-07-04 10:58:36 -0700 (Tue, 04 Jul 2017)

Log Message

FrameView should not set RenderView::logicalWidth directly for printing
https://bugs.webkit.org/show_bug.cgi?id=174135

Reviewed by Zalan Bujtas.

Source/WebCore:

Renderer logicalWidth should be set by layout. Direct override by RenderView when printing means
that we don't layout children in all cases when the width changes. This is currently mostly hidden
by spurious layouts but causes problems when trying to fix other things that reduces those.

* page/FrameView.cpp:
(WebCore::FrameView::forceLayoutForPagination):

    Instead of calling setLogicalWidth directly call the new setPageLogicalSize that sets both the width
    and the height uniformly.

* rendering/RenderView.cpp:
(WebCore::RenderView::updateLogicalWidth):

    Use pageLogicalSize->width() in printing state instead of skipping the logical width update entirely.
    This ensures that the layout will progress to children when the page logical width changes.

(WebCore::RenderView::initializeLayoutState):
(WebCore::RenderView::layout):
(WebCore::RenderView::pageOrViewLogicalHeight):
(WebCore::RenderView::setPageLogicalSize):
* rendering/RenderView.h:

    Replace the existing m_pageLogicalHeight with std::optional m_pageLogicalSize.

LayoutTests:

* platform/mac/printing/width-overflow-expected.txt:

    This is a progression, view and root element sizes now match.
    Printing output is not affected.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (219120 => 219121)


--- trunk/LayoutTests/ChangeLog	2017-07-04 17:51:30 UTC (rev 219120)
+++ trunk/LayoutTests/ChangeLog	2017-07-04 17:58:36 UTC (rev 219121)
@@ -1,3 +1,15 @@
+2017-07-04  Antti Koivisto  <an...@apple.com>
+
+        FrameView should not set RenderView::logicalWidth directly for printing
+        https://bugs.webkit.org/show_bug.cgi?id=174135
+
+        Reviewed by Zalan Bujtas.
+
+        * platform/mac/printing/width-overflow-expected.txt:
+
+            This is a progression, view and root element sizes now match.
+            Printing output is not affected.
+
 2017-07-03  Saam Barati  <sbar...@apple.com>
 
         LayoutTest workers/bomb.html is a Crash

Modified: trunk/LayoutTests/platform/mac/printing/width-overflow-expected.txt (219120 => 219121)


--- trunk/LayoutTests/platform/mac/printing/width-overflow-expected.txt	2017-07-04 17:51:30 UTC (rev 219120)
+++ trunk/LayoutTests/platform/mac/printing/width-overflow-expected.txt	2017-07-04 17:58:36 UTC (rev 219121)
@@ -1,9 +1,9 @@
 layer at (0,0) size 1300x2284
   RenderView at (0,0) size 1300x2284
-layer at (0,0) size 981x2284
-  RenderBlock {HTML} at (0,0) size 981x2284
-    RenderBody {BODY} at (0,16) size 981x2252
-      RenderBlock {P} at (0,0) size 981x18
+layer at (0,0) size 1300x2284
+  RenderBlock {HTML} at (0,0) size 1300x2284
+    RenderBody {BODY} at (0,16) size 1300x2252
+      RenderBlock {P} at (0,0) size 1300x18
         RenderText {#text} at (0,0) size 765x18
           text run at (0,0) width 765: "To run this test manually, print this page. If the right side of any lines is printed without being truncated, the test passes."
       RenderBlock {DIV} at (0,34) size 1300x2218

Modified: trunk/Source/WebCore/ChangeLog (219120 => 219121)


--- trunk/Source/WebCore/ChangeLog	2017-07-04 17:51:30 UTC (rev 219120)
+++ trunk/Source/WebCore/ChangeLog	2017-07-04 17:58:36 UTC (rev 219121)
@@ -1,3 +1,34 @@
+2017-07-04  Antti Koivisto  <an...@apple.com>
+
+        FrameView should not set RenderView::logicalWidth directly for printing
+        https://bugs.webkit.org/show_bug.cgi?id=174135
+
+        Reviewed by Zalan Bujtas.
+
+        Renderer logicalWidth should be set by layout. Direct override by RenderView when printing means
+        that we don't layout children in all cases when the width changes. This is currently mostly hidden
+        by spurious layouts but causes problems when trying to fix other things that reduces those.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::forceLayoutForPagination):
+
+            Instead of calling setLogicalWidth directly call the new setPageLogicalSize that sets both the width
+            and the height uniformly.
+
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::updateLogicalWidth):
+
+            Use pageLogicalSize->width() in printing state instead of skipping the logical width update entirely.
+            This ensures that the layout will progress to children when the page logical width changes.
+
+        (WebCore::RenderView::initializeLayoutState):
+        (WebCore::RenderView::layout):
+        (WebCore::RenderView::pageOrViewLogicalHeight):
+        (WebCore::RenderView::setPageLogicalSize):
+        * rendering/RenderView.h:
+
+            Replace the existing m_pageLogicalHeight with std::optional m_pageLogicalSize.
+
 2017-07-04  Xabier Rodriguez Calvar  <calva...@igalia.com>
 
         [EME] Solve a couple of compiler warnings

Modified: trunk/Source/WebCore/page/FrameView.cpp (219120 => 219121)


--- trunk/Source/WebCore/page/FrameView.cpp	2017-07-04 17:51:30 UTC (rev 219120)
+++ trunk/Source/WebCore/page/FrameView.cpp	2017-07-04 17:58:36 UTC (rev 219121)
@@ -4676,8 +4676,7 @@
         float pageLogicalWidth = renderView->style().isHorizontalWritingMode() ? pageSize.width() : pageSize.height();
         float pageLogicalHeight = renderView->style().isHorizontalWritingMode() ? pageSize.height() : pageSize.width();
 
-        renderView->setLogicalWidth(floor(pageLogicalWidth));
-        renderView->setPageLogicalHeight(floor(pageLogicalHeight));
+        renderView->setPageLogicalSize({ floor(pageLogicalWidth), floor(pageLogicalHeight) });
         renderView->setNeedsLayoutAndPrefWidthsRecalc();
         forceLayout();
 
@@ -4695,8 +4694,7 @@
             pageLogicalWidth = horizontalWritingMode ? maxPageSize.width() : maxPageSize.height();
             pageLogicalHeight = horizontalWritingMode ? maxPageSize.height() : maxPageSize.width();
 
-            renderView->setLogicalWidth(floor(pageLogicalWidth));
-            renderView->setPageLogicalHeight(floor(pageLogicalHeight));
+            renderView->setPageLogicalSize({ floor(pageLogicalWidth), floor(pageLogicalHeight) });
             renderView->setNeedsLayoutAndPrefWidthsRecalc();
             forceLayout();
 

Modified: trunk/Source/WebCore/rendering/RenderView.cpp (219120 => 219121)


--- trunk/Source/WebCore/rendering/RenderView.cpp	2017-07-04 17:51:30 UTC (rev 219120)
+++ trunk/Source/WebCore/rendering/RenderView.cpp	2017-07-04 17:58:36 UTC (rev 219121)
@@ -217,8 +217,7 @@
 
 void RenderView::updateLogicalWidth()
 {
-    if (!shouldUsePrintingLayout())
-        setLogicalWidth(viewLogicalWidth());
+    setLogicalWidth(shouldUsePrintingLayout() ? m_pageLogicalSize->width() : LayoutUnit(viewLogicalWidth()));
 }
 
 LayoutUnit RenderView::availableLogicalHeight(AvailableLogicalHeightType) const
@@ -268,7 +267,7 @@
     // FIXME: May be better to push a clip and avoid issuing offscreen repaints.
     state.m_clipped = false;
 
-    state.m_pageLogicalHeight = m_pageLogicalHeight;
+    state.m_pageLogicalHeight = m_pageLogicalSize ? m_pageLogicalSize->height() : LayoutUnit(0);
     state.m_pageLogicalHeightChanged = m_pageLogicalHeightChanged;
     ASSERT(state.m_pageLogicalHeight >= 0);
     state.m_isPaginated = state.m_pageLogicalHeight > 0;
@@ -339,10 +338,13 @@
 {
     StackStats::LayoutCheckPoint layoutCheckPoint;
     if (!document().paginated())
-        setPageLogicalHeight(0);
+        m_pageLogicalSize = { };
 
-    if (shouldUsePrintingLayout())
-        m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth = logicalWidth();
+    if (shouldUsePrintingLayout()) {
+        ASSERT(m_pageLogicalSize);
+        m_minPreferredLogicalWidth = m_pageLogicalSize->width();
+        m_maxPreferredLogicalWidth = m_minPreferredLogicalWidth;
+    }
 
     // Use calcWidth/Height to get the new width/height, since this will take the full page zoom factor into account.
     bool relayoutChildren = !shouldUsePrintingLayout() && (width() != viewWidth() || height() != viewHeight());
@@ -386,7 +388,7 @@
 LayoutUnit RenderView::pageOrViewLogicalHeight() const
 {
     if (document().printing())
-        return pageLogicalHeight();
+        return m_pageLogicalSize->height();
     
     if (multiColumnFlowThread() && !style().hasInlineColumnAxis()) {
         if (int pageLength = frameView().pagination().pageLength)
@@ -1219,6 +1221,14 @@
     return height;
 }
 
+void RenderView::setPageLogicalSize(LayoutSize size)
+{
+    if (!m_pageLogicalSize || m_pageLogicalSize->height() != size.height())
+        m_pageLogicalHeightChanged = true;
+
+    m_pageLogicalSize = size;
+}
+
 float RenderView::zoomFactor() const
 {
     return frameView().frame().pageZoomFactor();

Modified: trunk/Source/WebCore/rendering/RenderView.h (219120 => 219121)


--- trunk/Source/WebCore/rendering/RenderView.h	2017-07-04 17:51:30 UTC (rev 219120)
+++ trunk/Source/WebCore/rendering/RenderView.h	2017-07-04 17:58:36 UTC (rev 219121)
@@ -141,14 +141,7 @@
 
     void updateHitTestResult(HitTestResult&, const LayoutPoint&) override;
 
-    LayoutUnit pageLogicalHeight() const { return m_pageLogicalHeight; }
-    void setPageLogicalHeight(LayoutUnit height)
-    {
-        if (m_pageLogicalHeight != height) {
-            m_pageLogicalHeight = height;
-            m_pageLogicalHeightChanged = true;
-        }
-    }
+    void setPageLogicalSize(LayoutSize);
     LayoutUnit pageOrViewLogicalHeight() const;
 
     // This method is used to assign a page number only when pagination modes have
@@ -371,7 +364,7 @@
     HashSet<RenderBox*> m_renderersNeedingLazyRepaint;
 
     std::unique_ptr<ImageQualityController> m_imageQualityController;
-    LayoutUnit m_pageLogicalHeight;
+    std::optional<LayoutSize> m_pageLogicalSize;
     bool m_pageLogicalHeightChanged { false };
     std::unique_ptr<LayoutState> m_layoutState;
     unsigned m_layoutStateDisableCount { 0 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to