[webkit-reviews] review denied: [Bug 28131] [Haiku] Adding font-specific files to WebCore. : [Attachment 38638] Adding four font-specific files to WebCore.

[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 38638: Adding four font-specific files to WebCore.
https://bugs.webkit.org/attachment.cgi?id=38638&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Not needed:
+#include "NotImplemented.h"

I probably wouldn't have bothered to maintain these copyrights after such
extensive modification:
2  * Copyright (C) 2006 Apple Computer, Inc.
 3  * Copyright (C) 2006 Zack Rusin <zack at kde.org>
 4  * Copyright (C) 2006 Dirk Mueller <mueller at kde.org>
 5  * Copyright (C) 2006 Nikolas Zimmermann <zimmermann at kde.org>
 6  * Copyright (C) 2007 Ryan Leavengood <leavengood at gmail.com>
 7  * Copyright (C) 2009 Maxime Simon <simon.maxime at gmail.com>

This:
115	if (m_font)
 116	     return m_font->Spacing() == B_FIXED_SPACING;
 117 
 118	 return false;
Should just be:

return m_font && m_font->Spacing() == B_FIXED_SPACING;

Why is this needed?
09     if (m_font && m_font != hashTableDeletedFontValue())
 110	     m_font = 0;
Seems useless in the destructor.  setting the raw pointer to 0 does nothing. :)


So you're leaking m_font.  I think you meant to make it an OwnPtr<BFont>

This is going to need at least one more round of review since there were still
obvious leaks in this one. :(  Do you all have any way to check for leaks on
Haiku?