[webkit-reviews] review granted: [Bug 125278] Save original text for RenderText to a map : [Attachment 218529] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 5 12:34:03 PST 2013


Darin Adler <darin at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 125278: Save original text for RenderText to a map
https://bugs.webkit.org/show_bug.cgi?id=125278

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=218529&action=review


Gotta fix the NDEBUG problem. Otherwise looks good to me, and I can’t spot any
bugs. Not sure we have sufficient test coverage.

> Source/WebCore/rendering/RenderText.cpp:100
> +typedef HashMap<const RenderText*, String> OriginalTextMap;

Not sure we benefit from using a typedef for this since the type is only used
twice. Also extra blank line before this typedef.

> Source/WebCore/rendering/RenderText.cpp:204
>  RenderText::~RenderText()
>  {
> +    if (m_originalTextDiffersFromRendered)
> +	   originalTextMap().remove(this);
>  }

We need to take this out of #ifndef NDEBUG since we are going to use it for
non-debugging purposes.


More information about the webkit-reviews mailing list