[webkit-changes] [124615] trunk/Source/WebKit/blackberry

Fri, 03 Aug 2012 08:19:07 -0700

Title: [124615] trunk/Source/WebKit/blackberry
Revision
124615
Author
[email protected]
Date
2012-08-03 08:19:17 -0700 (Fri, 03 Aug 2012)

Log Message

[BlackBerry] Overlays display checkerboard that doesn't resolve
https://bugs.webkit.org/show_bug.cgi?id=93099

Patch by Arvid Nilsson <[email protected]> on 2012-08-03
Reviewed by Antonio Gomes.

The WebKit-thread overlays, like tap highlight, inspector highlight and
selection are all part of a separate graphics layer tree rooted in
WebPagePrivate::m_overlayLayer.

When LayerRenderer needs to schedule a commit to reactively render
tiles and resolve checkerboard, it does so through the root layer.
Since the overlay layer root didn't have a GraphicsLayerClient, there
was no implementation of GraphicsLayerClient::notifySyncRequired() to
call, and a commit was never scheduled, thus checkerboard never
resolved.

Fixed by adding a fallback implementation of GraphicsLayerClient in
WebPagePrivate and hooking up the overlay root to it. Also, this
implementation can be shared by the various overlays to avoide code
duplication, specifically to implement notifySyncRequired(),
showDebugBorders() and showRepaintCounter() only once.

Fixing this revealed a bug where the web page would get stuck in an
endless sequence of commits. It turned out that
WebPagePrivate::updateDelegatedOverlays() was called right in the
middle of the commit operation, after performing the webkit thread part
of the commit operation but before we continued on the compositing
thread. Since updateDelegatedOverlays() typically mutates layers, this
is very bad (layers should not be mutated mid-commit). The mutations
also cause a new commit to scheduled from within the current, which
results in an endless sequence of commits.

Fixed this latter bug by moving the updateDelegatedOverlays() call to
the beginning of the method where it can cause no harm. This is before
we mark the web page as no longer needing commit, so even if the
implementation flips the "needs commit" bit, we will immediately flip
it back and proceed with commit as usual.

PR 187458, 184377

* Api/WebPage.cpp:
(BlackBerry::WebKit::WebPagePrivate::overlayLayer):
(BlackBerry::WebKit::WebPagePrivate::commitRootLayerIfNeeded):
(WebKit):
(BlackBerry::WebKit::WebPagePrivate::notifySyncRequired):
(BlackBerry::WebKit::WebPagePrivate::showDebugBorders):
(BlackBerry::WebKit::WebPagePrivate::showRepaintCounter):
* Api/WebPage_p.h:
(WebPagePrivate):
(BlackBerry::WebKit::WebPagePrivate::notifyAnimationStarted):
(BlackBerry::WebKit::WebPagePrivate::paintContents):
* WebCoreSupport/InspectorOverlay.cpp:
(WebCore::InspectorOverlay::notifySyncRequired):
(WebCore::InspectorOverlay::showDebugBorders):
(WebCore::InspectorOverlay::showRepaintCounter):
* WebKitSupport/DefaultTapHighlight.cpp:
(BlackBerry::WebKit::DefaultTapHighlight::notifySyncRequired):
(BlackBerry::WebKit::DefaultTapHighlight::showDebugBorders):
(WebKit):
(BlackBerry::WebKit::DefaultTapHighlight::showRepaintCounter):
* WebKitSupport/DefaultTapHighlight.h:
(DefaultTapHighlight):
* WebKitSupport/SelectionOverlay.cpp:
(BlackBerry::WebKit::SelectionOverlay::notifySyncRequired):
(BlackBerry::WebKit::SelectionOverlay::showDebugBorders):
(WebKit):
(BlackBerry::WebKit::SelectionOverlay::showRepaintCounter):
* WebKitSupport/SelectionOverlay.h:
(SelectionOverlay):

Modified Paths

Diff

Modified: trunk/Source/WebKit/blackberry/Api/WebPage.cpp (124614 => 124615)


--- trunk/Source/WebKit/blackberry/Api/WebPage.cpp	2012-08-03 14:43:52 UTC (rev 124614)
+++ trunk/Source/WebKit/blackberry/Api/WebPage.cpp	2012-08-03 15:19:17 UTC (rev 124615)
@@ -5623,10 +5623,8 @@
 
 GraphicsLayer* WebPagePrivate::overlayLayer()
 {
-    // The overlay layer has no GraphicsLayerClient, it's just a container
-    // for various overlays.
     if (!m_overlayLayer)
-        m_overlayLayer = GraphicsLayer::create(0);
+        m_overlayLayer = GraphicsLayer::create(this);
 
     return m_overlayLayer.get();
 }
@@ -5745,6 +5743,10 @@
     if (!view)
         return false;
 
+    // This can do pretty much anything depending on the overlay,
+    // so in case it causes relayout or schedule a commit, call it early.
+    updateDelegatedOverlays();
+
     // If we sync compositing layers when a layout is pending, we may cause painting of compositing
     // layer content to occur before layout has happened, which will cause paintContents() to bail.
     if (needsLayoutRecursive(view)) {
@@ -5767,7 +5769,6 @@
     if (m_frameLayers && m_frameLayers->hasLayer())
         m_frameLayers->commitOnWebKitThread(scale);
 
-    updateDelegatedOverlays();
     if (m_overlayLayer)
         m_overlayLayer->platformLayer()->commitOnWebKitThread(scale);
 
@@ -5988,6 +5989,21 @@
     m_needsCommit = true;
     m_needsOneShotDrawingSynchronization = true;
 }
+
+void WebPagePrivate::notifySyncRequired(const GraphicsLayer*)
+{
+    scheduleRootLayerCommit();
+}
+
+bool WebPagePrivate::showDebugBorders(const GraphicsLayer*) const
+{
+    return m_page->settings()->showDebugBorders();
+}
+
+bool WebPagePrivate::showRepaintCounter(const GraphicsLayer*) const
+{
+    return m_page->settings()->showRepaintCounter();
+}
 #endif // USE(ACCELERATED_COMPOSITING)
 
 void WebPagePrivate::enterFullscreenForNode(Node* node)

Modified: trunk/Source/WebKit/blackberry/Api/WebPage_p.h (124614 => 124615)


--- trunk/Source/WebKit/blackberry/Api/WebPage_p.h	2012-08-03 14:43:52 UTC (rev 124614)
+++ trunk/Source/WebKit/blackberry/Api/WebPage_p.h	2012-08-03 15:19:17 UTC (rev 124615)
@@ -25,6 +25,7 @@
 #include "InspectorOverlay.h"
 #if USE(ACCELERATED_COMPOSITING)
 #include "GLES2Context.h"
+#include "GraphicsLayerClient.h"
 #include "LayerRenderer.h"
 #include <EGL/egl.h>
 #endif
@@ -83,7 +84,12 @@
 // In WebPagePrivate, the screen size is called the transformedViewportSize,
 // the viewport position is called the transformedScrollPosition,
 // and the viewport size is called the transformedActualVisibleSize.
-class WebPagePrivate : public PageClientBlackBerry, public WebSettingsDelegate, public Platform::GuardedPointerBase {
+class WebPagePrivate : public PageClientBlackBerry
+                     , public WebSettingsDelegate
+#if USE(ACCELERATED_COMPOSITING)
+                     , public WebCore::GraphicsLayerClient
+#endif
+                     , public Platform::GuardedPointerBase {
 public:
     enum ViewMode { Mobile, Desktop, FixedDesktop };
     enum LoadState { None /* on instantiation of page */, Provisional, Committed, Finished, Failed };
@@ -386,6 +392,13 @@
     WebCore::LayerRenderingResults lastCompositingResults() const;
     WebCore::GraphicsLayer* overlayLayer();
 
+    // Fallback GraphicsLayerClient implementation, used for various overlay layers.
+    virtual void notifyAnimationStarted(const WebCore::GraphicsLayer*, double time) { }
+    virtual void notifySyncRequired(const WebCore::GraphicsLayer*);
+    virtual void paintContents(const WebCore::GraphicsLayer*, WebCore::GraphicsContext&, WebCore::GraphicsLayerPaintingPhase, const WebCore::IntRect& inClip) { }
+    virtual bool showDebugBorders(const WebCore::GraphicsLayer*) const;
+    virtual bool showRepaintCounter(const WebCore::GraphicsLayer*) const;
+
     // WebKit thread, plumbed through from ChromeClientBlackBerry.
     void setRootLayerWebKitThread(WebCore::Frame*, WebCore::LayerWebKitThread*);
     void setNeedsOneShotDrawingSynchronization();

Modified: trunk/Source/WebKit/blackberry/ChangeLog (124614 => 124615)


--- trunk/Source/WebKit/blackberry/ChangeLog	2012-08-03 14:43:52 UTC (rev 124614)
+++ trunk/Source/WebKit/blackberry/ChangeLog	2012-08-03 15:19:17 UTC (rev 124615)
@@ -1,3 +1,75 @@
+2012-08-03  Arvid Nilsson  <[email protected]>
+
+        [BlackBerry] Overlays display checkerboard that doesn't resolve
+        https://bugs.webkit.org/show_bug.cgi?id=93099
+
+        Reviewed by Antonio Gomes.
+
+        The WebKit-thread overlays, like tap highlight, inspector highlight and
+        selection are all part of a separate graphics layer tree rooted in
+        WebPagePrivate::m_overlayLayer.
+
+        When LayerRenderer needs to schedule a commit to reactively render
+        tiles and resolve checkerboard, it does so through the root layer.
+        Since the overlay layer root didn't have a GraphicsLayerClient, there
+        was no implementation of GraphicsLayerClient::notifySyncRequired() to
+        call, and a commit was never scheduled, thus checkerboard never
+        resolved.
+
+        Fixed by adding a fallback implementation of GraphicsLayerClient in
+        WebPagePrivate and hooking up the overlay root to it. Also, this
+        implementation can be shared by the various overlays to avoide code
+        duplication, specifically to implement notifySyncRequired(),
+        showDebugBorders() and showRepaintCounter() only once.
+
+        Fixing this revealed a bug where the web page would get stuck in an
+        endless sequence of commits. It turned out that
+        WebPagePrivate::updateDelegatedOverlays() was called right in the
+        middle of the commit operation, after performing the webkit thread part
+        of the commit operation but before we continued on the compositing
+        thread. Since updateDelegatedOverlays() typically mutates layers, this
+        is very bad (layers should not be mutated mid-commit). The mutations
+        also cause a new commit to scheduled from within the current, which
+        results in an endless sequence of commits.
+
+        Fixed this latter bug by moving the updateDelegatedOverlays() call to
+        the beginning of the method where it can cause no harm. This is before
+        we mark the web page as no longer needing commit, so even if the
+        implementation flips the "needs commit" bit, we will immediately flip
+        it back and proceed with commit as usual.
+
+        PR 187458, 184377
+
+        * Api/WebPage.cpp:
+        (BlackBerry::WebKit::WebPagePrivate::overlayLayer):
+        (BlackBerry::WebKit::WebPagePrivate::commitRootLayerIfNeeded):
+        (WebKit):
+        (BlackBerry::WebKit::WebPagePrivate::notifySyncRequired):
+        (BlackBerry::WebKit::WebPagePrivate::showDebugBorders):
+        (BlackBerry::WebKit::WebPagePrivate::showRepaintCounter):
+        * Api/WebPage_p.h:
+        (WebPagePrivate):
+        (BlackBerry::WebKit::WebPagePrivate::notifyAnimationStarted):
+        (BlackBerry::WebKit::WebPagePrivate::paintContents):
+        * WebCoreSupport/InspectorOverlay.cpp:
+        (WebCore::InspectorOverlay::notifySyncRequired):
+        (WebCore::InspectorOverlay::showDebugBorders):
+        (WebCore::InspectorOverlay::showRepaintCounter):
+        * WebKitSupport/DefaultTapHighlight.cpp:
+        (BlackBerry::WebKit::DefaultTapHighlight::notifySyncRequired):
+        (BlackBerry::WebKit::DefaultTapHighlight::showDebugBorders):
+        (WebKit):
+        (BlackBerry::WebKit::DefaultTapHighlight::showRepaintCounter):
+        * WebKitSupport/DefaultTapHighlight.h:
+        (DefaultTapHighlight):
+        * WebKitSupport/SelectionOverlay.cpp:
+        (BlackBerry::WebKit::SelectionOverlay::notifySyncRequired):
+        (BlackBerry::WebKit::SelectionOverlay::showDebugBorders):
+        (WebKit):
+        (BlackBerry::WebKit::SelectionOverlay::showRepaintCounter):
+        * WebKitSupport/SelectionOverlay.h:
+        (SelectionOverlay):
+
 2012-08-02  Arvid Nilsson  <[email protected]>
 
         [BlackBerry] Add default implementation of GraphicsLayerClient::contentsVisible()

Modified: trunk/Source/WebKit/blackberry/WebCoreSupport/InspectorOverlay.cpp (124614 => 124615)


--- trunk/Source/WebKit/blackberry/WebCoreSupport/InspectorOverlay.cpp	2012-08-03 14:43:52 UTC (rev 124614)
+++ trunk/Source/WebKit/blackberry/WebCoreSupport/InspectorOverlay.cpp	2012-08-03 15:19:17 UTC (rev 124615)
@@ -24,8 +24,6 @@
 #include "FrameView.h"
 #include "GraphicsContext.h"
 #include "GraphicsLayer.h"
-#include "Page.h"
-#include "Settings.h"
 #include "WebPage_p.h"
 
 
@@ -43,9 +41,9 @@
 }
 
 #if USE(ACCELERATED_COMPOSITING)
-void InspectorOverlay::notifySyncRequired(const GraphicsLayer*)
+void InspectorOverlay::notifySyncRequired(const GraphicsLayer* layer)
 {
-    m_webPage->scheduleRootLayerCommit();
+    m_webPage->notifySyncRequired(layer);
 }
 
 void InspectorOverlay::paintContents(const GraphicsLayer*, GraphicsContext& context, GraphicsLayerPaintingPhase, const IntRect& inClip)
@@ -57,14 +55,14 @@
     context.restore();
 }
 
-bool InspectorOverlay::showDebugBorders(const GraphicsLayer*) const
+bool InspectorOverlay::showDebugBorders(const GraphicsLayer* layer) const
 {
-    return m_webPage->m_page->settings()->showDebugBorders();
+    return m_webPage->showDebugBorders(layer);
 }
 
-bool InspectorOverlay::showRepaintCounter(const GraphicsLayer*) const
+bool InspectorOverlay::showRepaintCounter(const GraphicsLayer* layer) const
 {
-    return m_webPage->m_page->settings()->showRepaintCounter();
+    return m_webPage->showRepaintCounter(layer);
 }
 #endif

Modified: trunk/Source/WebKit/blackberry/WebKitSupport/DefaultTapHighlight.cpp (124614 => 124615)


--- trunk/Source/WebKit/blackberry/WebKitSupport/DefaultTapHighlight.cpp	2012-08-03 14:43:52 UTC (rev 124614)
+++ trunk/Source/WebKit/blackberry/WebKitSupport/DefaultTapHighlight.cpp	2012-08-03 15:19:17 UTC (rev 124615)
@@ -110,9 +110,9 @@
         m_overlay->override()->addAnimation(fadeAnimation);
 }
 
-void DefaultTapHighlight::notifySyncRequired(const GraphicsLayer*)
+void DefaultTapHighlight::notifySyncRequired(const GraphicsLayer* layer)
 {
-    m_page->scheduleRootLayerCommit();
+    m_page->notifySyncRequired(layer);
 }
 
 void DefaultTapHighlight::paintContents(const GraphicsLayer*, GraphicsContext& c, GraphicsLayerPaintingPhase, const IntRect& /*inClip*/)
@@ -148,6 +148,16 @@
     c.restore();
 }
 
+bool DefaultTapHighlight::showDebugBorders(const GraphicsLayer* layer) const
+{
+    return m_page->showDebugBorders(layer);
+}
+
+bool DefaultTapHighlight::showRepaintCounter(const GraphicsLayer* layer) const
+{
+    return m_page->showRepaintCounter(layer);
+}
+
 } // namespace WebKit
 } // namespace BlackBerry

Modified: trunk/Source/WebKit/blackberry/WebKitSupport/DefaultTapHighlight.h (124614 => 124615)


--- trunk/Source/WebKit/blackberry/WebKitSupport/DefaultTapHighlight.h	2012-08-03 14:43:52 UTC (rev 124614)
+++ trunk/Source/WebKit/blackberry/WebKitSupport/DefaultTapHighlight.h	2012-08-03 15:19:17 UTC (rev 124615)
@@ -56,8 +56,8 @@
     virtual void notifyAnimationStarted(const WebCore::GraphicsLayer*, double time) { }
     virtual void notifySyncRequired(const WebCore::GraphicsLayer*);
     virtual void paintContents(const WebCore::GraphicsLayer*, WebCore::GraphicsContext&, WebCore::GraphicsLayerPaintingPhase, const WebCore::IntRect& inClip);
-    virtual bool showDebugBorders(const WebCore::GraphicsLayer*) const { return false; }
-    virtual bool showRepaintCounter(const WebCore::GraphicsLayer*) const { return false; }
+    virtual bool showDebugBorders(const WebCore::GraphicsLayer*) const;
+    virtual bool showRepaintCounter(const WebCore::GraphicsLayer*) const;
 
 private:
     DefaultTapHighlight(WebPagePrivate*);

Modified: trunk/Source/WebKit/blackberry/WebKitSupport/SelectionOverlay.cpp (124614 => 124615)


--- trunk/Source/WebKit/blackberry/WebKitSupport/SelectionOverlay.cpp	2012-08-03 14:43:52 UTC (rev 124614)
+++ trunk/Source/WebKit/blackberry/WebKitSupport/SelectionOverlay.cpp	2012-08-03 15:19:17 UTC (rev 124615)
@@ -94,9 +94,9 @@
     m_page->m_webPage->removeOverlay(m_overlay.get());
 }
 
-void SelectionOverlay::notifySyncRequired(const GraphicsLayer*)
+void SelectionOverlay::notifySyncRequired(const GraphicsLayer* layer)
 {
-    m_page->scheduleRootLayerCommit();
+    m_page->notifySyncRequired(layer);
 }
 
 void SelectionOverlay::paintContents(const GraphicsLayer*, GraphicsContext& c, GraphicsLayerPaintingPhase, const IntRect& inClip)
@@ -133,6 +133,16 @@
     c.restore();
 }
 
+bool SelectionOverlay::showDebugBorders(const GraphicsLayer* layer) const
+{
+    return m_page->showDebugBorders(layer);
+}
+
+bool SelectionOverlay::showRepaintCounter(const GraphicsLayer* layer) const
+{
+    return m_page->showRepaintCounter(layer);
+}
+
 } // namespace WebKit
 } // namespace BlackBerry

Modified: trunk/Source/WebKit/blackberry/WebKitSupport/SelectionOverlay.h (124614 => 124615)


--- trunk/Source/WebKit/blackberry/WebKitSupport/SelectionOverlay.h	2012-08-03 14:43:52 UTC (rev 124614)
+++ trunk/Source/WebKit/blackberry/WebKitSupport/SelectionOverlay.h	2012-08-03 15:19:17 UTC (rev 124615)
@@ -52,8 +52,8 @@
     virtual void notifyAnimationStarted(const WebCore::GraphicsLayer*, double time) { }
     virtual void notifySyncRequired(const WebCore::GraphicsLayer*);
     virtual void paintContents(const WebCore::GraphicsLayer*, WebCore::GraphicsContext&, WebCore::GraphicsLayerPaintingPhase, const WebCore::IntRect& inClip);
-    virtual bool showDebugBorders(const WebCore::GraphicsLayer*) const { return false; }
-    virtual bool showRepaintCounter(const WebCore::GraphicsLayer*) const { return false; }
+    virtual bool showDebugBorders(const WebCore::GraphicsLayer*) const;
+    virtual bool showRepaintCounter(const WebCore::GraphicsLayer*) const;
 
 private:
     SelectionOverlay(WebPagePrivate*);

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to