[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
cmarrin at apple.com
cmarrin at apple.com
Wed Dec 22 12:36:23 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit ff33a3c76f9f5e0cd917074df1fb00ba7b18d4e3
Author: cmarrin at apple.com <cmarrin at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Aug 25 22:49:38 2010 +0000
2010-08-25 Chris Marrin <cmarrin at apple.com>
Reviewed by Simon Fraser.
https://bugs.webkit.org/show_bug.cgi?id=44629
Add layer consistency checking and fix found crashing bug
The crash was being caused by some of the calls which mutated
the sublayer list leaving the list in an inconsistent state.
This eventually lead to a crash. It would also lead to visual
artifacts if the crash didn't occur. Added consistency checking
to catch this and any other inconsistencies in the sublayer list.
The particular bug in this case was caused by clamping an index
for insertion to the current size of the sublayer list. CACF uses
an index equal to the current length to indicate an append operation.
With tiled layers the apparent size of the list is one less than its
actual size (to accomodate the layer which holds the list of tiles)
so this clamping was causing the new layer to get inserted before the
tile parent. The tile parent was then mistaken for a WKCACFLayer and
it eventually tried to deref that layer, causing the crash.
I also added some protection when destroying a WKCACFLayer. The user data
for the corresponding CACFLayer is now changed to 0xDeadBeef rather than
null. This allows dangling layers to be more easily identified. This
value is checked and ASSERTed if seen. I also remove the sublayers
on destruction to make the consistency checks work properly while
a layer is being destroyed.
Test: compositing/tiling/crash-reparent-tiled-layer.html
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@66050 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 81c7daf..8383442 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2010-08-25 Chris Marrin <cmarrin at apple.com>
+
+ Reviewed by Simon Fraser.
+
+ https://bugs.webkit.org/show_bug.cgi?id=44629
+ Add layer consistency checking and fix found crashing bug
+
+ * compositing/tiling/crash-reparent-tiled-layer-expected.txt: Added.
+ * compositing/tiling/crash-reparent-tiled-layer.html: Added.
+
2010-08-25 Ryosuke Niwa <rniwa at webkit.org>
Reviewed by Darin Adler.
diff --git a/LayoutTests/compositing/tiling/crash-reparent-tiled-layer-expected.txt b/LayoutTests/compositing/tiling/crash-reparent-tiled-layer-expected.txt
new file mode 100644
index 0000000..6f21447
--- /dev/null
+++ b/LayoutTests/compositing/tiling/crash-reparent-tiled-layer-expected.txt
@@ -0,0 +1,60 @@
+From https://bugs.webkit.org/show_bug.cgi?id=44629. The parent is a tiled layer. When the child is a non-tiled layer and it is switched to a tiled layer, a crash occurs. This test should not crash.
+
+(GraphicsLayer
+ (position 0.00 0.00)
+ (anchor 0.50 0.50)
+ (bounds 785.00 5111.00)
+ (opacity 1.00)
+ (usingTiledLayer 0)
+ (preserves3D 0)
+ (drawsContent 0)
+ (backfaceVisibility visible)
+ (backgroundColor none)
+ (transform identity)
+ (children 1
+ (GraphicsLayer
+ (position 0.00 0.00)
+ (anchor 0.50 0.50)
+ (bounds 785.00 5111.00)
+ (opacity 1.00)
+ (usingTiledLayer 0)
+ (preserves3D 0)
+ (drawsContent 0)
+ (backfaceVisibility visible)
+ (backgroundColor none)
+ (transform identity)
+ (childrenTransform identity)
+ (children 1
+ (GraphicsLayer
+ (position 8.00 68.00)
+ (anchor 0.50 0.50)
+ (bounds 502.00 5002.00)
+ (opacity 1.00)
+ (usingTiledLayer 1)
+ (preserves3D 0)
+ (drawsContent 1)
+ (backfaceVisibility visible)
+ (backgroundColor none)
+ (transform identity)
+ (childrenTransform identity)
+ (children 1
+ (GraphicsLayer
+ (position 51.00 101.00)
+ (anchor 0.50 0.50)
+ (bounds 200.00 4800.00)
+ (opacity 1.00)
+ (usingTiledLayer 1)
+ (preserves3D 0)
+ (drawsContent 1)
+ (backfaceVisibility visible)
+ (backgroundColor none)
+ (transform identity)
+ (childrenTransform identity)
+ )
+ )
+ )
+ )
+ )
+ )
+)
+
diff --git a/LayoutTests/compositing/tiling/crash-reparent-tiled-layer.html b/LayoutTests/compositing/tiling/crash-reparent-tiled-layer.html
new file mode 100644
index 0000000..d3d793e
--- /dev/null
+++ b/LayoutTests/compositing/tiling/crash-reparent-tiled-layer.html
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>Test that switching to tiled layers when the parent is a tiled layer does not crash</title>
+
+ <style type="text/css" media="screen">
+
+ #container {
+ width: 500px;
+ height: 5000px;
+ border: 1px solid black;
+ background-color: yellow;
+ -webkit-transform:translateZ(0);
+ }
+
+ #box {
+ position: absolute;
+ left:50px;
+ width: 200px;
+ height: 200px;
+ -webkit-transform:translateZ(0);
+ top: 100px;
+ background-color: red;
+ }
+ </style>
+ <script type="text/javascript" charset="utf-8">
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+
+ result = "";
+
+ function testOnLoad()
+ {
+ window.setTimeout(function() {
+ document.getElementById('box').style.height = "4800px";
+
+ // Let it render
+ window.setTimeout(function() {
+ if (window.layoutTestController) {
+ document.getElementById('layers').innerHTML = layoutTestController.layerTreeAsText();
+ layoutTestController.notifyDone();
+ }
+ }, 0);
+ }, 0);
+ }
+
+ window.addEventListener('load', testOnLoad, false);
+ </script>
+ </head>
+ <body>
+ <p>
+ From https://bugs.webkit.org/show_bug.cgi?id=44629. The parent is a tiled layer.
+ When the child is a non-tiled layer and it is switched to a tiled layer, a crash
+ occurs. This test should not crash.
+ </p>
+ <div id="container">
+ <div id="box"></div>
+ </div>
+ <pre id="layers">Layer tree appears here in DRT.</pre>
+ </body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 252fe4c..22cf4e9 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2010-08-25 Chris Marrin <cmarrin at apple.com>
+
+ Reviewed by Simon Fraser.
+
+ https://bugs.webkit.org/show_bug.cgi?id=44629
+ Add layer consistency checking and fix found crashing bug
+
+ The crash was being caused by some of the calls which mutated
+ the sublayer list leaving the list in an inconsistent state.
+ This eventually lead to a crash. It would also lead to visual
+ artifacts if the crash didn't occur. Added consistency checking
+ to catch this and any other inconsistencies in the sublayer list.
+
+ The particular bug in this case was caused by clamping an index
+ for insertion to the current size of the sublayer list. CACF uses
+ an index equal to the current length to indicate an append operation.
+ With tiled layers the apparent size of the list is one less than its
+ actual size (to accomodate the layer which holds the list of tiles)
+ so this clamping was causing the new layer to get inserted before the
+ tile parent. The tile parent was then mistaken for a WKCACFLayer and
+ it eventually tried to deref that layer, causing the crash.
+
+ I also added some protection when destroying a WKCACFLayer. The user data
+ for the corresponding CACFLayer is now changed to 0xDeadBeef rather than
+ null. This allows dangling layers to be more easily identified. This
+ value is checked and ASSERTed if seen. I also remove the sublayers
+ on destruction to make the consistency checks work properly while
+ a layer is being destroyed.
+
+ Test: compositing/tiling/crash-reparent-tiled-layer.html
+
+ * platform/graphics/win/WKCACFLayer.cpp:
+ * platform/graphics/win/WKCACFLayer.h:
+ * platform/graphics/win/WebTiledLayer.cpp:
+ * platform/graphics/win/WebTiledLayer.h:
+
2010-08-25 Ryosuke Niwa <rniwa at webkit.org>
Reviewed by Darin Adler.
diff --git a/WebCore/platform/graphics/win/WKCACFLayer.cpp b/WebCore/platform/graphics/win/WKCACFLayer.cpp
index b5f3427..bbe5883 100644
--- a/WebCore/platform/graphics/win/WKCACFLayer.cpp
+++ b/WebCore/platform/graphics/win/WKCACFLayer.cpp
@@ -44,6 +44,24 @@ namespace WebCore {
using namespace std;
+#ifndef NDEBUG
+void WKCACFLayer::internalCheckLayerConsistency()
+{
+ ASSERT(layer());
+ size_t n = sublayerCount();
+ for (size_t i = 0; i < n; ++i) {
+ // This will ASSERT in internalSublayerAtIndex if this entry doesn't have proper user data
+ WKCACFLayer* sublayer = internalSublayerAtIndex(i);
+
+ // Make sure we don't have any null entries in the list
+ ASSERT(sublayer);
+
+ // Make sure the each layer has a corresponding CACFLayer
+ ASSERT(sublayer->layer());
+ }
+}
+#endif
+
static void displayCallback(CACFLayerRef layer, CGContextRef context)
{
ASSERT_ARG(layer, WKCACFLayer::layer(layer));
@@ -161,7 +179,14 @@ WKCACFLayer::~WKCACFLayer()
// Our superlayer should be holding a reference to us, so there should be no way for us to be destroyed while we still have a superlayer.
ASSERT(!superlayer());
+ // Get rid of the children so we don't have any dangling references around
+ removeAllSublayers();
+
+#ifndef NDEBUG
+ CACFLayerSetUserData(layer(), reinterpret_cast<void*>(0xDEADBEEF));
+#else
CACFLayerSetUserData(layer(), 0);
+#endif
CACFLayerSetDisplayCallback(layer(), 0);
}
@@ -192,10 +217,11 @@ void WKCACFLayer::addSublayer(PassRefPtr<WKCACFLayer> sublayer)
void WKCACFLayer::internalInsertSublayer(PassRefPtr<WKCACFLayer> sublayer, size_t index)
{
- index = min(index, sublayerCount());
+ index = min(index, sublayerCount() + 1);
sublayer->removeFromSuperlayer();
CACFLayerInsertSublayer(layer(), sublayer->layer(), index);
setNeedsCommit();
+ checkLayerConsistency();
}
void WKCACFLayer::insertSublayerAboveLayer(PassRefPtr<WKCACFLayer> sublayer, const WKCACFLayer* reference)
@@ -268,12 +294,14 @@ void WKCACFLayer::adoptSublayers(WKCACFLayer* source)
sublayers.append(source->internalSublayerAtIndex(i));
setSublayers(sublayers);
+ source->checkLayerConsistency();
}
void WKCACFLayer::removeFromSuperlayer()
{
WKCACFLayer* superlayer = this->superlayer();
CACFLayerRemoveFromSuperlayer(layer());
+ checkLayerConsistency();
if (superlayer)
superlayer->setNeedsCommit();
diff --git a/WebCore/platform/graphics/win/WKCACFLayer.h b/WebCore/platform/graphics/win/WKCACFLayer.h
index abc04c8..7243508 100644
--- a/WebCore/platform/graphics/win/WKCACFLayer.h
+++ b/WebCore/platform/graphics/win/WKCACFLayer.h
@@ -60,7 +60,11 @@ public:
BottomLeft, BottomRight, Resize, ResizeAspect, ResizeAspectFill };
static PassRefPtr<WKCACFLayer> create(LayerType);
- static WKCACFLayer* layer(CACFLayerRef layer) { return static_cast<WKCACFLayer*>(CACFLayerGetUserData(layer)); }
+ static WKCACFLayer* layer(CACFLayerRef layer)
+ {
+ ASSERT(CACFLayerGetUserData(layer) != reinterpret_cast<void*>(0xDEADBEEF));
+ return static_cast<WKCACFLayer*>(CACFLayerGetUserData(layer));
+ }
virtual ~WKCACFLayer();
@@ -133,7 +137,11 @@ public:
void adoptSublayers(WKCACFLayer* source);
void removeAllSublayers() { internalRemoveAllSublayers(); }
- void setSublayers(const Vector<RefPtr<WKCACFLayer> >& sublayers) { internalSetSublayers(sublayers); }
+ void setSublayers(const Vector<RefPtr<WKCACFLayer> >& sublayers)
+ {
+ internalSetSublayers(sublayers);
+ checkLayerConsistency();
+ }
void insertSublayer(PassRefPtr<WKCACFLayer> layer, size_t index) { internalInsertSublayer(layer, index); }
@@ -244,6 +252,13 @@ protected:
// This should only be called from removeFromSuperlayer.
void removeSublayer(const WKCACFLayer*);
+ void checkLayerConsistency()
+ {
+#ifndef NDEBUG
+ internalCheckLayerConsistency();
+#endif
+ }
+
// Methods to be overridden for sublayer and rendering management
virtual WKCACFLayer* internalSublayerAtIndex(int) const;
@@ -259,6 +274,10 @@ protected:
virtual void internalSetNeedsDisplay(const CGRect* dirtyRect);
#ifndef NDEBUG
+ virtual void internalCheckLayerConsistency();
+#endif
+
+#ifndef NDEBUG
// Print this layer and its children to the console
void printLayer(int indent) const;
#endif
diff --git a/WebCore/platform/graphics/win/WebTiledLayer.cpp b/WebCore/platform/graphics/win/WebTiledLayer.cpp
index d8c02d2..01dd6ae 100755
--- a/WebCore/platform/graphics/win/WebTiledLayer.cpp
+++ b/WebCore/platform/graphics/win/WebTiledLayer.cpp
@@ -36,6 +36,26 @@ namespace WebCore {
using namespace std;
+#ifndef NDEBUG
+void WebTiledLayer::internalCheckLayerConsistency()
+{
+ WKCACFLayer::internalCheckLayerConsistency();
+
+ // Additionally make sure the tiled parent is valid
+ CFArrayRef sublayers = CACFLayerGetSublayers(layer());
+
+ // Make sure there is a tile parent and it is the same as we remember
+ size_t n = CFArrayGetCount(sublayers);
+ ASSERT(n > 0);
+ const void* element = CFArrayGetValueAtIndex(sublayers, 0);
+ ASSERT(m_tileParent.get() == element);
+
+ // Make sure the tile parent doesn't have user data. If it does, it is probably
+ // a WKCACFLayer in the wrong place.
+ ASSERT(!layer(m_tileParent.get()));
+}
+#endif
+
void WebTiledLayer::tileDisplayCallback(CACFLayerRef layer, CGContextRef context)
{
static_cast<WebTiledLayer*>(CACFLayerGetUserData(layer))->drawTile(layer, context);
diff --git a/WebCore/platform/graphics/win/WebTiledLayer.h b/WebCore/platform/graphics/win/WebTiledLayer.h
index fdf0205..b8ae320 100644
--- a/WebCore/platform/graphics/win/WebTiledLayer.h
+++ b/WebCore/platform/graphics/win/WebTiledLayer.h
@@ -56,6 +56,10 @@ protected:
virtual void internalSetNeedsDisplay(const CGRect* dirtyRect);
+#ifndef NDEBUG
+ virtual void internalCheckLayerConsistency();
+#endif
+
private:
static void tileDisplayCallback(CACFLayerRef, CGContextRef);
void drawTile(CACFLayerRef, CGContextRef);
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list