[chromium] Force update of nonFastScrollableRegion if target CCLayerImpl has been freshly created.
Created attachment 133347 [details] Patch
I'll add a test if the approach in this patch is deemed correct.
Comment on attachment 133347 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133347&action=review Good catch! This looks like the right fix, but it definitely needs test coverage before R+. Dropping CCLayerImpl's on !visible is troublesome, we should perhaps revisit that sometimes. > Source/WebCore/ChangeLog:8 > + No new tests, yet. (OOPS!) This needs tests, obviously. > Source/WebCore/platform/graphics/chromium/TreeSynchronizer.cpp:76 > + // FIXME: Is it valid to assume a reused CCLayerImpl will have the correct nonFastScrollableRegion? yes, it is > Source/WebCore/platform/graphics/chromium/TreeSynchronizer.cpp:77 > + layer->setNonFastScrollableRegionChanged(); // We just cleared the impl layer, so don't opt-out of transferring the nonFastScrollableRegion. the double negative isn't hard to misunderstand here. rephrase perhaps?
Comment on attachment 133347 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133347&action=review >> Source/WebCore/ChangeLog:8 >> + No new tests, yet. (OOPS!) > > This needs tests, obviously. Working on one now, will upload with new patch shortly. >> Source/WebCore/platform/graphics/chromium/TreeSynchronizer.cpp:76 >> + // FIXME: Is it valid to assume a reused CCLayerImpl will have the correct nonFastScrollableRegion? > > yes, it is Good to know ... FIXME comment removed. >> Source/WebCore/platform/graphics/chromium/TreeSynchronizer.cpp:77 >> + layer->setNonFastScrollableRegionChanged(); // We just cleared the impl layer, so don't opt-out of transferring the nonFastScrollableRegion. > > the double negative isn't hard to misunderstand here. rephrase perhaps? Fixed.
Created attachment 133374 [details] Patch
(In reply to comment #3) > (From update of attachment 133347 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133347&action=review > > Good catch! This looks like the right fix, but it definitely needs test coverage before R+. > > Dropping CCLayerImpl's on !visible is troublesome, we should perhaps revisit that sometimes. I don't really agree, but am willing to be convinced. I think the problem here is really that there's this additional caching flag for whether or not this property should be synced or not. Properties that simply always get synced have no issue and no extra complications.
Comment on attachment 133374 [details] Patch Looks good to me. Thanks for the test.
Comment on attachment 133374 [details] Patch Clearing flags on attachment: 133374 Committed r111803: <http://trac.webkit.org/changeset/111803>
All reviewed patches have been landed. Closing bug.