Summary: | Move useRepaintBounds from RenderBlock::layoutRunsAndFloats to LineLayoutState | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexandru Chiculita <achicu> | ||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | hyatt, mihnea, morrita, rniwa, tony, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Alexandru Chiculita
2011-07-12 07:32:34 PDT
Created attachment 100483 [details]
Patch
I'm trying to split the RenderBlock::layoutRunsAndFloats into smaller methods. In the end it should make it easier to fix floats bugs that affect CSS Columns. I will add a master bug for it. Comment on attachment 100483 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100483&action=review > Source/WebCore/rendering/RenderBlockLineLayout.cpp:853 > + bool useRepaintBounds() const { return m_useRepaintBounds; } "uses" or "shouldUse"? (I know original variable name was wrong....) > Source/WebCore/rendering/RenderBlockLineLayout.cpp:-1293 > - if (!useRepaintBounds) Could you elaborate why we don't need this? Comment on attachment 100483 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100483&action=review Thanks for the review! >> Source/WebCore/rendering/RenderBlockLineLayout.cpp:853 >> + bool useRepaintBounds() const { return m_useRepaintBounds; } > > "uses" or "shouldUse"? (I know original variable name was wrong....) Ok. >> Source/WebCore/rendering/RenderBlockLineLayout.cpp:-1293 >> - if (!useRepaintBounds) > > Could you elaborate why we don't need this? Every time the updateRepaintRangeFromBox was called there was an if (!useRepaintBounds) useRepaintBounds = true. So I've moved it to updateRepaintRangeFromBox and removed the if clause. It should be faster to just update it every time than to check it and update only if needed. Created attachment 100642 [details]
Patch
- Changed useRepaintBounds to usesRepaintBounds.
Comment on attachment 100642 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100642&action=review > Source/WebCore/ChangeLog:8 > + No new tests needed. Please specify why no new tests are needed (e.g., because it's just a refactor). Created attachment 100935 [details]
Patch
Thanks for review. I've added the reason in the ChangeLog file. I'm not a committer, so I'm uploading the file again for review.
Comment on attachment 100935 [details] Patch Clearing flags on attachment: 100935 Committed r91057: <http://trac.webkit.org/changeset/91057> All reviewed patches have been landed. Closing bug. Some Chromium UI tests appear to start failing after this change but it's probably just a flake: http://build.chromium.org/p/chromium.webkit/builders/Mac10.6%20Tests/builds/5799 http://build.chromium.org/p/chromium.webkit/builders/Mac10.6%20Tests/builds/5800 |