Changeset 126198 in webkit

Timestamp:

Aug 21, 2012 3:35:41 PM (12 years ago)

Author:

jamesr@google.com

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:

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp (modified) (1 diff)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/tests/LayerChromiumTest.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-08-21  James Robinson  <jamesr@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  <ulan@chromium.org>
 

trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp

@@ -114 +114 @@
 CCLayerTreeHost::~CCLayerTreeHost()
 {
+    if (m_rootLayer)
+        m_rootLayer->setLayerTreeHost(0);
     ASSERT(CCProxy::isMainThread());
     TRACE_EVENT0("cc", "CCLayerTreeHost::~CCLayerTreeHost");

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2012-08-21  James Robinson  <jamesr@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  <jamesr@chromium.org>
 

trunk/Source/WebKit/chromium/tests/LayerChromiumTest.cpp

@@ -804 +804 @@
 }
 
+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: