[Webkit-unassigned] [Bug 43716] Use a HashMap for m_continuation to save memory

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 13 17:47:00 PST 2010


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





--- Comment #15 from Kenichi Ishibashi <bashi at google.com>  2010-12-13 17:46:59 PST ---
(From update of attachment 66184)
View in context: https://bugs.webkit.org/attachment.cgi?id=66184&action=review

Hi Darin,

Thank you very much for reviewing! I've applied your comments and updated to ToT. I'll send the patch soon.

>> WebCore/rendering/RenderBlock.h:135
>> +    void setContinuation(RenderBoxModelObject* continuation) { RenderBoxModelObject::setContinuation(continuation); }
> 
> As I said before, this should be “using” rather than a forwarding inline function.

Done. I misunderstood the usage of "using." Thank you!

>> WebCore/rendering/RenderBoxModelObject.cpp:63
>> +static ContinuationMap* gContinuationMap = 0;
> 
> The use of the “g” prefix here is not common in WebKit code. I did a search and found that most global variables of this type do not use this sort of prefix.

Thanks for the advice. Removed the "g" prefix.

>> WebCore/rendering/RenderBoxModelObject.cpp:1796
>> +        gContinuationMap->remove(this);
> 
> The creation of the map could be moved inside the non-zero case, since is no need to create the map to remove something from it. Probably OK the way it is, though, since we’d still need a null check of gContinuationMap.

I agree with you. Moved the creation of the map inside the non-zero case.

>> WebCore/rendering/RenderInline.h:63
>> +    void setContinuation(RenderBoxModelObject* continuation) { RenderBoxModelObject::setContinuation(continuation); }
> 
> Again, I think “using” is the way to go.

Done.

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


More information about the webkit-unassigned mailing list