[BlackBerry] Crash due to an assert in FrameView::doDeferredRepaints
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 May 2013 08:52:57 +0000 (08:52 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 May 2013 08:52:57 +0000 (08:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=115412

Patch by Carlos Garcia Campos <cgarcia@igalia.com> on 2013-05-14
Reviewed by Rob Buis.

PR 115412

The problem is that we are calling
updateLayoutAndStyleIfNeededRecursive() (because of
zoomToInitialScaleOnLoad) from ChomeClient::layoutUpdated()
callback which is not expected. It's expected to be called right
before painting, and not right after painting. Even if a new
layout is not done, updateLayoutAndStyleIfNeededRecursive() calls
flushDeferredRepaints() and it's possible that this is called in
the middle of a beginDeferredRepaints() and endDeferredRepaints()
apparently.
In general only BackingStore should call
updateLayoutAndStyleIfNeededRecursive before painting, and a simple
layout is enough in all other cases like resizing. This patch renames
requestLayoutIfNeeded as updateLayoutAndStyleIfNeededRecursive to
make more obvious what it does, and adds layoutIfNeeded that calls
layout. The former is used by the BackingStore and the latter by
WebPage.

* Api/BackingStore.cpp:
(BlackBerry::WebKit::BackingStorePrivate::resumeScreenUpdates):
(BlackBerry::WebKit::BackingStorePrivate::requestLayoutIfNeeded):
* Api/WebPage.cpp:
(BlackBerry::WebKit::WebPagePrivate::zoomAboutPoint):
(BlackBerry::WebKit::WebPagePrivate::updateLayoutAndStyleIfNeededRecursive):
(BlackBerry::WebKit::WebPagePrivate::layoutIfNeeded):
(WebKit):
(BlackBerry::WebKit::WebPagePrivate::overflowExceedsContentsSize):
(BlackBerry::WebKit::WebPagePrivate::zoomToInitialScaleOnLoad):
(BlackBerry::WebKit::WebPagePrivate::webContext):
(BlackBerry::WebKit::WebPagePrivate::zoomAnimationFinished):
(BlackBerry::WebKit::WebPagePrivate::setViewportSize):
(BlackBerry::WebKit::WebPage::setDefaultLayoutSize):
(BlackBerry::WebKit::WebPagePrivate::rootLayerCommitTimerFired):
* Api/WebPage_p.h:
(WebPagePrivate):

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

Source/WebKit/blackberry/Api/BackingStore.cpp
Source/WebKit/blackberry/Api/WebPage.cpp
Source/WebKit/blackberry/Api/WebPage_p.h
Source/WebKit/blackberry/ChangeLog

index 6f616cc..fb68243 100644 (file)
@@ -390,7 +390,7 @@ void BackingStorePrivate::resumeScreenUpdates(BackingStore::ResumeUpdateOperatio
 #if USE(ACCELERATED_COMPOSITING)
     // It needs layout and render before committing root layer if we set OSDS
     if (m_webPage->d->needsOneShotDrawingSynchronization())
-        m_webPage->d->requestLayoutIfNeeded();
+        m_webPage->d->updateLayoutAndStyleIfNeededRecursive();
 
     // This will also blit since we set the OSDS flag above.
     m_webPage->d->commitRootLayerIfNeeded();
@@ -1105,7 +1105,7 @@ TileIndexList BackingStorePrivate::render(const TileIndexList& tileIndexList)
 
 void BackingStorePrivate::requestLayoutIfNeeded() const
 {
-    m_webPage->d->requestLayoutIfNeeded();
+    m_webPage->d->updateLayoutAndStyleIfNeededRecursive();
 }
 
 void BackingStorePrivate::renderAndBlitVisibleContentsImmediately()
index a400f15..6127f96 100644 (file)
@@ -1246,8 +1246,8 @@ bool WebPagePrivate::zoomAboutPoint(double unclampedScale, const FloatPoint& anc
 
     if (m_webPage->settings()->textReflowMode() == WebSettings::TextReflowEnabled) {
         // This is a hack for email which has reflow always turned on.
-        m_mainFrame->view()->setNeedsLayout();
-        requestLayoutIfNeeded();
+        setNeedsLayout();
+        layoutIfNeeded();
         if (m_currentPinchZoomNode)
             newScrollPosition = calculateReflowedScrollPosition(anchorOffset, scale == minimumScale() ? 1 : inverseScale);
         m_currentPinchZoomNode = 0;
@@ -1314,7 +1314,7 @@ void WebPagePrivate::setNeedsLayout()
     view->setNeedsLayout();
 }
 
-void WebPagePrivate::requestLayoutIfNeeded() const
+void WebPagePrivate::updateLayoutAndStyleIfNeededRecursive() const
 {
     FrameView* view = m_mainFrame->view();
     ASSERT(view);
@@ -1322,6 +1322,14 @@ void WebPagePrivate::requestLayoutIfNeeded() const
     ASSERT(!view->needsLayout());
 }
 
+void WebPagePrivate::layoutIfNeeded() const
+{
+    FrameView* view = m_mainFrame->view();
+    ASSERT(view);
+    if (view->needsLayout())
+        view->layout();
+}
+
 IntPoint WebPagePrivate::scrollPosition() const
 {
     if (!m_backingStoreClient)
@@ -1543,7 +1551,7 @@ void WebPagePrivate::overflowExceedsContentsSize()
     if (absoluteVisibleOverflowSize().width() < DEFAULT_MAX_LAYOUT_WIDTH && !hasVirtualViewport()) {
         if (setViewMode(viewMode())) {
             setNeedsLayout();
-            requestLayoutIfNeeded();
+            layoutIfNeeded();
         }
     }
 }
@@ -1644,7 +1652,7 @@ void WebPagePrivate::zoomToInitialScaleOnLoad()
 #if DEBUG_WEBPAGE_LOAD
         Platform::logAlways(Platform::LogLevelInfo, "WebPagePrivate::zoomToInitialScaleOnLoad content is empty!");
 #endif
-        requestLayoutIfNeeded();
+        layoutIfNeeded();
         notifyTransformedContentsSizeChanged();
         return;
     }
@@ -1670,7 +1678,7 @@ void WebPagePrivate::zoomToInitialScaleOnLoad()
     }
 
     // zoomAboutPoint above can also toggle setNeedsLayout and cause recursive layout...
-    requestLayoutIfNeeded();
+    layoutIfNeeded();
 
     if (!performedZoom) {
         // We only notify if we didn't perform zoom, because zoom will notify on
@@ -2138,7 +2146,7 @@ Platform::WebContext WebPagePrivate::webContext(TargetDetectionStrategy strategy
         return context;
     }
 
-    requestLayoutIfNeeded();
+    layoutIfNeeded();
 
     bool nodeAllowSelectionOverride = false;
     Node* linkNode = node->enclosingLinkEventParentOrSelf();
@@ -2903,7 +2911,7 @@ void WebPagePrivate::zoomAnimationFinished(double finalAnimationScale, const Web
     }
 
 #if ENABLE(VIEWPORT_REFLOW)
-    requestLayoutIfNeeded();
+    layoutIfNeeded();
     if (willUseTextReflow && m_shouldReflowBlock) {
         Platform::IntPoint roundedReflowOffset(
             std::floorf(m_finalAnimationDocumentScrollPositionReflowOffset.x()),
@@ -3648,7 +3656,7 @@ bool WebPagePrivate::setViewportSize(const IntSize& transformedActualVisibleSize
     bool stillNeedsLayout = needsLayout;
     while (stillNeedsLayout) {
         setNeedsLayout();
-        requestLayoutIfNeeded();
+        layoutIfNeeded();
         stillNeedsLayout = false;
 
         // Emulate the zoomToFitWidthOnLoad algorithm if we're rotating.
@@ -3676,7 +3684,7 @@ bool WebPagePrivate::setViewportSize(const IntSize& transformedActualVisibleSize
         IntRect actualVisibleRect = enclosingIntRect(rotationMatrix.inverse().mapRect(FloatRect(viewportRect)));
         m_mainFrame->view()->setFixedReportedSize(actualVisibleRect.size());
         m_mainFrame->view()->repaintFixedElementsAfterScrolling();
-        requestLayoutIfNeeded();
+        layoutIfNeeded();
         m_mainFrame->view()->updateFixedElementsAfterScrolling();
     }
 
@@ -3821,7 +3829,7 @@ void WebPage::setDefaultLayoutSize(const Platform::IntSize& platformSize)
     if (needsLayout) {
         d->setNeedsLayout();
         if (!d->isLoading())
-            d->requestLayoutIfNeeded();
+            d->layoutIfNeeded();
     }
 }
 
@@ -5518,7 +5526,7 @@ void WebPagePrivate::rootLayerCommitTimerFired(Timer<WebPagePrivate>*)
     // to texture.
     // The layout can also turn of compositing altogether, so we need to be prepared
     // to handle a one shot drawing synchronization after the layout.
-    requestLayoutIfNeeded();
+    layoutIfNeeded();
 
     // If we transitioned to drawing the root layer using compositor instead of
     // backing store, doing a one shot drawing synchronization with the
index b7cbc53..bd56e5e 100644 (file)
@@ -150,7 +150,8 @@ public:
     void zoomAnimationFinished(double finalScale, const WebCore::FloatPoint& finalDocumentScrollPosition, bool shouldConstrainScrollingToContentEdge);
 
     // Called by the backing store as well as the method below.
-    void requestLayoutIfNeeded() const;
+    void updateLayoutAndStyleIfNeededRecursive() const;
+    void layoutIfNeeded() const;
     void setNeedsLayout();
 
     WebCore::IntPoint scrollPosition() const;
index e655c3c..98c45d0 100644 (file)
@@ -1,3 +1,47 @@
+2013-05-14  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [BlackBerry] Crash due to an assert in FrameView::doDeferredRepaints
+        https://bugs.webkit.org/show_bug.cgi?id=115412
+
+        Reviewed by Rob Buis.
+
+        PR 115412
+
+        The problem is that we are calling
+        updateLayoutAndStyleIfNeededRecursive() (because of
+        zoomToInitialScaleOnLoad) from ChomeClient::layoutUpdated()
+        callback which is not expected. It's expected to be called right
+        before painting, and not right after painting. Even if a new
+        layout is not done, updateLayoutAndStyleIfNeededRecursive() calls
+        flushDeferredRepaints() and it's possible that this is called in
+        the middle of a beginDeferredRepaints() and endDeferredRepaints()
+        apparently.
+        In general only BackingStore should call
+        updateLayoutAndStyleIfNeededRecursive before painting, and a simple
+        layout is enough in all other cases like resizing. This patch renames
+        requestLayoutIfNeeded as updateLayoutAndStyleIfNeededRecursive to
+        make more obvious what it does, and adds layoutIfNeeded that calls
+        layout. The former is used by the BackingStore and the latter by
+        WebPage.
+
+        * Api/BackingStore.cpp:
+        (BlackBerry::WebKit::BackingStorePrivate::resumeScreenUpdates):
+        (BlackBerry::WebKit::BackingStorePrivate::requestLayoutIfNeeded):
+        * Api/WebPage.cpp:
+        (BlackBerry::WebKit::WebPagePrivate::zoomAboutPoint):
+        (BlackBerry::WebKit::WebPagePrivate::updateLayoutAndStyleIfNeededRecursive):
+        (BlackBerry::WebKit::WebPagePrivate::layoutIfNeeded):
+        (WebKit):
+        (BlackBerry::WebKit::WebPagePrivate::overflowExceedsContentsSize):
+        (BlackBerry::WebKit::WebPagePrivate::zoomToInitialScaleOnLoad):
+        (BlackBerry::WebKit::WebPagePrivate::webContext):
+        (BlackBerry::WebKit::WebPagePrivate::zoomAnimationFinished):
+        (BlackBerry::WebKit::WebPagePrivate::setViewportSize):
+        (BlackBerry::WebKit::WebPage::setDefaultLayoutSize):
+        (BlackBerry::WebKit::WebPagePrivate::rootLayerCommitTimerFired):
+        * Api/WebPage_p.h:
+        (WebPagePrivate):
+
 2013-05-10  Laszlo Gombos  <l.gombos@samsung.com>
 
         Remove USE(OS_RANDOMNESS)