[Webkit-unassigned] [Bug 122815] Optimize RenderBlock::lowestFloatLogicalBottom by caching previously calculated values

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=122815


Brent Fulgham <bfulgham at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #226311|review?                     |review-
               Flag|                            |




--- Comment #3 from Brent Fulgham <bfulgham at webkit.org>  2014-04-16 13:57:18 PST ---
(From update of attachment 226311)
View in context: https://bugs.webkit.org/attachment.cgi?id=226311&action=review

This looks like a good change, but I'd like some confirmation that we really want to switch from using an '&' test to the equality. Also, the naming scheme you've switched to is not acceptable.

> Source/WebCore/rendering/FloatingObjects.cpp:302
> +        for (FloatingObjectSet::const_iterator it = floatingObjectSet.begin(); it != end; ++it) {

This would read better as an 'auto'. The original code this is replacing was using auto previously.

> Source/WebCore/rendering/FloatingObjects.cpp:303
> +            const FloatingObject* r = it->get();

'floatingObject' is a much better name than 'r', and is what was used in the code you are replacing.

> Source/WebCore/rendering/FloatingObjects.cpp:317
> +        for (FloatingObjectSet::const_iterator it = floatingObjectSet.begin(); it != end; ++it) {

Ditto above (auto).

> Source/WebCore/rendering/FloatingObjects.cpp:318
> +            const FloatingObject* r = it->get();

Ditto above ('floatingObject' name).

> Source/WebCore/rendering/FloatingObjects.cpp:319
> +            if (r->isPlaced() && r->type() == floatType)

We used to use an '& floatType' test here. Why do we want equality now? Do we no longer need to handle the case where floatingObject->type() is a superset of types that encompasses 'floatType'?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.