[webkit-reviews] review denied: [Bug 58113] DocumentMarkerController::MarkerMapVectorPair should be replaced with a list of some class. : [Attachment 90231] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied MORITA Hajime <morrita at google.com>'s
request for review:
Bug 58113: DocumentMarkerController::MarkerMapVectorPair should be replaced
with a list of some class.
https://bugs.webkit.org/show_bug.cgi?id=58113

Attachment 90231: Patch
https://bugs.webkit.org/attachment.cgi?id=90231&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=90231&action=review

Does this change improve memory usage or is it just a refactor.  Might be good
to include that in the changelog.

r- to get out of the review queue so you can upload a version with the fixed
ChangeLog

> Source/WebCore/ChangeLog:47
> +2011-04-19  MORITA Hajime  <morrita at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).

Change description shows up twice.

> Source/WebCore/dom/DocumentMarkerController.cpp:338
> +	   unsigned markerCount = list->size();

Nit: Should we use size_t here?

> Source/WebCore/dom/RenderedDocumentMarker.h:50
> +    static IntRect invalidMarkerRect() { return IntRect(-1, -1, -1, -1); }

Nit: Should we just make this a const static member variable to avoid making a
copy for every call?  I guess that has the downside of needing to add a .cpp
file.