[Webkit-unassigned] [Bug 52380] Lazily generate and store logical ordering of InlineBoxes

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Levi Weintraub <leviw at google.com>  2011-01-14 14:58:06 PST ---
(In reply to comment #7)
> (From update of attachment 79005 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=79005&action=review
> 
> Looks good.  tis OK as is, but we could go another round now, or we can clean up more of this (old ugly!) code in later patches.
> 
> > Source/WebCore/rendering/RootInlineBox.cpp:209
> > +void RootInlineBox::childAdded()
> 
> Seems if we're adding this method we might as well pass the InlineBox*, even if it's ignored.
> 
> > Source/WebCore/rendering/RootInlineBox.cpp:587
> > +        if (r->bidiLevel() > maxLevel)
> > +            maxLevel = r->bidiLevel();
> > +        if (r->bidiLevel() < minLevel)
> > +            minLevel = r->bidiLevel();
> 
> Hah.  These are just std::min, std::max :)
> 
> > Source/WebCore/rendering/RootInlineBox.cpp:615
> > +            while (iter != end) {
> > +                if ((*iter)->bidiLevel() >= minLevel)
> > +                    break;
> > +                ++iter;
> > +            }
> 
> This is repeated twice and should be an inline which returns a bool or similar maybe?
> 
> > Source/WebCore/rendering/RootInlineBox.h:79
> >      void childRemoved(InlineBox* box);
> 
> "box" can be removed.

All great points, thanks for the review!

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