Summary: | RenderLayerCompositor should let ScrollingCoordinator update main thread scrolling reasons after change of layers | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Xianzhu Wang <wangxianzhu> | ||||||||||
Component: | Layout and Rendering | Assignee: | Xianzhu Wang <wangxianzhu> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | andersca, eric, jamesr, ojan.autocc, simon.fraser, tonikitoo, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Xianzhu Wang
2012-12-21 12:41:17 PST
Created attachment 180578 [details]
Patch
Comment on attachment 180578 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180578&action=review > Source/WebCore/rendering/RenderLayerCompositor.cpp:473 > + if (scrollingCoordinator && m_fixedPositionLayerNotCompositedReasonMap != originalFixedPositionLayerNotCompositedReasonMap) I don't really like m_fixedPositionLayerNotCompositedReasonMap, and the fact that you're comparing two HashMaps here. (In reply to comment #2) > (From update of attachment 180578 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=180578&action=review > > > Source/WebCore/rendering/RenderLayerCompositor.cpp:473 > > + if (scrollingCoordinator && m_fixedPositionLayerNotCompositedReasonMap != originalFixedPositionLayerNotCompositedReasonMap) > > I don't really like m_fixedPositionLayerNotCompositedReasonMap, and the fact that you're comparing two HashMaps here. (I thought I posted the following on last Friday, but just found I made it lost somehow) How about moving the reason information map into individual RenderLayers or RenderLayerModelObjects as a new field? Created attachment 181770 [details]
Patch
Comment on attachment 181770 [details] Patch Attachment 181770 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15758280 Created attachment 181789 [details]
Fix mac break
Simon, do you like the latest patch? Kindly ping reviewers... Comment on attachment 181789 [details] Fix mac break View in context: https://bugs.webkit.org/attachment.cgi?id=181789&action=review Nice cleanup, thanks! > Source/WebCore/rendering/RenderLayerCompositor.cpp:636 > + if (layerChanged) > + if (ScrollingCoordinator* scrollingCoordinator = this->scrollingCoordinator()) > + scrollingCoordinator->frameViewFixedObjectsDidChange(m_renderView->frameView()); We put {} around multi-line conditional blocks like this. Created attachment 182365 [details]
for landing
Comment on attachment 181789 [details] Fix mac break View in context: https://bugs.webkit.org/attachment.cgi?id=181789&action=review >> Source/WebCore/rendering/RenderLayerCompositor.cpp:636 >> + scrollingCoordinator->frameViewFixedObjectsDidChange(m_renderView->frameView()); > > We put {} around multi-line conditional blocks like this. Done. Comment on attachment 182365 [details] for landing Clearing flags on attachment: 182365 Committed r139461: <http://trac.webkit.org/changeset/139461> All reviewed patches have been landed. Closing bug. |