[Webkit-unassigned] [Bug 37508] [Haiku] Rework FontPlatformData implementation

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 22 12:38:57 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=37508


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54037|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #5 from Adam Barth <abarth at webkit.org>  2010-04-22 12:38:57 PST ---
(From update of attachment 54037)
WebCore/ChangeLog:1
 +  2010-04-13  Stephan Aßmus  <superstippi at gmx.de>
Is this character in your name correct or do we have some sort of encoding
problem?

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:44
 +          if (familyName.contains("Sans", false) != B_ERROR)
This seems really hacky.  What if there's a non-san-serif font that happens to
contain the string "sans"?

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:81
 +  // #pragma mark -
What does this comment mean?

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:91
 +      void ref();
Why doesn't this inherit from RefCounted?

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:107
 +      : m_refCount(1)
Usually we have a static FontPlatformData::create method to help people not
leak because of initializing the refcount to 1.

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:152
 +  // #pragma mark -
This again.

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:202
 +          m_data->ref();
Please don't use manual reference counting like this.  We have lots of tools to
avoid these error-prone patterns.

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:217
 +           m_data->deref();
Bad indent.

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:231
 +          || isHashTableDeletedValue() || other.isHashTableDeletedValue()) {
Why is this clause split in a strange place?

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:243
 +      return &(m_data->m_font);
No parens needed.

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:290
 +      return m_data == reinterpret_cast<FontPlatformDataPrivate*>(-1);
Yuck

WebCore/platform/graphics/haiku/FontPlatformDataHaiku.cpp:309
 +      return String(fontFamily) + "/" + String(fontStyle) +
String::format("/%.1f/%d&%d", size, isBold, isOblique);
Why do you use concatenation for part of this string but String::format for the
rest?  Please pick one or the other.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list