Diff
Modified: trunk/LayoutTests/ChangeLog (183709 => 183710)
--- trunk/LayoutTests/ChangeLog 2015-05-02 02:23:01 UTC (rev 183709)
+++ trunk/LayoutTests/ChangeLog 2015-05-02 02:32:29 UTC (rev 183710)
@@ -1,3 +1,17 @@
+2015-05-01 Simon Fraser <[email protected]>
+
+ Avoid compositing updates after style recalcs which have no compositing implications
+ https://bugs.webkit.org/show_bug.cgi?id=144502
+
+ Reviewed by Darin Adler.
+
+ Use internals.compositingUpdateCount() to see if various document mutations
+ cause a compositing update. Doesn't actually detect any behavior change
+ from this patch, but seems useful in general.
+
+ * compositing/updates/no-style-change-updates-expected.txt: Added.
+ * compositing/updates/no-style-change-updates.html: Added.
+
2015-05-01 Ryosuke Niwa <[email protected]>
Class syntax should allow string and numeric identifiers for method names
Added: trunk/LayoutTests/compositing/updates/no-style-change-updates-expected.txt (0 => 183710)
--- trunk/LayoutTests/compositing/updates/no-style-change-updates-expected.txt (rev 0)
+++ trunk/LayoutTests/compositing/updates/no-style-change-updates-expected.txt 2015-05-02 02:32:29 UTC (rev 183710)
@@ -0,0 +1,14 @@
+Test that compositing updates do not happen for style changes that do not need them.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS updateCount is 0
+PASS updateCount is 1
+PASS updateCount is 0
+PASS updateCount is 1
+PASS updateCount is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/compositing/updates/no-style-change-updates.html (0 => 183710)
--- trunk/LayoutTests/compositing/updates/no-style-change-updates.html (rev 0)
+++ trunk/LayoutTests/compositing/updates/no-style-change-updates.html 2015-05-02 02:32:29 UTC (rev 183710)
@@ -0,0 +1,109 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+ <style>
+ .box {
+ width: 100px;
+ height: 100px;
+ margin: 10px;
+ background-color: blue;
+ border: 2px solid black;
+ -webkit-transform: translateZ(0);
+ }
+
+ .colorchange {
+ background-color: green;
+ }
+
+ .dummy {
+ /* Empty. */
+ }
+
+ </style>
+ <script src=""
+ <script>
+ description('Test that compositing updates do not happen for style changes that do not need them.');
+ window.jsTestIsAsync = true;
+
+ var updateCount;
+
+ function zeroUpdateCount()
+ {
+ if (window.internals) {
+ internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
+ internals.startTrackingCompositingUpdates();
+ }
+ }
+
+ function updateLayoutAndCompositingCount()
+ {
+ if (window.internals) {
+ internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
+ updateCount = internals.compositingUpdateCount();
+ }
+ }
+
+ function testForCompositingUpdate(callback)
+ {
+ zeroUpdateCount();
+ callback();
+ updateLayoutAndCompositingCount();
+ }
+
+ function runTest()
+ {
+ updateLayoutAndCompositingCount();
+
+ if (window.internals) {
+ internals.startTrackingCompositingUpdates();
+ updateCount = internals.compositingUpdateCount();
+ }
+
+ shouldBe('updateCount', '0');
+
+ testForCompositingUpdate(function() {
+ document.getElementById('box').classList.add('colorchange');
+ });
+
+ shouldBe('updateCount', '1');
+
+ testForCompositingUpdate(function() {
+ document.getElementById('box').classList.add('dummy');
+ });
+
+ shouldBe('updateCount', '0');
+
+ testForCompositingUpdate(function() {
+ var div = document.createElement('div');
+ div.className = 'box';
+ document.body.appendChild(div);
+ });
+
+ shouldBe('updateCount', '1');
+
+ testForCompositingUpdate(function() {
+ var stylesheet = document.createElement('style');
+ stylesheet.type = 'text/css';
+ stylesheet.rel = 'stylesheet';
+ stylesheet.textContent = '.unmatched { border: 5px solid blue; }';
+ document.getElementsByTagName("head")[0].appendChild(stylesheet);
+ });
+
+ shouldBe('updateCount', '0');
+
+ finishJSTest();
+ }
+
+ window.addEventListener('load', function() {
+ window.setTimeout(runTest, 200);
+ }, false);
+ </script>
+
+</head>
+<body>
+ <div id="box" class="box"></div>
+
+ <script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (183709 => 183710)
--- trunk/Source/WebCore/ChangeLog 2015-05-02 02:23:01 UTC (rev 183709)
+++ trunk/Source/WebCore/ChangeLog 2015-05-02 02:32:29 UTC (rev 183710)
@@ -1,3 +1,43 @@
+2015-05-01 Simon Fraser <[email protected]>
+
+ Avoid compositing updates after style recalcs which have no compositing implications
+ https://bugs.webkit.org/show_bug.cgi?id=144502
+
+ Reviewed by Darin Adler.
+
+ After r183461, we have reliable information about whether a style change with zero
+ diff can be reliably ignored. Use that information to track whether a given
+ recalcStyle() does anything which should force a compositing update.
+
+ This eliminates up to 40% of the post-recalcStyle compositing updates on some pages.
+
+ Add Internals API to test.
+
+ Test: compositing/updates/no-style-change-updates.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::recalcStyle): Tell the FrameView we're going to recalc style.
+ * page/FrameView.cpp:
+ (WebCore::FrameView::willRecalcStyle): Pass it on to the compositor.
+ (WebCore::FrameView::updateCompositingLayersAfterStyleChange): Move the code
+ that was here into RenderLayerCompositor::didRecalcStyleWithNoPendingLayout().
+ * page/FrameView.h:
+ * rendering/RenderLayerCompositor.cpp:
+ (WebCore::RenderLayerCompositor::willRecalcStyle): Reset the m_layerNeedsCompositingUpdate flag.
+ (WebCore::RenderLayerCompositor::didRecalcStyleWithNoPendingLayout): Bail on the update if
+ no layers changed.
+ (WebCore::RenderLayerCompositor::updateCompositingLayers): Logging. Increment m_compositingUpdateCount,
+ which is used for testing.
+ (WebCore::RenderLayerCompositor::layerStyleChanged): Set the m_layerNeedsCompositingUpdate flag.
+ (WebCore::RenderLayerCompositor::startTrackingCompositingUpdates): Reset the counter.
+ (WebCore::RenderLayerCompositor::compositingUpdateCount):
+ * rendering/RenderLayerCompositor.h:
+ * testing/Internals.cpp:
+ (WebCore::Internals::startTrackingCompositingUpdates):
+ (WebCore::Internals::compositingUpdateCount):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
2015-05-01 Andreas Kling <[email protected]>
Reproducible crash removing name attribute from <img> node
Modified: trunk/Source/WebCore/dom/Document.cpp (183709 => 183710)
--- trunk/Source/WebCore/dom/Document.cpp 2015-05-02 02:23:01 UTC (rev 183709)
+++ trunk/Source/WebCore/dom/Document.cpp 2015-05-02 02:32:29 UTC (rev 183710)
@@ -1748,6 +1748,8 @@
m_styleSheetCollection.flushPendingUpdates();
+ frameView.willRecalcStyle();
+
InspectorInstrumentationCookie cookie = InspectorInstrumentation::willRecalculateStyle(*this);
// FIXME: We never reset this flags.
Modified: trunk/Source/WebCore/page/FrameView.cpp (183709 => 183710)
--- trunk/Source/WebCore/page/FrameView.cpp 2015-05-02 02:23:01 UTC (rev 183709)
+++ trunk/Source/WebCore/page/FrameView.cpp 2015-05-02 02:32:29 UTC (rev 183710)
@@ -728,6 +728,15 @@
}
}
+void FrameView::willRecalcStyle()
+{
+ RenderView* renderView = this->renderView();
+ if (!renderView)
+ return;
+
+ renderView->compositor().willRecalcStyle();
+}
+
void FrameView::updateCompositingLayersAfterStyleChange()
{
RenderView* renderView = this->renderView();
@@ -738,10 +747,7 @@
if (inPreLayoutStyleUpdate() || layoutPending() || renderView->needsLayout())
return;
- RenderLayerCompositor& compositor = renderView->compositor();
- // This call will make sure the cached hasAcceleratedCompositing is updated from the pref
- compositor.cacheAcceleratedCompositingFlags();
- compositor.updateCompositingLayers(CompositingUpdateAfterStyleChange);
+ renderView->compositor().didRecalcStyleWithNoPendingLayout();
}
void FrameView::updateCompositingLayersAfterLayout()
Modified: trunk/Source/WebCore/page/FrameView.h (183709 => 183710)
--- trunk/Source/WebCore/page/FrameView.h 2015-05-02 02:23:01 UTC (rev 183709)
+++ trunk/Source/WebCore/page/FrameView.h 2015-05-02 02:32:29 UTC (rev 183710)
@@ -147,6 +147,7 @@
WEBCORE_EXPORT void serviceScriptedAnimations(double monotonicAnimationStartTime);
#endif
+ void willRecalcStyle();
void updateCompositingLayersAfterStyleChange();
void updateCompositingLayersAfterLayout();
bool flushCompositingStateForThisFrame(Frame* rootFrameForFlush);
Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (183709 => 183710)
--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2015-05-02 02:23:01 UTC (rev 183709)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2015-05-02 02:32:29 UTC (rev 183710)
@@ -384,6 +384,20 @@
m_compositingLayersNeedRebuild = needRebuild;
}
+void RenderLayerCompositor::willRecalcStyle()
+{
+ m_layerNeedsCompositingUpdate = false;
+}
+
+void RenderLayerCompositor::didRecalcStyleWithNoPendingLayout()
+{
+ if (!m_layerNeedsCompositingUpdate)
+ return;
+
+ cacheAcceleratedCompositingFlags();
+ updateCompositingLayers(CompositingUpdateAfterStyleChange);
+}
+
void RenderLayerCompositor::customPositionForVisibleRectComputation(const GraphicsLayer* graphicsLayer, FloatPoint& position) const
{
if (graphicsLayer != m_scrollLayer.get())
@@ -656,6 +670,8 @@
void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType updateType, RenderLayer* updateRoot)
{
+ LOG(Compositing, "RenderLayerCompositor %p updateCompositingLayers %d %p", this, updateType, updateRoot);
+
m_updateCompositingLayersTimer.stop();
ASSERT(!m_renderView.document().inPageCache());
@@ -674,6 +690,8 @@
if (!m_reevaluateCompositingAfterLayout && !m_compositing)
return;
+ ++m_compositingUpdateCount;
+
AnimationUpdateBlock animationUpdateBlock(&m_renderView.frameView().frame().animation());
TemporaryChange<bool> postLayoutChange(m_inPostLayoutUpdate, true);
@@ -710,6 +728,8 @@
if (isFullUpdate && updateType == CompositingUpdateAfterLayout)
m_reevaluateCompositingAfterLayout = false;
+ LOG(Compositing, " checkForHierarchyUpdate %d, needGeometryUpdate %d", checkForHierarchyUpdate, needHierarchyUpdate);
+
#if !LOG_DISABLED
double startTime = 0;
if (compositingLogEnabled()) {
@@ -920,6 +940,8 @@
if (diff == StyleDifferenceEqual)
return;
+ m_layerNeedsCompositingUpdate = true;
+
const RenderStyle& newStyle = layer.renderer().style();
if (updateLayerCompositingState(layer) || (oldStyle && styleChangeRequiresLayerRebuild(layer, *oldStyle, newStyle)))
setCompositingLayersNeedRebuild();
@@ -4186,4 +4208,14 @@
return m_layerFlushCount;
}
+void RenderLayerCompositor::startTrackingCompositingUpdates()
+{
+ m_compositingUpdateCount = 0;
+}
+
+unsigned RenderLayerCompositor::compositingUpdateCount() const
+{
+ return m_compositingUpdateCount;
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.h (183709 => 183710)
--- trunk/Source/WebCore/rendering/RenderLayerCompositor.h 2015-05-02 02:23:01 UTC (rev 183709)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.h 2015-05-02 02:32:29 UTC (rev 183710)
@@ -119,6 +119,9 @@
// created, destroyed or re-parented).
void setCompositingLayersNeedRebuild(bool needRebuild = true);
bool compositingLayersNeedRebuild() const { return m_compositingLayersNeedRebuild; }
+
+ void willRecalcStyle();
+ void didRecalcStyleWithNoPendingLayout();
// GraphicsLayers buffer state, which gets pushed to the underlying platform layers
// at specific times.
@@ -311,6 +314,9 @@
WEBCORE_EXPORT void startTrackingLayerFlushes();
WEBCORE_EXPORT unsigned layerFlushCount() const;
+ WEBCORE_EXPORT void startTrackingCompositingUpdates();
+ WEBCORE_EXPORT unsigned compositingUpdateCount() const;
+
private:
class OverlapMap;
struct CompositingState;
@@ -498,6 +504,7 @@
int m_compositedLayerCount { 0 };
unsigned m_layersWithTiledBackingCount { 0 };
unsigned m_layerFlushCount { 0 };
+ unsigned m_compositingUpdateCount { 0 };
RootLayerAttachment m_rootLayerAttachment;
@@ -534,6 +541,7 @@
bool m_layerFlushThrottlingEnabled;
bool m_layerFlushThrottlingTemporarilyDisabledForInteraction;
bool m_hasPendingLayerFlush;
+ bool m_layerNeedsCompositingUpdate { false };
Timer m_paintRelatedMilestonesTimer;
Modified: trunk/Source/WebCore/testing/Internals.cpp (183709 => 183710)
--- trunk/Source/WebCore/testing/Internals.cpp 2015-05-02 02:23:01 UTC (rev 183709)
+++ trunk/Source/WebCore/testing/Internals.cpp 2015-05-02 02:32:29 UTC (rev 183710)
@@ -2114,6 +2114,28 @@
return document->styleRecalcCount();
}
+void Internals::startTrackingCompositingUpdates(ExceptionCode& ec)
+{
+ Document* document = contextDocument();
+ if (!document || !document->renderView()) {
+ ec = INVALID_ACCESS_ERR;
+ return;
+ }
+
+ document->renderView()->compositor().startTrackingCompositingUpdates();
+}
+
+unsigned long Internals::compositingUpdateCount(ExceptionCode& ec)
+{
+ Document* document = contextDocument();
+ if (!document || !document->renderView()) {
+ ec = INVALID_ACCESS_ERR;
+ return 0;
+ }
+
+ return document->renderView()->compositor().compositingUpdateCount();
+}
+
void Internals::updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(ExceptionCode& ec)
{
updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(nullptr, ec);
Modified: trunk/Source/WebCore/testing/Internals.h (183709 => 183710)
--- trunk/Source/WebCore/testing/Internals.h 2015-05-02 02:23:01 UTC (rev 183709)
+++ trunk/Source/WebCore/testing/Internals.h 2015-05-02 02:32:29 UTC (rev 183710)
@@ -303,6 +303,9 @@
void startTrackingStyleRecalcs(ExceptionCode&);
unsigned long styleRecalcCount(ExceptionCode&);
+ void startTrackingCompositingUpdates(ExceptionCode&);
+ unsigned long compositingUpdateCount(ExceptionCode&);
+
void updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(ExceptionCode&);
void updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(Node*, ExceptionCode&);
Modified: trunk/Source/WebCore/testing/Internals.idl (183709 => 183710)
--- trunk/Source/WebCore/testing/Internals.idl 2015-05-02 02:23:01 UTC (rev 183709)
+++ trunk/Source/WebCore/testing/Internals.idl 2015-05-02 02:32:29 UTC (rev 183710)
@@ -280,6 +280,9 @@
[RaisesException] void startTrackingStyleRecalcs();
[RaisesException] unsigned long styleRecalcCount();
+ [RaisesException] void startTrackingCompositingUpdates();
+ [RaisesException] unsigned long compositingUpdateCount();
+
// |node| should be Document, HTMLIFrameElement, or unspecified.
// If |node| is an HTMLIFrameElement, it assumes node.contentDocument is
// specified without security checks. Unspecified means this document.
