Title: [111963] trunk/Source
- Revision
- 111963
- Author
- [email protected]
- Date
- 2012-03-23 18:28:12 -0700 (Fri, 23 Mar 2012)
Log Message
[chromium] Fix race bug that clobbers CCLayerImpl updateRect
https://bugs.webkit.org/show_bug.cgi?id=82109
Reviewed by Dirk Pranke.
Source/WebCore:
If the main thread commits twice before the impl thread actually
draws, then the updateRect of the first frame gets lost forever,
and not propagated to the damage tracker.
The solution is to accumulate the updateRect. The CCLayerImpl
updateRect is already being correctly cleared at the appropriate
time after drawing.
Unit test added to LayerChromiumTest.cpp.
* platform/graphics/chromium/LayerChromium.cpp:
(WebCore::LayerChromium::pushPropertiesTo):
Source/WebKit/chromium:
* tests/LayerChromiumTest.cpp:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (111962 => 111963)
--- trunk/Source/WebCore/ChangeLog 2012-03-24 01:25:20 UTC (rev 111962)
+++ trunk/Source/WebCore/ChangeLog 2012-03-24 01:28:12 UTC (rev 111963)
@@ -1,3 +1,23 @@
+2012-03-23 Shawn Singh <[email protected]>
+
+ [chromium] Fix race bug that clobbers CCLayerImpl updateRect
+ https://bugs.webkit.org/show_bug.cgi?id=82109
+
+ Reviewed by Dirk Pranke.
+
+ If the main thread commits twice before the impl thread actually
+ draws, then the updateRect of the first frame gets lost forever,
+ and not propagated to the damage tracker.
+
+ The solution is to accumulate the updateRect. The CCLayerImpl
+ updateRect is already being correctly cleared at the appropriate
+ time after drawing.
+
+ Unit test added to LayerChromiumTest.cpp.
+
+ * platform/graphics/chromium/LayerChromium.cpp:
+ (WebCore::LayerChromium::pushPropertiesTo):
+
2012-03-23 Rafael Weinstein <[email protected]>
[MutationObservers] attributeFilter should be case sensitive at all times
Modified: trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.cpp (111962 => 111963)
--- trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.cpp 2012-03-24 01:25:20 UTC (rev 111962)
+++ trunk/Source/WebCore/platform/graphics/chromium/LayerChromium.cpp 2012-03-24 01:28:12 UTC (rev 111963)
@@ -480,6 +480,11 @@
layer->setSublayerTransform(m_sublayerTransform);
if (!transformIsAnimating())
layer->setTransform(m_transform);
+
+ // If the main thread commits multiple times before the impl thread actually draws, then damage tracking
+ // will become incorrect if we simply clobber the updateRect here. The CCLayerImpl's updateRect needs to
+ // accumulate (i.e. union) any update changes that have occurred on the main thread.
+ m_updateRect.uniteIfNonZero(layer->updateRect());
layer->setUpdateRect(m_updateRect);
layer->setScrollDelta(layer->scrollDelta() - layer->sentScrollDelta());
Modified: trunk/Source/WebKit/chromium/ChangeLog (111962 => 111963)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-03-24 01:25:20 UTC (rev 111962)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-03-24 01:28:12 UTC (rev 111963)
@@ -1,3 +1,12 @@
+2012-03-23 Shawn Singh <[email protected]>
+
+ [chromium] Fix race bug that clobbers CCLayerImpl updateRect
+ https://bugs.webkit.org/show_bug.cgi?id=82109
+
+ Reviewed by Dirk Pranke.
+
+ * tests/LayerChromiumTest.cpp:
+
2012-03-23 Daniel Cheng <[email protected]>
[chromium] Support file drag out using DataTransferItemList::add(File)
Modified: trunk/Source/WebKit/chromium/tests/LayerChromiumTest.cpp (111962 => 111963)
--- trunk/Source/WebKit/chromium/tests/LayerChromiumTest.cpp 2012-03-24 01:25:20 UTC (rev 111962)
+++ trunk/Source/WebKit/chromium/tests/LayerChromiumTest.cpp 2012-03-24 01:28:12 UTC (rev 111963)
@@ -26,13 +26,14 @@
#include "LayerChromium.h"
-#include "cc/CCLayerTreeHost.h"
#include "CCLayerTreeTestCommon.h"
#include "FakeCCLayerTreeHostClient.h"
#include "LayerPainterChromium.h"
#include "NonCompositedContentHost.h"
#include "WebCompositor.h"
+#include "cc/CCLayerImpl.h"
#include "cc/CCLayerTreeHost.h"
+#include "cc/CCSingleThreadProxy.h"
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -530,6 +531,29 @@
EXPECT_TRUE(testLayer->needsDisplay());
}
+TEST_F(LayerChromiumTest, verifyPushPropertiesAccumulatesUpdateRect)
+{
+ DebugScopedSetImplThread setImplThread;
+
+ RefPtr<LayerChromium> testLayer = LayerChromium::create();
+ OwnPtr<CCLayerImpl> implLayer = CCLayerImpl::create(1);
+
+ testLayer->setNeedsDisplayRect(FloatRect(FloatPoint::zero(), FloatSize(5, 5)));
+ testLayer->pushPropertiesTo(implLayer.get());
+ EXPECT_FLOAT_RECT_EQ(FloatRect(FloatPoint::zero(), FloatSize(5, 5)), implLayer->updateRect());
+
+ // The CCLayerImpl's updateRect should be accumulated here, since we did not do anything to clear it.
+ testLayer->setNeedsDisplayRect(FloatRect(FloatPoint(10, 10), FloatSize(5, 5)));
+ testLayer->pushPropertiesTo(implLayer.get());
+ EXPECT_FLOAT_RECT_EQ(FloatRect(FloatPoint::zero(), FloatSize(15, 15)), implLayer->updateRect());
+
+ // If we do clear the CCLayerImpl side, then the next updateRect should be fresh without accumulation.
+ implLayer->resetAllChangeTrackingForSubtree();
+ testLayer->setNeedsDisplayRect(FloatRect(FloatPoint(10, 10), FloatSize(5, 5)));
+ testLayer->pushPropertiesTo(implLayer.get());
+ EXPECT_FLOAT_RECT_EQ(FloatRect(FloatPoint(10, 10), FloatSize(5, 5)), implLayer->updateRect());
+}
+
class LayerChromiumWithContentScaling : public LayerChromium {
public:
explicit LayerChromiumWithContentScaling()
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
