[webkit-reviews] review denied: [Bug 28131] [Haiku] Adding font-specific files to WebCore. : [Attachment 34421] Patch to add four font-specific files to WebCore/platform/graphics/haiku/.

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Maxime Simon
<simon.maxime at gmail.com>'s request for review:
Bug 28131: [Haiku] Adding font-specific files to WebCore.
https://bugs.webkit.org/show_bug.cgi?id=28131

Attachment 34421: Patch to add four font-specific files to
WebCore/platform/graphics/haiku/.
https://bugs.webkit.org/attachment.cgi?id=34421&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
 39 static void releaseData(void* data)
looks to never be used, please remove it.

strange indenting here:
     struct FontCustomPlatformData : Noncopyable {
 33	    public:
 34		FontCustomPlatformData() { }
 35

No need for the "buffer" argument name here:
 40	FontCustomPlatformData* createFontCustomPlatformData(SharedBuffer*
buffer);

Wrong:
  * This file is part of the internal font implementation.  It should not be
included by anyone other than
 3  * FontMac.cpp, FontWin.cpp and Font.cpp.

So strange:
120	if (m_font && m_font != hashTableDeletedFontValue())
 121	     delete m_font;
That's gonna die a horrible death with this as a copy-constructor:
FontPlatformData::FontPlatformData(const FontPlatformData& other)
 114 {
 115	 *this = other;
 116 }
your' going to be double-deleting pointers left and right.

Why does FontPlatformData use FontDescription at all?  the Mac version doesn't
at least.  Shouldn't this just wrap a BFont?