[Webkit-unassigned] [Bug 54682] Improvement in code quality, memory usage, and performance in ContainerNode.cpp

[bugzilla-daemon at webkit.org]

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


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #82850|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #2 from David Levin <levin at chromium.org>  2011-02-17 18:04:21 PST ---
(From update of attachment 82850)
View in context: https://bugs.webkit.org/attachment.cgi?id=82850&action=review

> Source/WebCore/ChangeLog:5
> +        Need a short description and bug URL 

Typically you'd remove this line.

> Source/WebCore/ChangeLog:6
> +I am improving code quality in the method getUpperLeftCorner() in the file ContainerNode.cpp.  In this function, originally we were setting the value of pointer p to the value of pointer o.  By having a careful look at the code, it looks like we do not need to have the pointer p because it is not used anywhere.  The bug url for this is https://bugs.webkit.org/show_bug?id=54682.  

Typically, you'd indent this line (and wrap the text).

> Source/WebCore/ChangeLog:8
> +        No new tests. 

This line should go away.

> Source/WebCore/ChangeLog:9
> +There are no tests for this particular case because this is an improvement in code quality which does not in anyway impact the behavior of the given function.  

This line should be indented.

> Source/WebCore/ChangeLog:11
> +

In short see other ChangeLog entries and how they are formated.

> Source/WebCore/dom/ContainerNode.cpp:788
>      // What is this code really trying to do?

Love this comment.

> Source/WebCore/dom/ContainerNode.cpp:821
> +        if (o->node() && o->node() == this && o->isText() && !o->isBR() && !toRenderText(o)->firstTextBox()) {

It is not true that p == o here. Note that if o has a first child, then o gets moved to that, but p stays where it is, so this code is not the same as before.

If this code change passes all layout tests, it would be interesting to create a layout test fails due to this change.

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