<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[182706] releases/WebKitGTK/webkit-2.8/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/182706">182706</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2015-04-13 04:16:27 -0700 (Mon, 13 Apr 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Merge <a href="http://trac.webkit.org/projects/webkit/changeset/182374">r182374</a> - 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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit28SourceWebCoreChangeLog">releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit28SourceWebCorepageFrameViewcpp">releases/WebKitGTK/webkit-2.8/Source/WebCore/page/FrameView.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="releasesWebKitGTKwebkit28SourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog (182705 => 182706)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog 2015-04-13 11:14:13 UTC (rev 182705)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog 2015-04-13 11:16:27 UTC (rev 182706)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2015-04-05 Darin Adler <darin@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.
+
</ins><span class="cx"> 2015-04-01 Zalan Bujtas <zalan@apple.com>
</span><span class="cx">
</span><span class="cx"> Lots of time spent querying table cell borders, when there are none.
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit28SourceWebCorepageFrameViewcpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/page/FrameView.cpp (182705 => 182706)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.8/Source/WebCore/page/FrameView.cpp 2015-04-13 11:14:13 UTC (rev 182705)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/page/FrameView.cpp 2015-04-13 11:16:27 UTC (rev 182706)
</span><span class="lines">@@ -1316,13 +1316,14 @@
</span><span class="cx">
</span><span class="cx"> root->layout();
</span><span class="cx"> #if ENABLE(IOS_TEXT_AUTOSIZING)
</span><del>- 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();
</del><ins>+ 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();
+ }
</ins><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx"> #if ENABLE(TEXT_AUTOSIZING)
</span><span class="lines">@@ -2078,7 +2079,7 @@
</span><span class="cx"> ScrollView::setFixedVisibleContentRect(visibleContentRect);
</span><span class="cx"> if (offset != scrollOffset()) {
</span><span class="cx"> updateLayerPositionsAfterScrolling();
</span><del>- if (frame().page()->settings().acceleratedCompositingForFixedPositionEnabled())
</del><ins>+ if (frame().settings().acceleratedCompositingForFixedPositionEnabled())
</ins><span class="cx"> updateCompositingLayersAfterScrolling();
</span><span class="cx"> IntPoint newPosition = scrollPosition();
</span><span class="cx"> scrollAnimator()->setCurrentPosition(scrollPosition());
</span><span class="lines">@@ -2117,7 +2118,8 @@
</span><span class="cx">
</span><span class="cx"> void FrameView::scrollPositionChanged(const IntPoint& oldPosition, const IntPoint& newPosition)
</span><span class="cx"> {
</span><del>- std::chrono::milliseconds throttlingDelay = frame().page()->chrome().client().eventThrottlingDelay();
</del><ins>+ Page* page = frame().page();
+ auto throttlingDelay = page ? page->chrome().client().eventThrottlingDelay() : std::chrono::milliseconds::zero();
</ins><span class="cx">
</span><span class="cx"> if (throttlingDelay == std::chrono::milliseconds::zero()) {
</span><span class="cx"> m_delayedScrollEventTimer.stop();
</span><span class="lines">@@ -2385,13 +2387,16 @@
</span><span class="cx">
</span><span class="cx"> void FrameView::updateLayerFlushThrottling()
</span><span class="cx"> {
</span><ins>+ Page* page = frame().page();
+ if (!page)
+ return;
+
</ins><span class="cx"> ASSERT(frame().isMainFrame());
</span><del>- auto& page = *frame().page();
</del><span class="cx">
</span><del>- LayerFlushThrottleState::Flags flags = determineLayerFlushThrottleState(page);
</del><ins>+ LayerFlushThrottleState::Flags flags = determineLayerFlushThrottleState(*page);
</ins><span class="cx">
</span><span class="cx"> // See if the client is handling throttling.
</span><del>- if (page.chrome().client().adjustLayerFlushThrottling(flags))
</del><ins>+ if (page->chrome().client().adjustLayerFlushThrottling(flags))
</ins><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> for (Frame* frame = m_frame.get(); frame; frame = frame->tree().traverseNext(m_frame.get())) {
</span><span class="lines">@@ -2416,7 +2421,8 @@
</span><span class="cx">
</span><span class="cx"> static bool shouldEnableSpeculativeTilingDuringLoading(const FrameView& view)
</span><span class="cx"> {
</span><del>- return view.isVisuallyNonEmpty() && !view.frame().page()->progress().isMainLoadProgressing();
</del><ins>+ Page* page = view.frame().page();
+ return page && view.isVisuallyNonEmpty() && !page->progress().isMainLoadProgressing();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void FrameView::enableSpeculativeTilingIfNeeded()
</span><span class="lines">@@ -2950,8 +2956,10 @@
</span><span class="cx"> // Only send layout-related delegate callbacks synchronously for the main frame to
</span><span class="cx"> // avoid re-entering layout for the main frame while delivering a layout-related delegate
</span><span class="cx"> // callback for a subframe.
</span><del>- if (frame().isMainFrame())
- frame().page()->chrome().client().didLayout();
</del><ins>+ if (frame().isMainFrame()) {
+ if (Page* page = frame().page())
+ page->chrome().client().didLayout();
+ }
</ins><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> #if ENABLE(FONT_LOAD_EVENTS)
</span><span class="lines">@@ -3236,8 +3244,10 @@
</span><span class="cx"> if (m_pagination != Pagination())
</span><span class="cx"> return m_pagination;
</span><span class="cx">
</span><del>- if (frame().isMainFrame())
- return frame().page()->pagination();
</del><ins>+ if (frame().isMainFrame()) {
+ if (Page* page = frame().page())
+ return page->pagination();
+ }
</ins><span class="cx">
</span><span class="cx"> return m_pagination;
</span><span class="cx"> }
</span><span class="lines">@@ -3385,7 +3395,11 @@
</span><span class="cx"> if (!frame().isMainFrame() || !frame().settings().delegatesPageScaling())
</span><span class="cx"> return 1;
</span><span class="cx">
</span><del>- return frame().page()->pageScaleFactor();
</del><ins>+ Page* page = frame().page();
+ if (!page)
+ return 1;
+
+ return page->pageScaleFactor();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void FrameView::setVisibleScrollerThumbRect(const IntRect& scrollerThumb)
</span><span class="lines">@@ -3393,7 +3407,11 @@
</span><span class="cx"> if (!frame().isMainFrame())
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- frame().page()->chrome().client().notifyScrollerThumbIsVisibleInRect(scrollerThumb);
</del><ins>+ Page* page = frame().page();
+ if (!page)
+ return;
+
+ page->chrome().client().notifyScrollerThumbIsVisibleInRect(scrollerThumb);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ScrollableArea* FrameView::enclosingScrollableArea() const
</span><span class="lines">@@ -3491,7 +3509,8 @@
</span><span class="cx"> if (!frame().isMainFrame())
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- frame().page()->chrome().client().recommendedScrollbarStyleDidChange(newStyle);
</del><ins>+ if (Page* page = frame().page())
+ page->chrome().client().recommendedScrollbarStyleDidChange(newStyle);
</ins><span class="cx">
</span><span class="cx"> if (forceUpdate)
</span><span class="cx"> ScrollView::scrollbarStyleChanged(newStyle, forceUpdate);
</span><span class="lines">@@ -4642,8 +4661,10 @@
</span><span class="cx"> {
</span><span class="cx"> m_scrollPinningBehavior = pinning;
</span><span class="cx">
</span><del>- if (ScrollingCoordinator* scrollingCoordinator = frame().page()->scrollingCoordinator())
- scrollingCoordinator->setScrollPinningBehavior(pinning);
</del><ins>+ if (Page* page = frame().page()) {
+ if (auto* scrollingCoordinator = page->scrollingCoordinator())
+ scrollingCoordinator->setScrollPinningBehavior(pinning);
+ }
</ins><span class="cx">
</span><span class="cx"> updateScrollbars(scrollOffset());
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>