Optimize GraphicsLayer::removeAllChildren()
Created attachment 244083 [details] Patch
Comment on attachment 244083 [details] Patch This breaks Mac/iOS because removeFromParent is no longer called, so call to noteSublayersChanged() no longer happens.
(In reply to comment #2) > Comment on attachment 244083 [details] > Patch > > This breaks Mac/iOS because removeFromParent is no longer called, so call to > noteSublayersChanged() no longer happens. Did not spot that one.
Created attachment 244153 [details] Patch
Comment on attachment 244153 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244153&action=review > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h:78 > + WEBCORE_EXPORT virtual void removeAllChildren() override; Please add __ZN7WebCore15GraphicsLayerCA17removeAllChildrenEv to WebCore.exp.in then run Tools/Scripts/sort-export-file. This should fix mac build
Created attachment 245884 [details] Patch
Comment on attachment 245884 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245884&action=review > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:456 > + noteSublayersChanged(); > + GraphicsLayer::removeAllChildren(); I think this should be flipped. > Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:191 > + didChangeChildren(); > + GraphicsLayer::removeAllChildren(); Ditto.
Created attachment 245897 [details] Patch
Comment on attachment 245897 [details] Patch Attachment 245897 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5014396556279808 Number of test failures exceeded the failure limit.
Created attachment 245901 [details] Archive of layout-test-results from ews106 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 245884 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245884&action=review >> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:456 >> + GraphicsLayer::removeAllChildren(); > > I think this should be flipped. This (and the previous iteration as well) caused a bunch of assertions on Mac -- noteSublayersChanged() has to be called for parent once for each child that is being removed.
Created attachment 246037 [details] Patch
Comment on attachment 246037 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246037&action=review > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:465 > + for (size_t i = 0; i < m_children.size(); ++i) > + noteSublayersChanged(); Do we really need to call noteSublayersChanged over and over again? How is that useful?
Comment on attachment 246037 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246037&action=review >> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:465 >> + noteSublayersChanged(); > > Do we really need to call noteSublayersChanged over and over again? How is that useful? No, we only need to call it once.
Comment on attachment 246037 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246037&action=review >>> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:465 >>> + noteSublayersChanged(); >> >> Do we really need to call noteSublayersChanged over and over again? How is that useful? > > No, we only need to call it once. As I wrote in comment #11, Mac EWS rejected the patch that only called noteSublayersChanged() (comment #9 and comment #10). I'm able to confirm the problem on Mac, with lots of crashes regardless of whether noteSublayersChanged() is called before or after GraphicsLayer::removeAllChildren(). I know nothing about CA and why the crashes occur, but keeping the previous behaviour of notifying about changed sublayers for each removed layer works without a problem. I'll attach backtraces from a debug build.