Bug 90342 – Make compositing updates incremental

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

It's expensive to traverse the whole layer tree for compositing updates, and when only a portion of the tree has changed, this is wasted work. We should try to do incremental updates when possible.

[Simon Fraser (smfr)]

Created attachment 150323 [details]
First cut, not ready for review.

[WebKit Review Bot]

Attachment 150323 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:10:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Comment on attachment 150323 [details]
First cut, not ready for review.

Attachment 150323 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13109780

[WebKit Review Bot]

Comment on attachment 150323 [details]
First cut, not ready for review.

Attachment 150323 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13110777

[Early Warning System Bot]

Comment on attachment 150323 [details]
First cut, not ready for review.

Attachment 150323 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13120686

[Radar WebKit Bug Importer]

<rdar://problem/11784760>

[Build Bot]

Comment on attachment 150323 [details]
First cut, not ready for review.

Attachment 150323 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13107811

[Simon Fraser (smfr)]

Created attachment 352389 [details]
Patch for EWS

[EWS Watchlist]

Attachment 352389 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderLayer.h:216:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/rendering/RenderLayer.h:225:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/rendering/RenderLayerCompositor.cpp:971:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/rendering/RenderLayerCompositor.cpp:1050:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/rendering/RenderLayerCompositor.cpp:3001:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/WebCore/rendering/RenderLayerCompositor.cpp:3058:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 6 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 352389 [details]
Patch for EWS

Attachment 352389 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9600029

New failing tests:
css3/blending/blend-mode-isolation-flags-turn-off-blending.html
legacy-animation-engine/compositing/layer-creation/mismatched-rotated-transform-animation-overlap.html
inspector/layers/layers-for-node.html
compositing/layer-creation/fixed-overlap-extent.html
legacy-animation-engine/compositing/layer-creation/translate-scale-animation-overlap.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html
legacy-animation-engine/compositing/layer-creation/multiple-keyframes-animation-overlap.html
inspector/layers/layerTreeDidChange.html

[EWS Watchlist]

Created attachment 352400 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352389 [details]
Patch for EWS

Attachment 352389 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9600380

New failing tests:
fast/visual-viewport/tiled-drawing/zoomed-fixed-scrolling-layers-state.html
css3/blending/blend-mode-isolation-flags-turn-off-blending.html
legacy-animation-engine/compositing/layer-creation/mismatched-rotated-transform-animation-overlap.html
inspector/layers/layers-for-node.html
legacy-animation-engine/compositing/layer-creation/translate-scale-animation-overlap.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html
legacy-animation-engine/compositing/layer-creation/multiple-keyframes-animation-overlap.html
inspector/layers/layerTreeDidChange.html
tiled-drawing/background-transparency-toggle.html
tiled-drawing/simple-document-with-dynamic-background-color.html

[EWS Watchlist]

Created attachment 352403 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352389 [details]
Patch for EWS

Attachment 352389 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9600408

New failing tests:
fast/scrolling/ios/textarea-scroll-touch.html
css3/blending/blend-mode-isolation-flags-turn-off-blending.html
legacy-animation-engine/compositing/layer-creation/mismatched-rotated-transform-animation-overlap.html
fast/scrolling/ios/overflow-scroll-touch.html
compositing/overflow/textarea-scroll-touch.html
legacy-animation-engine/compositing/layer-creation/translate-scale-animation-overlap.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html
legacy-animation-engine/compositing/layer-creation/multiple-keyframes-animation-overlap.html
compositing/scrolling/touch-scrolling-repaint.html
compositing/scrolling/touch-scrolling-repaint-spans.html
compositing/overflow/scrolling-without-painting.html
compositing/overflow/updating-scrolling-content.html

[EWS Watchlist]

Created attachment 352409 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 352389 [details]
Patch for EWS

Attachment 352389 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9601011

New failing tests:
css3/blending/blend-mode-isolation-flags-turn-off-blending.html
legacy-animation-engine/compositing/layer-creation/mismatched-rotated-transform-animation-overlap.html
inspector/layers/layers-for-node.html
compositing/layer-creation/fixed-overlap-extent.html
legacy-animation-engine/compositing/layer-creation/translate-scale-animation-overlap.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html
legacy-animation-engine/compositing/layer-creation/multiple-keyframes-animation-overlap.html
inspector/layers/layerTreeDidChange.html

[EWS Watchlist]

Created attachment 352413 [details]
Archive of layout-test-results from ews116 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Simon Fraser (smfr)]

Created attachment 352419 [details]
Patch

[EWS Watchlist]

Comment on attachment 352419 [details]
Patch

Attachment 352419 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9606370

New failing tests:
legacy-animation-engine/compositing/layer-creation/mismatched-rotated-transform-animation-overlap.html
inspector/layers/layers-for-node.html
compositing/layer-creation/fixed-overlap-extent.html
legacy-animation-engine/compositing/layer-creation/translate-scale-animation-overlap.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html
legacy-animation-engine/compositing/layer-creation/multiple-keyframes-animation-overlap.html
inspector/layers/layerTreeDidChange.html

[EWS Watchlist]

Created attachment 352423 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352419 [details]
Patch

Attachment 352419 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9607187

New failing tests:
fast/visual-viewport/tiled-drawing/zoomed-fixed-scrolling-layers-state.html
legacy-animation-engine/compositing/layer-creation/mismatched-rotated-transform-animation-overlap.html
inspector/layers/layers-for-node.html
legacy-animation-engine/compositing/layer-creation/translate-scale-animation-overlap.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html
legacy-animation-engine/compositing/layer-creation/multiple-keyframes-animation-overlap.html
inspector/layers/layerTreeDidChange.html

[EWS Watchlist]

Created attachment 352427 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352419 [details]
Patch

Attachment 352419 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9607366

New failing tests:
legacy-animation-engine/compositing/layer-creation/mismatched-rotated-transform-animation-overlap.html
inspector/layers/layers-for-node.html
compositing/layer-creation/fixed-overlap-extent.html
legacy-animation-engine/compositing/layer-creation/translate-scale-animation-overlap.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html
legacy-animation-engine/compositing/layer-creation/multiple-keyframes-animation-overlap.html
inspector/layers/layerTreeDidChange.html

[EWS Watchlist]

Created attachment 352430 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352419 [details]
Patch

Attachment 352419 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9607369

New failing tests:
fast/scrolling/ios/textarea-scroll-touch.html
compositing/scrolling/touch-scrolling-repaint.html
legacy-animation-engine/compositing/layer-creation/mismatched-rotated-transform-animation-overlap.html
fast/scrolling/ios/overflow-scroll-touch.html
compositing/overflow/textarea-scroll-touch.html
legacy-animation-engine/compositing/layer-creation/translate-scale-animation-overlap.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html
legacy-animation-engine/compositing/layer-creation/multiple-keyframes-animation-overlap.html
compositing/scrolling/touch-scrolling-repaint-spans.html
compositing/overflow/scrolling-without-painting.html
compositing/overflow/updating-scrolling-content.html

[EWS Watchlist]

Created attachment 352431 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 352419 [details]
Patch

Attachment 352419 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9611340

New failing tests:
legacy-animation-engine/compositing/layer-creation/mismatched-rotated-transform-animation-overlap.html
inspector/layers/layers-for-node.html
compositing/layer-creation/fixed-overlap-extent.html
legacy-animation-engine/compositing/layer-creation/translate-scale-animation-overlap.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html
legacy-animation-engine/compositing/layer-creation/multiple-keyframes-animation-overlap.html
inspector/layers/layerTreeDidChange.html

[EWS Watchlist]

Created attachment 352439 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Simon Fraser (smfr)]

Created attachment 352440 [details]
Patch

[EWS Watchlist]

Comment on attachment 352440 [details]
Patch

Attachment 352440 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9615197

New failing tests:
compositing/layer-creation/fixed-overlap-extent.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html

[EWS Watchlist]

Created attachment 352441 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352440 [details]
Patch

Attachment 352440 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9615320

New failing tests:
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html

[EWS Watchlist]

Created attachment 352442 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352440 [details]
Patch

Attachment 352440 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9615610

New failing tests:
fast/scrolling/ios/textarea-scroll-touch.html
compositing/scrolling/touch-scrolling-repaint.html
fast/scrolling/ios/overflow-scroll-touch.html
compositing/overflow/textarea-scroll-touch.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html
compositing/scrolling/touch-scrolling-repaint-spans.html
compositing/overflow/scrolling-without-painting.html
compositing/overflow/updating-scrolling-content.html

[EWS Watchlist]

Created attachment 352443 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 352440 [details]
Patch

Attachment 352440 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9615671

New failing tests:
compositing/layer-creation/fixed-overlap-extent.html
webgl/1.0.2/conformance/canvas/buffer-preserve-test.html

[EWS Watchlist]

Created attachment 352444 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Simon Fraser (smfr)]

Created attachment 352477 [details]
Patch

[EWS Watchlist]

Comment on attachment 352477 [details]
Patch

Attachment 352477 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9625070

New failing tests:
compositing/layer-creation/fixed-overlap-extent.html

[EWS Watchlist]

Created attachment 352489 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352477 [details]
Patch

Attachment 352477 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9625956

New failing tests:
compositing/layer-creation/fixed-overlap-extent.html

[EWS Watchlist]

Created attachment 352501 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352477 [details]
Patch

Attachment 352477 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9626299

New failing tests:
fast/scrolling/ios/textarea-scroll-touch.html
compositing/scrolling/touch-scrolling-repaint.html
fast/scrolling/ios/overflow-scroll-touch.html
compositing/overflow/textarea-scroll-touch.html
compositing/scrolling/touch-scrolling-repaint-spans.html
compositing/overflow/scrolling-without-painting.html
compositing/overflow/updating-scrolling-content.html

[EWS Watchlist]

Created attachment 352505 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Simon Fraser (smfr)]

Created attachment 352531 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 352572 [details]
Patch

[EWS Watchlist]

Comment on attachment 352572 [details]
Patch

Attachment 352572 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9637237

New failing tests:
css3/filters/backdrop/backdrop-filter-with-border-radius-value-remove.html
compositing/layer-creation/overlap-animation-container.html
css3/filters/backdrop/backdrop-filter-with-border-radius-value-change.html
compositing/video/video-border-radius.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html

[EWS Watchlist]

Created attachment 352579 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352572 [details]
Patch

Attachment 352572 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9637311

New failing tests:
css3/filters/backdrop/backdrop-filter-with-border-radius-value-change.html
compositing/video/video-border-radius.html
tiled-drawing/toggle-to-fixed-background.html
imported/blink/compositing/reorder-z-with-style.html
compositing/layer-creation/overlap-animation-container.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html
css3/filters/backdrop/backdrop-filter-with-border-radius-value-remove.html

[EWS Watchlist]

Created attachment 352586 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352572 [details]
Patch

Attachment 352572 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9637493

New failing tests:
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html
compositing/layer-creation/overlap-animation-container.html
css3/filters/backdrop/backdrop-filter-with-border-radius-value-remove.html
compositing/video/video-border-radius.html
css3/filters/backdrop/backdrop-filter-with-border-radius-value-change.html

[EWS Watchlist]

Created attachment 352595 [details]
Archive of layout-test-results from ews112 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352572 [details]
Patch

Attachment 352572 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9644471

New failing tests:
css3/filters/backdrop/backdrop-filter-with-border-radius-value-change.html
compositing/video/video-border-radius.html
imported/blink/compositing/reorder-z-with-style.html
compositing/layer-creation/overlap-animation-container.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html
css3/filters/backdrop/backdrop-filter-with-border-radius-value-remove.html

[EWS Watchlist]

Created attachment 352665 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Simon Fraser (smfr)]

Created attachment 352711 [details]
Patch

[EWS Watchlist]

Comment on attachment 352711 [details]
Patch

Attachment 352711 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9653453

New failing tests:
css3/filters/backdrop/backdrop-filter-with-border-radius-value-change.html
compositing/layer-creation/overlap-animation-container.html
css3/filters/backdrop/backdrop-filter-with-border-radius-value-remove.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html

[EWS Watchlist]

Created attachment 352716 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352711 [details]
Patch

Attachment 352711 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9653486

New failing tests:
css3/filters/backdrop/backdrop-filter-with-border-radius-value-change.html
tiled-drawing/toggle-to-fixed-background.html
imported/blink/compositing/reorder-z-with-style.html
compositing/layer-creation/overlap-animation-container.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html
css3/filters/backdrop/backdrop-filter-with-border-radius-value-remove.html

[EWS Watchlist]

Created attachment 352718 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352711 [details]
Patch

Attachment 352711 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9653533

New failing tests:
css3/filters/backdrop/backdrop-filter-with-border-radius-value-remove.html
compositing/layer-creation/overlap-animation-container.html
css3/filters/backdrop/backdrop-filter-with-border-radius-value-change.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html

[EWS Watchlist]

Created attachment 352722 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352711 [details]
Patch

Attachment 352711 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9653560

New failing tests:
css3/filters/backdrop/backdrop-filter-with-border-radius-value-remove.html
imported/blink/compositing/reorder-z-with-style.html
compositing/layer-creation/overlap-animation-container.html
css3/filters/backdrop/backdrop-filter-with-border-radius-value-change.html
legacy-animation-engine/compositing/layer-creation/overlap-animation-container.html

[EWS Watchlist]

Created attachment 352724 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Simon Fraser (smfr)]

Created attachment 352869 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 352870 [details]
Patch

[EWS Watchlist]

Comment on attachment 352870 [details]
Patch

Attachment 352870 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9685665

New failing tests:
imported/blink/compositing/reorder-z-with-style.html

[EWS Watchlist]

Created attachment 352871 [details]
Archive of layout-test-results from ews105 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 352870 [details]
Patch

Attachment 352870 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9685746

New failing tests:
imported/blink/compositing/reorder-z-with-style.html

[EWS Watchlist]

Created attachment 352872 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Simon Fraser (smfr)]

Created attachment 353585 [details]
Patch

[EWS Watchlist]

Attachment 353585 [details] did not pass style-queue:


ERROR: /Volumes/Data/StyleQueue/WebKit/LayoutTests/platform/mac-wk1/TestExpectations:654:  More specific entry for compositing/repaint/iframes/compositing-iframe-scroll-repaint.html on line LayoutTests/platform/mac-wk1/TestExpectations:627 overrides line LayoutTests/platform/mac-wk1/TestExpectations:654.  [test/expectations] [5]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 353585 [details]
Patch

Attachment 353585 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9808490

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 353586 [details]
Archive of layout-test-results from ews112 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 353585 [details]
Patch

Attachment 353585 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9808615

New failing tests:
compositing/video/video-clip-change-src.html

[EWS Watchlist]

Created attachment 353587 [details]
Archive of layout-test-results from ews105 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Simon Fraser (smfr)]

Created attachment 354091 [details]
Patch

[EWS Watchlist]

Comment on attachment 354091 [details]
Patch

Attachment 354091 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9894847

New failing tests:
compositing/video/video-clip-change-src.html

[EWS Watchlist]

Created attachment 354105 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 354091 [details]
Patch

Attachment 354091 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9894514

New failing tests:
fast/dynamic/jQuery-animation-crash.html

[EWS Watchlist]

Created attachment 354109 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Simon Fraser (smfr)]

Created attachment 354390 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 354396 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 354400 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 354406 [details]
Patch

[EWS Watchlist]

Comment on attachment 354406 [details]
Patch

Attachment 354406 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9931946

New failing tests:
compositing/video/video-clip-change-src.html

[EWS Watchlist]

Created attachment 354421 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Simon Fraser (smfr)]

Created attachment 354456 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 354475 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 354491 [details]
Patch

[EWS Watchlist]

Attachment 354491 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderLayer.h:397:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Created attachment 354508 [details]
Patch

[EWS Watchlist]

Attachment 354508 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderLayer.h:397:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

*** Bug 81239 has been marked as a duplicate of this bug. ***

[EWS Watchlist]

Comment on attachment 354508 [details]
Patch

Attachment 354508 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9950641

New failing tests:
compositing/filters/sw-layer-overlaps-hw-shadow.html

[EWS Watchlist]

Created attachment 354510 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Simon Fraser (smfr)]

Failing test just needs iOS results.

[Antti Koivisto]

Comment on attachment 354508 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=354508&action=review

r=me, nice!

> Source/WebCore/ChangeLog:33
> +        These changes fix a correctness issue where transform changes now trigger overlap re-evaluation, which causes
> +        more layer geometry updates than before. This regressed the MotionMark "Focus" test, when geometry updates
> +        triggered layer resizes as the filter blur radius changed, which then triggered repaints. This is fixed by
> +        excluding composited filters from the composited bounds (but still taking them into account for overlap).

Do we have some performance tests that progress?

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2162
> +    if (madeLayer) {
>          updateBackdropFiltersRect();
> +        noteSublayersChanged(DontScheduleFlush);
> +    }

Why not schedule a flush?

> Source/WebCore/rendering/RenderLayer.cpp:279
>      , m_forcedStackingContext(rendererLayerModelObject.isMedia())
>      , m_zOrderListsDirty(false)
>      , m_normalFlowListDirty(true)
> +    , m_hadNegativeZOrderList(false)
>      , m_inResizeMode(false)
>      , m_scrollDimensionsDirty(true)
>      , m_hasSelfPaintingLayerDescendant(false)

Would be nice to turn these into normal bools (or OptionSets) at some point. Pretty sure memory optimization from bitfields is not meaningful.

> Source/WebCore/rendering/RenderLayer.cpp:617
> +    RenderLayer* layer = parent();

auto*

> Source/WebCore/rendering/RenderLayer.h:197
> +    enum CompositingStateFlag {

You might consider making this 'enum class CompositingState', 'CompositingFlag' or just 'Compositing' and dropping the then redundant word 'Compositing' from the value names.

> Source/WebCore/rendering/RenderLayer.h:256
> +    bool needsPostLayoutCompositingUpdate() const { return m_compositingDirtyBits.contains(NeedsPostLayoutCompositingUpdate); }

Not sure how all these accessors are better than making the enum public and exposing m_compositingDirtyBits via const accessor. They are not used that much.

> Source/WebCore/rendering/RenderLayer.h:261
> +    void setNeedsPostLayoutCompositingUpdate()
> +    {
> +        m_compositingDirtyBits.add(NeedsPostLayoutCompositingUpdate);
> +        setAncestorsHaveCompositingDirtyFlag(HasDescendantNeedingCompositingRequirementsTraversal);
> +    }

Do all these really need to be in the header? It is already big.

> Source/WebCore/rendering/RenderLayerBacking.cpp:629
>      } else
>          m_artificiallyInflatedBounds = false;
>  
> -    setCompositedBounds(layerBounds);
> +    return setCompositedBounds(layerBounds);

Could m_artificiallyInflatedBounds change even when the computed bounds stay the same? Should we return true in that case?

> Source/WebCore/rendering/RenderLayerBacking.cpp:661
> +// This can only update things that don't require up-to-date layout.
> +void RenderLayerBacking::updateConfigurationAfterStyleChange()

updateConfigurationAfterStyleChangeWithoutRequiringUpToDateLayout? :D

> Source/WebCore/rendering/RenderLayerCompositor.cpp:227
> +        , fullPaintOrderTraversalRequired(false)
> +        , descendantsRequireCompositingUpdate(false)
>          , ancestorHasTransformAnimation(false)

These could be switched to modern initialization.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:657
> +// Returns true on a successful update.
>  bool RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType updateType, RenderLayer* updateRoot)

enum return value would avoid the comment.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:661
> +    UNUSED_PARAM(updateType);
> +    UNUSED_PARAM(updateRoot);

Neither of these parameters is unused.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:691
> +        // Scrolling can affect overlap. FIXME: avoid for page scrolling.
> +        updateRoot->setDescendantsNeedCompositingRequirementsTraversal();

Sounds like a big FIXME!

> Source/WebCore/rendering/RenderLayerCompositor.cpp:697
> +    if (!updateRoot->hasDescendantNeedingCompositingRequirementsTraversal() && !m_compositing) {
> +        LOG_WITH_STREAM(Compositing, stream << " no compositing work to do");
>          return false;
> +    }

Why is this case considered 'unsuccessful'...

> Source/WebCore/rendering/RenderLayerCompositor.cpp:702
> +    if (!updateRoot->needsAnyCompositingTraversal()) {
> +        LOG_WITH_STREAM(Compositing, stream << " updateRoot has no dirty child and doesn't need update");
> +        return true;
> +    }

while this is considered 'successful'.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:802
> +    compositingState.fullPaintOrderTraversalRequired |= layer.needsCompositingRequirementsTraversal();
> +    compositingState.descendantsRequireCompositingUpdate |= layer.descendantsNeedCompositingRequirementsTraversal();

Using bit operators like this is slightly ugly.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1019
> +// We have to traverse unchanged layers to fill in the overlap map.
> +void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer, RenderLayer& layer, OverlapMap& overlapMap, CompositingState& compositingState, bool& descendantHas3DTransform)

Can results of this traversal be cached? Presumably the results don't change if the layers are unchanged.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1252
> +void RenderLayerCompositor::layerBecameNonComposited(const RenderLayer& layer)
> +{
> +    // Inform the inspector that the given RenderLayer was destroyed.
> +    InspectorInstrumentation::renderLayerDestroyed(&page(), layer);

Something here seems like a lie. Was the layer destroyed or did it just become non-composited?

> Source/WebCore/rendering/RenderLayerCompositor.h:142
> +    struct CompositingQueryData {

This seems to introduce concept of "compositing query" but there are no functions or anything else referencing this concept, so it is bit difficult to understand how it is used.

> Source/WebCore/rendering/RenderLayerCompositor.h:144
> +        RenderLayer::ViewportConstrainedNotCompositedReason nonCompositedForPositionReason { RenderLayer::NoNotCompositedReason };

NoNotCompositedReason :(

> Source/WebCore/rendering/RenderLayerCompositor.h:351
> +    bool needsToBeComposited(const RenderLayer&, CompositingQueryData&) const;
>      // Whether the layer has an intrinsic need for compositing layer.
> -    bool requiresCompositingLayer(const RenderLayer&, RenderLayer::ViewportConstrainedNotCompositedReason* = nullptr) const;
> +    bool requiresCompositingLayer(const RenderLayer&, CompositingQueryData&) const;

It is bit unclear if CompositingQueryData is an input, output or both for number of functions that take it. Could it be a (perhaps optional) return value when it is a pure output? Many clients initialize it in stack and invoke some function without doing anything between.

> Source/WebCore/rendering/RenderLayerCompositor.h:379
> +    enum class UpdateLevel {
> +        AllDescendants          = 1 << 0,
> +        CompositedChildren      = 1 << 1,
> +    };
>      // Recurses down the tree, parenting descendant compositing layers and collecting an array of child layers for the current compositing layer.
> -    void rebuildCompositingLayerTree(RenderLayer&, Vector<Ref<GraphicsLayer>>& childGraphicsLayersOfEnclosingLayer, int depth);
> +    void updateBackingAndHierarchy(RenderLayer&, Vector<Ref<GraphicsLayer>>& childGraphicsLayersOfEnclosingLayer, OptionSet<UpdateLevel> = { }, int depth = 0);

Why OptionSet? Doesn't 'AllDescendants' also cover 'CompositedChildren'? The logic might read better with a regular 3-value enum.

[Antti Koivisto]

Comment on attachment 354508 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=354508&action=review

> Source/WebCore/rendering/RenderLayer.cpp:690
> +    if (hasNegativeZOrderList != m_hadNegativeZOrderList) {
> +        m_hadNegativeZOrderList = hasNegativeZOrderList;

The bit seems the reflect the current state so maybe m_has instead of m_had?

> Source/WebCore/rendering/RenderLayer.h:1180
> +    OptionSet<CompositingStateFlag> m_compositingDirtyBits;

Are these "state flags" or "dirty bits"? It would be nice if the type name and the field name matched.

[Simon Fraser (smfr)]

(In reply to Antti Koivisto from comment #97)
> Comment on attachment 354508 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354508&action=review
> 
> r=me, nice!
> 
> > Source/WebCore/ChangeLog:33
> > +        These changes fix a correctness issue where transform changes now trigger overlap re-evaluation, which causes
> > +        more layer geometry updates than before. This regressed the MotionMark "Focus" test, when geometry updates
> > +        triggered layer resizes as the filter blur radius changed, which then triggered repaints. This is fixed by
> > +        excluding composited filters from the composited bounds (but still taking them into account for overlap).
> 
> Do we have some performance tests that progress?

Yes, MotionMark shows it on the Focus test.

> > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2162
> > +    if (madeLayer) {
> >          updateBackdropFiltersRect();
> > +        noteSublayersChanged(DontScheduleFlush);
> > +    }
> 
> Why not schedule a flush?

We're already inside a flush here.

> > Source/WebCore/rendering/RenderLayer.cpp:279
> >      , m_forcedStackingContext(rendererLayerModelObject.isMedia())
> >      , m_zOrderListsDirty(false)
> >      , m_normalFlowListDirty(true)
> > +    , m_hadNegativeZOrderList(false)
> >      , m_inResizeMode(false)
> >      , m_scrollDimensionsDirty(true)
> >      , m_hasSelfPaintingLayerDescendant(false)
> 
> Would be nice to turn these into normal bools (or OptionSets) at some point.
> Pretty sure memory optimization from bitfields is not meaningful.

Some pages can have thousands of RenderLayers, so I think some amount of space optimization is worthwhile.

C++17 (?) gives us initializers for bitfields.

> > Source/WebCore/rendering/RenderLayer.h:197
> > +    enum CompositingStateFlag {
> 
> You might consider making this 'enum class CompositingState',
> 'CompositingFlag' or just 'Compositing' and dropping the then redundant word
> 'Compositing' from the value names.
> 
> > Source/WebCore/rendering/RenderLayer.h:256
> > +    bool needsPostLayoutCompositingUpdate() const { return m_compositingDirtyBits.contains(NeedsPostLayoutCompositingUpdate); }
> 
> Not sure how all these accessors are better than making the enum public and
> exposing m_compositingDirtyBits via const accessor. They are not used that
> much.

I tried it the other way, but this way makes the code using the accessors more compact. Maybe grouping all the accessors together would make this header less noisy.

> > Source/WebCore/rendering/RenderLayer.h:261
> > +    void setNeedsPostLayoutCompositingUpdate()
> > +    {
> > +        m_compositingDirtyBits.add(NeedsPostLayoutCompositingUpdate);
> > +        setAncestorsHaveCompositingDirtyFlag(HasDescendantNeedingCompositingRequirementsTraversal);
> > +    }
> 
> Do all these really need to be in the header? It is already big.

I shrunk things down a bit.

> > Source/WebCore/rendering/RenderLayerBacking.cpp:629
> >      } else
> >          m_artificiallyInflatedBounds = false;
> >  
> > -    setCompositedBounds(layerBounds);
> > +    return setCompositedBounds(layerBounds);
> 
> Could m_artificiallyInflatedBounds change even when the computed bounds stay
> the same? Should we return true in that case?

I think it's OK; all that matters whether the composited bounds changed (whether or not they were inflated).

> > Source/WebCore/rendering/RenderLayerCompositor.cpp:227
> > +        , fullPaintOrderTraversalRequired(false)
> > +        , descendantsRequireCompositingUpdate(false)
> >          , ancestorHasTransformAnimation(false)
> 
> These could be switched to modern initialization.

Done

> > Source/WebCore/rendering/RenderLayerCompositor.cpp:691
> > +        // Scrolling can affect overlap. FIXME: avoid for page scrolling.
> > +        updateRoot->setDescendantsNeedCompositingRequirementsTraversal();
> 
> Sounds like a big FIXME!

Yeah, filed bug 191546 to track.

> > Source/WebCore/rendering/RenderLayerCompositor.cpp:697
> > +    if (!updateRoot->hasDescendantNeedingCompositingRequirementsTraversal() && !m_compositing) {
> > +        LOG_WITH_STREAM(Compositing, stream << " no compositing work to do");
> >          return false;
> > +    }
> 
> Why is this case considered 'unsuccessful'...

Fixed.

> > Source/WebCore/rendering/RenderLayerCompositor.cpp:802
> > +    compositingState.fullPaintOrderTraversalRequired |= layer.needsCompositingRequirementsTraversal();
> > +    compositingState.descendantsRequireCompositingUpdate |= layer.descendantsNeedCompositingRequirementsTraversal();
> 
> Using bit operators like this is slightly ugly.

Is there a nicer way?

> > Source/WebCore/rendering/RenderLayerCompositor.cpp:1019
> > +// We have to traverse unchanged layers to fill in the overlap map.
> > +void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer, RenderLayer& layer, OverlapMap& overlapMap, CompositingState& compositingState, bool& descendantHas3DTransform)
> 
> Can results of this traversal be cached? Presumably the results don't change
> if the layers are unchanged.

We'd have to keep the overlap map around, and then know how to dirty it.


> 
> > Source/WebCore/rendering/RenderLayerCompositor.cpp:1252
> > +void RenderLayerCompositor::layerBecameNonComposited(const RenderLayer& layer)
> > +{
> > +    // Inform the inspector that the given RenderLayer was destroyed.
> > +    InspectorInstrumentation::renderLayerDestroyed(&page(), layer);
> 
> Something here seems like a lie. Was the layer destroyed or did it just
> become non-composited?

Agreed. Filed bug 191547.

> > Source/WebCore/rendering/RenderLayerCompositor.h:142
> > +    struct CompositingQueryData {
> 
> This seems to introduce concept of "compositing query" but there are no
> functions or anything else referencing this concept, so it is bit difficult
> to understand how it is used.

Yeah. The "query" is the "do you need to composite" question. This struct really just wraps up a bunch of in and out params to these functions. I'll rename this to RequiresCompositingData.

> > Source/WebCore/rendering/RenderLayerCompositor.h:144
> > +        RenderLayer::ViewportConstrainedNotCompositedReason nonCompositedForPositionReason { RenderLayer::NoNotCompositedReason };
> 
> NoNotCompositedReason :(

Not new :|

> > Source/WebCore/rendering/RenderLayerCompositor.h:351
> > +    bool needsToBeComposited(const RenderLayer&, CompositingQueryData&) const;
> >      // Whether the layer has an intrinsic need for compositing layer.
> > -    bool requiresCompositingLayer(const RenderLayer&, RenderLayer::ViewportConstrainedNotCompositedReason* = nullptr) const;
> > +    bool requiresCompositingLayer(const RenderLayer&, CompositingQueryData&) const;
> 
> It is bit unclear if CompositingQueryData is an input, output or both for
> number of functions that take it. Could it be a (perhaps optional) return
> value when it is a pure output? Many clients initialize it in stack and
> invoke some function without doing anything between.

It has both inputs and outputs.

> > Source/WebCore/rendering/RenderLayerCompositor.h:379
> > +    enum class UpdateLevel {
> > +        AllDescendants          = 1 << 0,
> > +        CompositedChildren      = 1 << 1,
> > +    };
> >      // Recurses down the tree, parenting descendant compositing layers and collecting an array of child layers for the current compositing layer.
> > -    void rebuildCompositingLayerTree(RenderLayer&, Vector<Ref<GraphicsLayer>>& childGraphicsLayersOfEnclosingLayer, int depth);
> > +    void updateBackingAndHierarchy(RenderLayer&, Vector<Ref<GraphicsLayer>>& childGraphicsLayersOfEnclosingLayer, OptionSet<UpdateLevel> = { }, int depth = 0);
> 
> Why OptionSet? Doesn't 'AllDescendants' also cover 'CompositedChildren'? The
> logic might read better with a regular 3-value enum.

AllDescendants trumps CompositedChildren, yes. With a 3-state, there would be more if-then code when toggling CompositedChildren.

[Simon Fraser (smfr)]

(In reply to Antti Koivisto from comment #98)
> Comment on attachment 354508 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354508&action=review
> 
> > Source/WebCore/rendering/RenderLayer.cpp:690
> > +    if (hasNegativeZOrderList != m_hadNegativeZOrderList) {
> > +        m_hadNegativeZOrderList = hasNegativeZOrderList;
> 
> The bit seems the reflect the current state so maybe m_has instead of m_had?

It's "had" in the sense that last time we updated z-order lists, we saw negative z children, but after lists are dirtied (which clears them), we are in a limbo state.

> > Source/WebCore/rendering/RenderLayer.h:1180
> > +    OptionSet<CompositingStateFlag> m_compositingDirtyBits;
> 
> Are these "state flags" or "dirty bits"? It would be nice if the type name
> and the field name matched.

It's OptionSet<Compositing> m_compositingDirtyBits now. "state" implies information about the steady state of the layer; these are dirty bits.

[Simon Fraser (smfr)]

https://trac.webkit.org/r238090

[Antti Koivisto]

> > Using bit operators like this is slightly ugly.
> 
> Is there a nicer way?

Both if() and operator|| avoid unnecessary evaluation of the right side (though it doesn't matter in practice here).

[Charlie Turner]

This breaks release builds with logging enabled. I've noticed a few times now that there's some confusion about when to use if !LOG_DISABLED, ifndef NDEBUG and now in this patch if ENABLE(TREE_DEBUGGING). We need a bot building release with logging enabled to catch these imo, I know I'm not the only one building like this, because full debug builds are too slow on my hardware :). Could someone have a look at 191583 where I've applied some build fixes for release+logging

[Jon Lee]

rdar://problem/11784760