[webkit-reviews] review granted: [Bug 27671] Add functions to print the glyph page trees for debugging : [Attachment 33478] Add functions to print the glyph page trees for debugging

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted mitz at webkit.org's request for review:
Bug 27671: Add functions to print the glyph page trees for debugging
https://bugs.webkit.org/show_bug.cgi?id=27671

Attachment 33478: Add functions to print the glyph page trees for debugging
https://bugs.webkit.org/attachment.cgi?id=33478&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +#ifndef NDEBUG
> +#include "PlatformString.h"
> +#endif

I'm conflicted about how useful it is to have the #include be #ifndef'd. Maybe
it's better to not do that.

But also, declaring a function that returns a String requires only a forward
declaration, not the definition, so the include isn't needed.

> +#ifndef NDEBUG
> +#include "PlatformString.h"
> +#endif
> +
>  namespace WebCore {
>  
>  class FontPlatformData {
> @@ -107,6 +111,10 @@ public:
>  #endif
>      };
>  
> +#ifndef NDEBUG
> +    String description() const { return String(); }
> +#endif

In this case, and others like it, "PlatformString.h" is needed, but only
because the implementation is in the header. Can we put the implementation in a
.cpp file instead?

r=me, but consider a version with less PlatformString.h