Synchronous media query evaluation could destroy current Frame/FrameView.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Dec 2018 19:57:14 +0000 (19:57 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Dec 2018 19:57:14 +0000 (19:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192781
<rdar://problem/34416793>

Reviewed by Chris Dumez.

Source/WebCore:

Protect Frame and FrameView when coming back from printing and check if the current Frame/FrameView/FrameLoader objects are still valid.

Test: printing/print-with-media-query-destory.html

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::finishedLoading):
* page/Frame.cpp:
(WebCore::Frame::setPrinting):
* page/FrameView.cpp:
(WebCore::FrameView::forceLayoutForPagination):
* page/PrintContext.cpp:
(WebCore::PrintContext::PrintContext):
(WebCore::PrintContext::computePageRects):
(WebCore::PrintContext::computePageRectsWithPageSizeInternal):
(WebCore::PrintContext::begin):
(WebCore::PrintContext::computeAutomaticScaleFactor):
(WebCore::PrintContext::spoolPage):
(WebCore::PrintContext::spoolRect):
(WebCore::PrintContext::end):
* page/PrintContext.h:
(WebCore::PrintContext::frame const): Deleted.

LayoutTests:

* printing/print-with-media-query-destory-expected.txt: Added.
* printing/print-with-media-query-destory.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239512 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/printing/print-with-media-query-destory-expected.txt [new file with mode: 0644]
LayoutTests/printing/print-with-media-query-destory.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/page/Frame.cpp
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/PrintContext.cpp
Source/WebCore/page/PrintContext.h

index ff8a86d..ffd7c51 100644 (file)
@@ -1,3 +1,14 @@
+2018-12-21  Zalan Bujtas  <zalan@apple.com>
+
+        Synchronous media query evaluation could destroy current Frame/FrameView.
+        https://bugs.webkit.org/show_bug.cgi?id=192781
+        <rdar://problem/34416793>
+
+        Reviewed by Chris Dumez.
+
+        * printing/print-with-media-query-destory-expected.txt: Added.
+        * printing/print-with-media-query-destory.html: Added.
+
 2018-12-21  Alex Christensen  <achristensen@webkit.org>
 
         Revert r239503.
diff --git a/LayoutTests/printing/print-with-media-query-destory-expected.txt b/LayoutTests/printing/print-with-media-query-destory-expected.txt
new file mode 100644 (file)
index 0000000..6e69738
--- /dev/null
@@ -0,0 +1,2 @@
+Pass if no crash or assert
+
diff --git a/LayoutTests/printing/print-with-media-query-destory.html b/LayoutTests/printing/print-with-media-query-destory.html
new file mode 100644 (file)
index 0000000..ebeb7ff
--- /dev/null
@@ -0,0 +1,28 @@
+<div>Pass if no crash or assert</div>\r
+<script>\r
+if (window.testRunner)\r
+    testRunner.dumpAsText();\r
+    \r
+function wait() {\r
+if (window.testRunner)\r
+    testRunner.waitUntilDone();\r
+}\r
+\r
+function done() {\r
+if (window.testRunner)\r
+    testRunner.notifyDone();\r
+}\r
+</script>\r
+<iframe srcdoc="<script>\r
+window.matchMedia('print').addListener(function(mql) {\r
+    let parentWindow = window.parent;\r
+    window.frameElement.remove();\r
+    parentWindow.done();\r
+});\r
+\r
+if (window.internals) {\r
+    internals.setPrinting(800, 600);\r
+    window.parent.wait();\r
+} else\r
+    print();\r
+</script>">\r
index 81bf3fc..1e56380 100644 (file)
@@ -1,3 +1,33 @@
+2018-12-21  Zalan Bujtas  <zalan@apple.com>
+
+        Synchronous media query evaluation could destroy current Frame/FrameView.
+        https://bugs.webkit.org/show_bug.cgi?id=192781
+        <rdar://problem/34416793>
+
+        Reviewed by Chris Dumez.
+
+        Protect Frame and FrameView when coming back from printing and check if the current Frame/FrameView/FrameLoader objects are still valid.
+
+        Test: printing/print-with-media-query-destory.html
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::finishedLoading):
+        * page/Frame.cpp:
+        (WebCore::Frame::setPrinting):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::forceLayoutForPagination):
+        * page/PrintContext.cpp:
+        (WebCore::PrintContext::PrintContext):
+        (WebCore::PrintContext::computePageRects):
+        (WebCore::PrintContext::computePageRectsWithPageSizeInternal):
+        (WebCore::PrintContext::begin):
+        (WebCore::PrintContext::computeAutomaticScaleFactor):
+        (WebCore::PrintContext::spoolPage):
+        (WebCore::PrintContext::spoolRect):
+        (WebCore::PrintContext::end):
+        * page/PrintContext.h:
+        (WebCore::PrintContext::frame const): Deleted.
+
 2018-12-21  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Setting the file wrapper and content type of an attachment to a PDF should update its image
index e98b968..d531db4 100644 (file)
@@ -445,6 +445,8 @@ void DocumentLoader::finishedLoading()
     if (!m_mainDocumentError.isNull())
         return;
     clearMainResourceLoader();
+    if (!frameLoader())
+        return;
     if (!frameLoader()->stateMachine().creatingInitialEmptyDocument())
         frameLoader()->checkLoadComplete();
 
index 8d2c3ef..25d3e55 100644 (file)
@@ -661,22 +661,23 @@ bool Frame::selectionChangeCallbacksDisabled() const
 
 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
     ResourceCacheValidationSuppressor validationSuppressor(m_doc->cachedResourceLoader());
 
     m_doc->setPrinting(printing);
-    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();
-        }
+    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();
     }
 
     // Subframes of the one we're printing don't lay out to the page size.
index a7e0a5e..ff60973 100644 (file)
@@ -4586,48 +4586,56 @@ void FrameView::forceLayout(bool allowSubtreeLayout)
 
 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
-    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();
+    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();
         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();
-            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.
-        }
+        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 (shouldAdjustViewSize)
index c6b0351..6d2b7fc 100644 (file)
@@ -34,7 +34,7 @@
 namespace WebCore {
 
 PrintContext::PrintContext(Frame* frame)
-    : m_frame(frame)
+    : FrameDestructionObserver(frame)
 {
 }
 
@@ -46,10 +46,14 @@ PrintContext::~PrintContext()
 
 void PrintContext::computePageRects(const FloatRect& printRect, float headerHeight, float footerHeight, float userScaleFactor, float& outPageHeight, bool allowHorizontalTiling)
 {
+    if (!frame())
+        return;
+
+    auto& frame = *this->frame();
     m_pageRects.clear();
     outPageHeight = 0;
 
-    if (!m_frame->document() || !m_frame->view() || !m_frame->document()->renderView())
+    if (!frame.document() || !frame.view() || !frame.document()->renderView())
         return;
 
     if (userScaleFactor <= 0) {
@@ -57,9 +61,9 @@ void PrintContext::computePageRects(const FloatRect& printRect, float headerHeig
         return;
     }
 
-    RenderView* view = m_frame->document()->renderView();
+    RenderView* view = frame.document()->renderView();
     const IntRect& documentRect = view->documentRect();
-    FloatSize pageSize = m_frame->resizePageRectsKeepingRatio(FloatSize(printRect.width(), printRect.height()), FloatSize(documentRect.width(), documentRect.height()));
+    FloatSize pageSize = frame.resizePageRectsKeepingRatio(FloatSize(printRect.width(), printRect.height()), FloatSize(documentRect.width(), documentRect.height()));
     float pageWidth = pageSize.width();
     float pageHeight = pageSize.height();
 
@@ -82,10 +86,14 @@ void PrintContext::computePageRectsWithPageSize(const FloatSize& pageSizeInPixel
 
 void PrintContext::computePageRectsWithPageSizeInternal(const FloatSize& pageSizeInPixels, bool allowInlineDirectionTiling)
 {
-    if (!m_frame->document() || !m_frame->view() || !m_frame->document()->renderView())
+    if (!frame())
+        return;
+
+    auto& frame = *this->frame();
+    if (!frame.document() || !frame.view() || !frame.document()->renderView())
         return;
 
-    RenderView* view = m_frame->document()->renderView();
+    RenderView* view = frame.document()->renderView();
 
     IntRect docRect = view->documentRect();
 
@@ -151,26 +159,34 @@ void PrintContext::computePageRectsWithPageSizeInternal(const FloatSize& pageSiz
 
 void PrintContext::begin(float width, float height)
 {
+    if (!frame())
+        return;
+
+    auto& frame = *this->frame();
     // This function can be called multiple times to adjust printing parameters without going back to screen mode.
     m_isPrinting = true;
 
     FloatSize originalPageSize = FloatSize(width, height);
-    FloatSize minLayoutSize = m_frame->resizePageRectsKeepingRatio(originalPageSize, FloatSize(width * minimumShrinkFactor(), height * minimumShrinkFactor()));
+    FloatSize minLayoutSize = frame.resizePageRectsKeepingRatio(originalPageSize, FloatSize(width * minimumShrinkFactor(), height * minimumShrinkFactor()));
 
     // This changes layout, so callers need to make sure that they don't paint to screen while in printing mode.
-    m_frame->setPrinting(true, minLayoutSize, originalPageSize, maximumShrinkFactor() / minimumShrinkFactor(), AdjustViewSize);
+    frame.setPrinting(true, minLayoutSize, originalPageSize, maximumShrinkFactor() / minimumShrinkFactor(), AdjustViewSize);
 }
 
 float PrintContext::computeAutomaticScaleFactor(const FloatSize& availablePaperSize)
 {
-    if (!m_frame->view())
+    if (!frame())
+        return 1;
+
+    auto& frame = *this->frame();
+    if (!frame.view())
         return 1;
 
     bool useViewWidth = true;
-    if (m_frame->document() && m_frame->document()->renderView())
-        useViewWidth = m_frame->document()->renderView()->style().isHorizontalWritingMode();
+    if (frame.document() && frame.document()->renderView())
+        useViewWidth = frame.document()->renderView()->style().isHorizontalWritingMode();
 
-    float viewLogicalWidth = useViewWidth ? m_frame->view()->contentsWidth() : m_frame->view()->contentsHeight();
+    float viewLogicalWidth = useViewWidth ? frame.view()->contentsWidth() : frame.view()->contentsHeight();
     if (viewLogicalWidth < 1)
         return 1;
 
@@ -181,6 +197,10 @@ float PrintContext::computeAutomaticScaleFactor(const FloatSize& availablePaperS
 
 void PrintContext::spoolPage(GraphicsContext& ctx, int pageNumber, float width)
 {
+    if (!frame())
+        return;
+
+    auto& frame = *this->frame();
     // FIXME: Not correct for vertical text.
     IntRect pageRect = m_pageRects[pageNumber];
     float scale = width / pageRect.width();
@@ -189,27 +209,35 @@ void PrintContext::spoolPage(GraphicsContext& ctx, int pageNumber, float width)
     ctx.scale(scale);
     ctx.translate(-pageRect.x(), -pageRect.y());
     ctx.clip(pageRect);
-    m_frame->view()->paintContents(ctx, pageRect);
-    outputLinkedDestinations(ctx, *m_frame->document(), pageRect);
+    frame.view()->paintContents(ctx, pageRect);
+    outputLinkedDestinations(ctx, *frame.document(), pageRect);
     ctx.restore();
 }
 
 void PrintContext::spoolRect(GraphicsContext& ctx, const IntRect& rect)
 {
+    if (!frame())
+        return;
+
+    auto& frame = *this->frame();
     // FIXME: Not correct for vertical text.
     ctx.save();
     ctx.translate(-rect.x(), -rect.y());
     ctx.clip(rect);
-    m_frame->view()->paintContents(ctx, rect);
-    outputLinkedDestinations(ctx, *m_frame->document(), rect);
+    frame.view()->paintContents(ctx, rect);
+    outputLinkedDestinations(ctx, *frame.document(), rect);
     ctx.restore();
 }
 
 void PrintContext::end()
 {
+    if (!frame())
+        return;
+
+    auto& frame = *this->frame();
     ASSERT(m_isPrinting);
     m_isPrinting = false;
-    m_frame->setPrinting(false, FloatSize(), FloatSize(), 0, AdjustViewSize);
+    frame.setPrinting(false, FloatSize(), FloatSize(), 0, AdjustViewSize);
     m_linkedDestinations = nullptr;
 }
 
index ceaa23c..9c5b632 100644 (file)
@@ -20,6 +20,7 @@
 
 #pragma once
 
+#include "FrameDestructionObserver.h"
 #include <wtf/Forward.h>
 #include <wtf/HashMap.h>
 #include <wtf/Vector.h>
@@ -36,13 +37,11 @@ class GraphicsContext;
 class IntRect;
 class Node;
 
-class PrintContext {
+class PrintContext : public FrameDestructionObserver {
 public:
     WEBCORE_EXPORT explicit PrintContext(Frame*);
     WEBCORE_EXPORT ~PrintContext();
 
-    Frame* frame() const { return m_frame; }
-
     // Break up a page into rects without relayout.
     // FIXME: This means that CSS page breaks won't be on page boundary if the size is different than what was passed to begin(). That's probably not always desirable.
     // FIXME: Header and footer height should be applied before layout, not after.
@@ -96,7 +95,6 @@ public:
     static constexpr float maximumShrinkFactor() { return 2; }
 
 protected:
-    Frame* m_frame;
     Vector<IntRect> m_pageRects;
 
 private: