Title: [126198] trunk/Source
Revision
126198
Author
jam...@google.com
Date
2012-08-21 15:35:41 -0700 (Tue, 21 Aug 2012)

Log Message

[chromium] Should be able to destroy a CCLayerTreeHost without manually setting the root layer
https://bugs.webkit.org/show_bug.cgi?id=94631

Reviewed by Adrienne Walker.

Source/WebCore:

In the depths of time when dinosaurs roamed the earth, LayerChromium and CCLayerTreeHost were both reference
counted and there was a cycle between the root LayerChromium and CCLayerTreeHost. This required all users of
CCLayerTreeHost to manually break the cycle by calling setRootLayer(0) before dropping their reference to the
host. Nowadays, CCLayerTreeHost has a single owner and LayerChromiums only have a weak pointer to their host
so we should just do this cleanup ourselves instead of imposing it on callers.

Unit test added to LayerChromiumTest.cpp

* platform/graphics/chromium/cc/CCLayerTreeHost.cpp:
(WebCore::CCLayerTreeHost::~CCLayerTreeHost):

Source/WebKit/chromium:

Tests that destroying a CCLayerTreeHost that still points to a non-null root doesn't crash.

* tests/LayerChromiumTest.cpp:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (126197 => 126198)


--- trunk/Source/WebCore/ChangeLog	2012-08-21 22:35:21 UTC (rev 126197)
+++ trunk/Source/WebCore/ChangeLog	2012-08-21 22:35:41 UTC (rev 126198)
@@ -1,3 +1,21 @@
+2012-08-21  James Robinson  <jam...@chromium.org>
+
+        [chromium] Should be able to destroy a CCLayerTreeHost without manually setting the root layer
+        https://bugs.webkit.org/show_bug.cgi?id=94631
+
+        Reviewed by Adrienne Walker.
+
+        In the depths of time when dinosaurs roamed the earth, LayerChromium and CCLayerTreeHost were both reference
+        counted and there was a cycle between the root LayerChromium and CCLayerTreeHost. This required all users of
+        CCLayerTreeHost to manually break the cycle by calling setRootLayer(0) before dropping their reference to the
+        host. Nowadays, CCLayerTreeHost has a single owner and LayerChromiums only have a weak pointer to their host
+        so we should just do this cleanup ourselves instead of imposing it on callers.
+
+        Unit test added to LayerChromiumTest.cpp
+
+        * platform/graphics/chromium/cc/CCLayerTreeHost.cpp:
+        (WebCore::CCLayerTreeHost::~CCLayerTreeHost):
+
 2012-08-21  Ulan Degenbaev  <u...@chromium.org>
 
         Call AdjustAmountOfExternalAllocatedMemory when V8ArrayBuffer constructed and destructed

Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp (126197 => 126198)


--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp	2012-08-21 22:35:21 UTC (rev 126197)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp	2012-08-21 22:35:41 UTC (rev 126198)
@@ -113,6 +113,8 @@
 
 CCLayerTreeHost::~CCLayerTreeHost()
 {
+    if (m_rootLayer)
+        m_rootLayer->setLayerTreeHost(0);
     ASSERT(CCProxy::isMainThread());
     TRACE_EVENT0("cc", "CCLayerTreeHost::~CCLayerTreeHost");
     ASSERT(m_proxy);

Modified: trunk/Source/WebKit/chromium/ChangeLog (126197 => 126198)


--- trunk/Source/WebKit/chromium/ChangeLog	2012-08-21 22:35:21 UTC (rev 126197)
+++ trunk/Source/WebKit/chromium/ChangeLog	2012-08-21 22:35:41 UTC (rev 126198)
@@ -1,5 +1,16 @@
 2012-08-21  James Robinson  <jam...@chromium.org>
 
+        [chromium] Should be able to destroy a CCLayerTreeHost without manually setting the root layer
+        https://bugs.webkit.org/show_bug.cgi?id=94631
+
+        Reviewed by Adrienne Walker.
+
+        Tests that destroying a CCLayerTreeHost that still points to a non-null root doesn't crash.
+
+        * tests/LayerChromiumTest.cpp:
+
+2012-08-21  James Robinson  <jam...@chromium.org>
+
         Unreviewed, rolling out r126170.
         http://trac.webkit.org/changeset/126170
         https://bugs.webkit.org/show_bug.cgi?id=94614

Modified: trunk/Source/WebKit/chromium/tests/LayerChromiumTest.cpp (126197 => 126198)


--- trunk/Source/WebKit/chromium/tests/LayerChromiumTest.cpp	2012-08-21 22:35:21 UTC (rev 126197)
+++ trunk/Source/WebKit/chromium/tests/LayerChromiumTest.cpp	2012-08-21 22:35:41 UTC (rev 126198)
@@ -803,6 +803,18 @@
     WebKit::WebCompositor::shutdown();
 }
 
+TEST(LayerChromiumLayerTreeHostTest, destroyHostWithNonNullRootLayer)
+{
+    WebKit::WebCompositor::initialize(0);
+    RefPtr<LayerChromium> root = LayerChromium::create();
+    RefPtr<LayerChromium> child = LayerChromium::create();
+    root->addChild(child);
+    OwnPtr<FakeCCLayerTreeHost> layerTreeHost(FakeCCLayerTreeHost::create());
+    layerTreeHost->setRootLayer(root);
+    layerTreeHost.clear();
+    WebKit::WebCompositor::shutdown();
+}
+
 class MockLayerChromium : public LayerChromium {
 public:
     bool needsDisplay() const { return m_needsDisplay; }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to