Bug 54682 - Improvement in code quality, memory usage, and performance in ContainerNode.cpp
Summary: Improvement in code quality, memory usage, and performance in ContainerNode.cpp
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All S60 3rd edition
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-17 12:46 PST by sangeetha.sugavanam
Modified: 2011-05-19 09:51 PDT (History)
1 user (show)

See Also:


Attachments
patch improving code quality, performance, and memory usage in ContainerNode.cpp (2.39 KB, patch)
2011-02-17 12:50 PST, sangeetha.sugavanam
levin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description sangeetha.sugavanam 2011-02-17 12:46:51 PST
This bug improves the code quality, performance, and memory usage of the file ContainerNode.cpp by getting rid of an unneeded pointer declaration.
Comment 1 sangeetha.sugavanam 2011-02-17 12:50:40 PST
Created attachment 82850 [details]
patch improving code quality, performance, and memory usage in ContainerNode.cpp
Comment 2 David Levin 2011-02-17 18:04:21 PST
Comment on attachment 82850 [details]
patch improving code quality, performance, and memory usage in ContainerNode.cpp

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.
Comment 3 sangeetha.sugavanam 2011-05-19 09:51:24 PDT
This bug should also be closed cause it is not possible create appropriate test cases for it.