Changeset 239381 in webkit

Timestamp:

Dec 19, 2018, 11:11:06 AM (8 years ago)

Author:

Ryan Haddad

Message:

Unreviewed, rolling out r239347.

Caused the leaks bot to hit an exception and the new test
crashes on certain configurations.

Reverted changeset:

"Synchronous media query evaluation could destroy current
Frame/FrameView."
​https://bugs.webkit.org/show_bug.cgi?id=192781
​https://trac.webkit.org/changeset/239347

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/printing/print-with-media-query-destory-expected.txt (deleted)
• LayoutTests/printing/print-with-media-query-destory.html (deleted)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/DocumentLoader.cpp (modified) (1 diff)
• Source/WebCore/page/Frame.cpp (modified) (2 diffs)
• Source/WebCore/page/FrameView.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-12-19  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r239347.
+
+        Caused the leaks bot to hit an exception and the new test
+        crashes on certain configurations.
+
+        Reverted changeset:
+
+        "Synchronous media query evaluation could destroy current
+        Frame/FrameView."
+        https://bugs.webkit.org/show_bug.cgi?id=192781
+        https://trac.webkit.org/changeset/239347
+
 2018-12-18  Justin Michaud  <justin_michaud@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-12-19  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r239347.
+
+        Caused the leaks bot to hit an exception and the new test
+        crashes on certain configurations.
+
+        Reverted changeset:
+
+        "Synchronous media query evaluation could destroy current
+        Frame/FrameView."
+        https://bugs.webkit.org/show_bug.cgi?id=192781
+        https://trac.webkit.org/changeset/239347
+
 2018-12-19  Truitt Savell  <tsavell@apple.com>
 

trunk/Source/WebCore/loader/DocumentLoader.cpp

@@ -436 +436 @@
         return;
     clearMainResourceLoader();
-    if (!frameLoader())
-        return;
     if (!frameLoader()->stateMachine().creatingInitialEmptyDocument())
         frameLoader()->checkLoadComplete();

trunk/Source/WebCore/page/Frame.cpp

@@ -662 +662 @@
 void Frame::setPrinting(bool printing, const FloatSize& pageSize, const FloatSize& originalPageSize, float maximumShrinkRatio, AdjustViewSizeOrNot shouldAdjustViewSize)
 {
-    if (!view())
-        return;
     // In setting printing, we should not validate resources already cached for the document.
     // See https://bugs.webkit.org/show_bug.cgi?id=43704
@@ -669 +667 @@
 
     m_doc->setPrinting(printing);
-    auto& frameView = *view();
-    frameView.adjustMediaTypeForPrinting(printing);
-
-    m_doc->styleScope().didChangeStyleSheetEnvironment();
-    if (shouldUsePrintingLayout())
-        frameView.forceLayoutForPagination(pageSize, originalPageSize, maximumShrinkRatio, shouldAdjustViewSize);
-    else {
-        frameView.forceLayout();
-        if (shouldAdjustViewSize == AdjustViewSize)
-            frameView.adjustViewSize();
+    if (auto* frameView = view()) {
+        frameView->adjustMediaTypeForPrinting(printing);
+
+        m_doc->styleScope().didChangeStyleSheetEnvironment();
+        if (shouldUsePrintingLayout())
+            frameView->forceLayoutForPagination(pageSize, originalPageSize, maximumShrinkRatio, shouldAdjustViewSize);
+        else {
+            frameView->forceLayout();
+            if (shouldAdjustViewSize == AdjustViewSize)
+                frameView->adjustViewSize();
+        }
     }
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -4587 +4587 @@
 void FrameView::forceLayoutForPagination(const FloatSize& pageSize, const FloatSize& originalPageSize, float maximumShrinkFactor, AdjustViewSizeOrNot shouldAdjustViewSize)
 {
-    if (!renderView())
-        return;
-
-    Ref<FrameView> protectedThis(*this);
-    auto& renderView = *this->renderView();
-
     // Dumping externalRepresentation(frame().renderer()).ascii() is a good trick to see
     // the state of things before and after the layout
-    float pageLogicalWidth = renderView.style().isHorizontalWritingMode() ? pageSize.width() : pageSize.height();
-    float pageLogicalHeight = renderView.style().isHorizontalWritingMode() ? pageSize.height() : pageSize.width();
-
-    renderView.setPageLogicalSize({ floor(pageLogicalWidth), floor(pageLogicalHeight) });
-    renderView.setNeedsLayoutAndPrefWidthsRecalc();
-    forceLayout();
-    if (hasOneRef())
-        return;
-
-    // If we don't fit in the given page width, we'll lay out again. If we don't fit in the
-    // page width when shrunk, we will lay out at maximum shrink and clip extra content.
-    // FIXME: We are assuming a shrink-to-fit printing implementation. A cropping
-    // implementation should not do this!
-    bool horizontalWritingMode = renderView.style().isHorizontalWritingMode();
-    const LayoutRect& documentRect = renderView.documentRect();
-    LayoutUnit docLogicalWidth = horizontalWritingMode ? documentRect.width() : documentRect.height();
-    if (docLogicalWidth > pageLogicalWidth) {
-        int expectedPageWidth = std::min<float>(documentRect.width(), pageSize.width() * maximumShrinkFactor);
-        int expectedPageHeight = std::min<float>(documentRect.height(), pageSize.height() * maximumShrinkFactor);
-        FloatSize maxPageSize = frame().resizePageRectsKeepingRatio(FloatSize(originalPageSize.width(), originalPageSize.height()), FloatSize(expectedPageWidth, expectedPageHeight));
-        pageLogicalWidth = horizontalWritingMode ? maxPageSize.width() : maxPageSize.height();
-        pageLogicalHeight = horizontalWritingMode ? maxPageSize.height() : maxPageSize.width();
-
-        renderView.setPageLogicalSize({ floor(pageLogicalWidth), floor(pageLogicalHeight) });
-        renderView.setNeedsLayoutAndPrefWidthsRecalc();
+    if (RenderView* renderView = this->renderView()) {
+        float pageLogicalWidth = renderView->style().isHorizontalWritingMode() ? pageSize.width() : pageSize.height();
+        float pageLogicalHeight = renderView->style().isHorizontalWritingMode() ? pageSize.height() : pageSize.width();
+
+        renderView->setPageLogicalSize({ floor(pageLogicalWidth), floor(pageLogicalHeight) });
+        renderView->setNeedsLayoutAndPrefWidthsRecalc();
         forceLayout();
-        if (hasOneRef())
-            return;
-
-        const LayoutRect& updatedDocumentRect = renderView.documentRect();
-        LayoutUnit docLogicalHeight = horizontalWritingMode ? updatedDocumentRect.height() : updatedDocumentRect.width();
-        LayoutUnit docLogicalTop = horizontalWritingMode ? updatedDocumentRect.y() : updatedDocumentRect.x();
-        LayoutUnit docLogicalRight = horizontalWritingMode ? updatedDocumentRect.maxX() : updatedDocumentRect.maxY();
-        LayoutUnit clippedLogicalLeft;
-        if (!renderView.style().isLeftToRightDirection())
-            clippedLogicalLeft = docLogicalRight - pageLogicalWidth;
-        LayoutRect overflow(clippedLogicalLeft, docLogicalTop, pageLogicalWidth, docLogicalHeight);
-
-        if (!horizontalWritingMode)
-            overflow = overflow.transposedRect();
-        renderView.clearLayoutOverflow();
-        renderView.addLayoutOverflow(overflow); // This is how we clip in case we overflow again.
+
+        // If we don't fit in the given page width, we'll lay out again. If we don't fit in the
+        // page width when shrunk, we will lay out at maximum shrink and clip extra content.
+        // FIXME: We are assuming a shrink-to-fit printing implementation.  A cropping
+        // implementation should not do this!
+        bool horizontalWritingMode = renderView->style().isHorizontalWritingMode();
+        const LayoutRect& documentRect = renderView->documentRect();
+        LayoutUnit docLogicalWidth = horizontalWritingMode ? documentRect.width() : documentRect.height();
+        if (docLogicalWidth > pageLogicalWidth) {
+            int expectedPageWidth = std::min<float>(documentRect.width(), pageSize.width() * maximumShrinkFactor);
+            int expectedPageHeight = std::min<float>(documentRect.height(), pageSize.height() * maximumShrinkFactor);
+            FloatSize maxPageSize = frame().resizePageRectsKeepingRatio(FloatSize(originalPageSize.width(), originalPageSize.height()), FloatSize(expectedPageWidth, expectedPageHeight));
+            pageLogicalWidth = horizontalWritingMode ? maxPageSize.width() : maxPageSize.height();
+            pageLogicalHeight = horizontalWritingMode ? maxPageSize.height() : maxPageSize.width();
+
+            renderView->setPageLogicalSize({ floor(pageLogicalWidth), floor(pageLogicalHeight) });
+            renderView->setNeedsLayoutAndPrefWidthsRecalc();
+            forceLayout();
+
+            const LayoutRect& updatedDocumentRect = renderView->documentRect();
+            LayoutUnit docLogicalHeight = horizontalWritingMode ? updatedDocumentRect.height() : updatedDocumentRect.width();
+            LayoutUnit docLogicalTop = horizontalWritingMode ? updatedDocumentRect.y() : updatedDocumentRect.x();
+            LayoutUnit docLogicalRight = horizontalWritingMode ? updatedDocumentRect.maxX() : updatedDocumentRect.maxY();
+            LayoutUnit clippedLogicalLeft;
+            if (!renderView->style().isLeftToRightDirection())
+                clippedLogicalLeft = docLogicalRight - pageLogicalWidth;
+            LayoutRect overflow(clippedLogicalLeft, docLogicalTop, pageLogicalWidth, docLogicalHeight);
+
+            if (!horizontalWritingMode)
+                overflow = overflow.transposedRect();
+            renderView->clearLayoutOverflow();
+            renderView->addLayoutOverflow(overflow); // This is how we clip in case we overflow again.
+        }
     }