Title: [182374] trunk/Source/WebCore
Revision
182374
Author
da...@apple.com
Date
2015-04-05 20:29:06 -0700 (Sun, 05 Apr 2015)

Log Message

FrameView code uses page() without null checking
https://bugs.webkit.org/show_bug.cgi?id=143425
rdar://problem/18920601

Reviewed by Anders Carlsson.

While we don't have tests that cover this, we are seeing crashes coming in
that indicate the shouldEnableSpeculativeTilingDuringLoading function is
being called when the page is null. This patch adds null checks to all the
places in FrameView that use page() without doing null checking.

* page/FrameView.cpp:
(WebCore::FrameView::layout): If page is null, don't try to do the
auto-sizing logic that involves the textAutosizingWidth value from the page.
(WebCore::FrameView::setFixedVisibleContentRect): Get settings from the
frame rather than the page to avoid possible null-dereference.
(WebCore::FrameView::scrollPositionChanged): Check the page for null when
getting the event throttling delay.
(WebCore::FrameView::updateLayerFlushThrottling): Check the page for null,
and return early if it is null.
(WebCore::shouldEnableSpeculativeTilingDuringLoading): Check the page for
null, and return false if it is null.
(WebCore::FrameView::performPostLayoutTasks): Guard the code that calls
didLayout on the page client by a check if the page is null.
(WebCore::FrameView::pagination): Don't call Page::pagination on a null
page here.
(WebCore::FrameView::visibleContentScaleFactor): Use a scale factor of 1
if the page is null.
(WebCore::FrameView::setVisibleScrollerThumbRect): Don't call through to
the page client if the page is null.
(WebCore::FrameView::scrollbarStyleChanged): Ditto.
(WebCore::FrameView::setScrollPinningBehavior): Check the page for null
before asking it for the scrolling coordinator.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (182373 => 182374)


--- trunk/Source/WebCore/ChangeLog	2015-04-06 02:59:57 UTC (rev 182373)
+++ trunk/Source/WebCore/ChangeLog	2015-04-06 03:29:06 UTC (rev 182374)
@@ -1,3 +1,39 @@
+2015-04-05  Darin Adler  <da...@apple.com>
+
+        FrameView code uses page() without null checking
+        https://bugs.webkit.org/show_bug.cgi?id=143425
+        rdar://problem/18920601
+
+        Reviewed by Anders Carlsson.
+
+        While we don't have tests that cover this, we are seeing crashes coming in
+        that indicate the shouldEnableSpeculativeTilingDuringLoading function is
+        being called when the page is null. This patch adds null checks to all the
+        places in FrameView that use page() without doing null checking.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::layout): If page is null, don't try to do the
+        auto-sizing logic that involves the textAutosizingWidth value from the page.
+        (WebCore::FrameView::setFixedVisibleContentRect): Get settings from the
+        frame rather than the page to avoid possible null-dereference.
+        (WebCore::FrameView::scrollPositionChanged): Check the page for null when
+        getting the event throttling delay.
+        (WebCore::FrameView::updateLayerFlushThrottling): Check the page for null,
+        and return early if it is null.
+        (WebCore::shouldEnableSpeculativeTilingDuringLoading): Check the page for
+        null, and return false if it is null.
+        (WebCore::FrameView::performPostLayoutTasks): Guard the code that calls
+        didLayout on the page client by a check if the page is null.
+        (WebCore::FrameView::pagination): Don't call Page::pagination on a null
+        page here.
+        (WebCore::FrameView::visibleContentScaleFactor): Use a scale factor of 1
+        if the page is null.
+        (WebCore::FrameView::setVisibleScrollerThumbRect): Don't call through to
+        the page client if the page is null.
+        (WebCore::FrameView::scrollbarStyleChanged): Ditto.
+        (WebCore::FrameView::setScrollPinningBehavior): Check the page for null
+        before asking it for the scrolling coordinator.
+
 2015-04-05  Simon Fraser  <simon.fra...@apple.com>
 
         Free up some bits in RenderObject by moving rarely used bits into a side table

Modified: trunk/Source/WebCore/page/FrameView.cpp (182373 => 182374)


--- trunk/Source/WebCore/page/FrameView.cpp	2015-04-06 02:59:57 UTC (rev 182373)
+++ trunk/Source/WebCore/page/FrameView.cpp	2015-04-06 03:29:06 UTC (rev 182374)
@@ -1313,13 +1313,14 @@
 
         root->layout();
 #if ENABLE(IOS_TEXT_AUTOSIZING)
-        float minZoomFontSize = frame().settings().minimumZoomFontSize();
-        float visWidth = frame().page()->textAutosizingWidth();
-        if (minZoomFontSize && visWidth && !root->view().printing()) {
-            root->adjustComputedFontSizesOnBlocks(minZoomFontSize, visWidth);    
-            bool needsLayout = root->needsLayout();
-            if (needsLayout)
-                root->layout();
+        if (Page* page = frame().page()) {
+            float minimumZoomFontSize = frame().settings().minimumZoomFontSize();
+            float textAutosizingWidth = page->textAutosizingWidth();
+            if (minimumZoomFontSize && textAutosizingWidth && !root->view().printing()) {
+                root->adjustComputedFontSizesOnBlocks(minimumZoomFontSize, textAutosizingWidth);
+                if (root->needsLayout())
+                    root->layout();
+            }
         }
 #endif
 #if ENABLE(TEXT_AUTOSIZING)
@@ -2085,7 +2086,7 @@
     ScrollView::setFixedVisibleContentRect(visibleContentRect);
     if (offset != scrollOffset()) {
         updateLayerPositionsAfterScrolling();
-        if (frame().page()->settings().acceleratedCompositingForFixedPositionEnabled())
+        if (frame().settings().acceleratedCompositingForFixedPositionEnabled())
             updateCompositingLayersAfterScrolling();
         IntPoint newPosition = scrollPosition();
         scrollAnimator().setCurrentPosition(scrollPosition());
@@ -2124,7 +2125,8 @@
 
 void FrameView::scrollPositionChanged(const IntPoint& oldPosition, const IntPoint& newPosition)
 {
-    std::chrono::milliseconds throttlingDelay = frame().page()->chrome().client().eventThrottlingDelay();
+    Page* page = frame().page();
+    auto throttlingDelay = page ? page->chrome().client().eventThrottlingDelay() : std::chrono::milliseconds::zero();
 
     if (throttlingDelay == std::chrono::milliseconds::zero()) {
         m_delayedScrollEventTimer.stop();
@@ -2386,13 +2388,16 @@
 
 void FrameView::updateLayerFlushThrottling()
 {
+    Page* page = frame().page();
+    if (!page)
+        return;
+
     ASSERT(frame().isMainFrame());
-    auto& page = *frame().page();
 
-    LayerFlushThrottleState::Flags flags = determineLayerFlushThrottleState(page);
+    LayerFlushThrottleState::Flags flags = determineLayerFlushThrottleState(*page);
 
     // See if the client is handling throttling.
-    if (page.chrome().client().adjustLayerFlushThrottling(flags))
+    if (page->chrome().client().adjustLayerFlushThrottling(flags))
         return;
 
     for (Frame* frame = m_frame.get(); frame; frame = frame->tree().traverseNext(m_frame.get())) {
@@ -2417,7 +2422,8 @@
 
 static bool shouldEnableSpeculativeTilingDuringLoading(const FrameView& view)
 {
-    return view.isVisuallyNonEmpty() && !view.frame().page()->progress().isMainLoadProgressing();
+    Page* page = view.frame().page();
+    return page && view.isVisuallyNonEmpty() && !page->progress().isMainLoadProgressing();
 }
 
 void FrameView::enableSpeculativeTilingIfNeeded()
@@ -2951,8 +2957,10 @@
     // Only send layout-related delegate callbacks synchronously for the main frame to
     // avoid re-entering layout for the main frame while delivering a layout-related delegate
     // callback for a subframe.
-    if (frame().isMainFrame())
-        frame().page()->chrome().client().didLayout();
+    if (frame().isMainFrame()) {
+        if (Page* page = frame().page())
+            page->chrome().client().didLayout();
+    }
 #endif
 
 #if ENABLE(FONT_LOAD_EVENTS)
@@ -3247,8 +3255,10 @@
     if (m_pagination != Pagination())
         return m_pagination;
 
-    if (frame().isMainFrame())
-        return frame().page()->pagination();
+    if (frame().isMainFrame()) {
+        if (Page* page = frame().page())
+            return page->pagination();
+    }
 
     return m_pagination;
 }
@@ -3396,7 +3406,11 @@
     if (!frame().isMainFrame() || !frame().settings().delegatesPageScaling())
         return 1;
 
-    return frame().page()->pageScaleFactor();
+    Page* page = frame().page();
+    if (!page)
+        return 1;
+
+    return page->pageScaleFactor();
 }
 
 void FrameView::setVisibleScrollerThumbRect(const IntRect& scrollerThumb)
@@ -3404,7 +3418,11 @@
     if (!frame().isMainFrame())
         return;
 
-    frame().page()->chrome().client().notifyScrollerThumbIsVisibleInRect(scrollerThumb);
+    Page* page = frame().page();
+    if (!page)
+        return;
+
+    page->chrome().client().notifyScrollerThumbIsVisibleInRect(scrollerThumb);
 }
 
 ScrollableArea* FrameView::enclosingScrollableArea() const
@@ -3502,7 +3520,8 @@
     if (!frame().isMainFrame())
         return;
 
-    frame().page()->chrome().client().recommendedScrollbarStyleDidChange(newStyle);
+    if (Page* page = frame().page())
+        page->chrome().client().recommendedScrollbarStyleDidChange(newStyle);
 
     ScrollView::scrollbarStyleChanged(newStyle, forceUpdate);
 }
@@ -4666,8 +4685,10 @@
 {
     m_scrollPinningBehavior = pinning;
     
-    if (ScrollingCoordinator* scrollingCoordinator = frame().page()->scrollingCoordinator())
-        scrollingCoordinator->setScrollPinningBehavior(pinning);
+    if (Page* page = frame().page()) {
+        if (auto* scrollingCoordinator = page->scrollingCoordinator())
+            scrollingCoordinator->setScrollPinningBehavior(pinning);
+    }
     
     updateScrollbars(scrollOffset());
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to