Bug 26949 - WebCore part of the Haiku WebKit port
: WebCore part of the Haiku WebKit port
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: All Other
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-07-02 23:59 PST by
Modified: 2009-08-15 03:12 PST (History)


Attachments
Patch to add Haiku-specific files for WebCore/platform/image-decoders/. (5.44 KB, patch)
2009-07-03 00:03 PST, Maxime Simon
eric: review-
Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific files for WebCore/platform/text/. (4.77 KB, patch)
2009-07-03 00:04 PST, Maxime Simon
no flags Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific files for WebCore/bindings/js/. (3.12 KB, patch)
2009-07-03 00:13 PST, Maxime Simon
no flags Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific files for WebCore/editing/. (2.95 KB, patch)
2009-07-03 00:21 PST, Maxime Simon
no flags Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific files for WebCore/page/. (11.82 KB, patch)
2009-07-03 00:35 PST, Maxime Simon
no flags Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific font files for WebCore/platform/graphics/. (27.53 KB, patch)
2009-07-03 01:54 PST, Maxime Simon
eric: review-
Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/. (57.83 KB, patch)
2009-07-03 02:22 PST, Maxime Simon
eric: review-
Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific files for WebCore/platform/image-decoders/. (5.43 KB, patch)
2009-07-07 01:34 PST, Maxime Simon
no flags Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific font files for WebCore/platform/graphics/. (27.53 KB, patch)
2009-07-07 03:17 PST, Maxime Simon
oliver: review-
Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/. (57.53 KB, patch)
2009-07-16 02:24 PST, Maxime Simon
no flags Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific font files for WebCore/platform/graphics/. (27.26 KB, patch)
2009-07-16 04:17 PST, Maxime Simon
no flags Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/. (57.54 KB, patch)
2009-07-16 06:27 PST, Maxime Simon
levin: review-
Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/. (57.66 KB, patch)
2009-07-17 03:12 PST, Maxime Simon
no flags Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific font files for WebCore/platform/graphics/. (27.26 KB, patch)
2009-07-17 03:25 PST, Maxime Simon
eric: review-
Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/. (57.63 KB, patch)
2009-07-17 05:09 PST, Maxime Simon
no flags Review Patch | Details | Formatted Diff | Diff
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/. (57.71 KB, patch)
2009-07-19 05:54 PST, Maxime Simon
eric: review-
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-07-02 23:59:10 PST
As the previous bug contains too much patches,
( https://bugs.webkit.org/show_bug.cgi?id=26620 )
I fill a new bug with only patches related to WebCore.
Patches about JavaScriptCore and other modifications have already been r+.
I will fill another bug for the WebCoreSupport ( in WebKit ),
and maybe for WebCore/platform/haiku/
------- Comment #1 From 2009-07-03 00:03:34 PST -------
Created an attachment (id=32219) [details]
Patch to add Haiku-specific files for WebCore/platform/image-decoders/.
------- Comment #2 From 2009-07-03 00:04:35 PST -------
Created an attachment (id=32220) [details]
Patch to add Haiku-specific files for WebCore/platform/text/.
------- Comment #3 From 2009-07-03 00:13:36 PST -------
Created an attachment (id=32221) [details]
Patch to add Haiku-specific files for WebCore/bindings/js/.
------- Comment #4 From 2009-07-03 00:21:28 PST -------
Created an attachment (id=32222) [details]
Patch to add Haiku-specific files for WebCore/editing/.
------- Comment #5 From 2009-07-03 00:35:16 PST -------
Created an attachment (id=32223) [details]
Patch to add Haiku-specific files for WebCore/page/.
------- Comment #6 From 2009-07-03 01:54:22 PST -------
Created an attachment (id=32227) [details]
Patch to add Haiku-specific font files for WebCore/platform/graphics/.
------- Comment #7 From 2009-07-03 02:22:30 PST -------
Created an attachment (id=32228) [details]
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/.
------- Comment #8 From 2009-07-07 00:23:03 PST -------
(From update of attachment 32222 [details])
Looks OK.
------- Comment #9 From 2009-07-07 00:23:47 PST -------
(From update of attachment 32221 [details])
OK.  Seems kinda silly that this code can't be shared between more ports?
------- Comment #10 From 2009-07-07 00:25:08 PST -------
(From update of attachment 32223 [details])
Sad that more of this code can't be shared between ports.
------- Comment #11 From 2009-07-07 00:26:30 PST -------
(From update of attachment 32220 [details])
Does this copy the data?
string.SetTo(utf8().data());

If it doesn't then your WebKit will crash all the time as StringImpls get released before the corresponding BStrings do...

r+ assuming this won't crash. :)
------- Comment #12 From 2009-07-07 00:27:56 PST -------
(From update of attachment 32219 [details])
This cast seems unecessary:
+    const void* bytes = (const void*)m_bytes.data();

STyle:
+int RGBA32Buffer::width() const {
+    return m_size.width();
+}
+
+int RGBA32Buffer::height() const {
+    return m_size.height();
+}
------- Comment #13 From 2009-07-07 00:33:22 PST -------
(From update of attachment 32227 [details])
Why the prefix "pc"?
65     const BFont* pcDefaultFont = be_plain_font;

Are you sure you meant to levae this in?
 72     printf("FontCache::getLastResortFallbackFont -> %s\n", family);

Spacing:
 74     BView* view = static_cast<BView*> (graphicsContext->platformContext());

Style:
 50         FontPlatformData(const FontPlatformData& f);
 48         FontPlatformData(const FontDescription&, BFont* font);

count_font_families() sounds expensive. :)
 71     for(int i = 0; i < count_font_families(); i++) {

style:
 73         if(!BString(familyName.string()).Compare(family)){

MOre style:
 90             if ( get_font_style(family, i, &style, &flags) == B_OK) {
 91                 if((bItalic && bBold) &&  (!strcmp( style, "BoldItalic")
 92                         || !strcmp(style, "Bold Oblique"))) {


You could also convert "style" to be a String and then style == "Bold Oblique" would read a bit better.

Style:
53         } else {
 54             character = characterBuffer[i];
 55         }

Doesn't ICU, and other parts of WebCore already have code for dealign with this:
         if(isUtf16) {
 50             UChar lead = characterBuffer[i * 2];
 51             UChar trail = characterBuffer[i * 2 + 1];
 52             character = U16_GET_SUPPLEMENTARY(lead, trail);
 53         } else {
 54             character = characterBuffer[i];
 55         } 

Style:
 46     BFont *font = m_platformData.font();
------- Comment #14 From 2009-07-07 00:33:48 PST -------
(From update of attachment 32228 [details])
Too large for me to review tonight.
------- Comment #15 From 2009-07-07 01:01:28 PST -------
(In reply to comment #11)
> (From update of attachment 32220 [details] [details])
> Does this copy the data?
> string.SetTo(utf8().data());
> 
> If it doesn't then your WebKit will crash all the time as StringImpls get
> released before the corresponding BStrings do...
> 
> r+ assuming this won't crash. :)

SetTo() copy the data, so it may not crash.
------- Comment #16 From 2009-07-07 01:34:59 PST -------
Created an attachment (id=32364) [details]
Patch to add Haiku-specific files for WebCore/platform/image-decoders/.
------- Comment #17 From 2009-07-07 02:46:55 PST -------
(In reply to comment #13)
> (From update of attachment 32227 [details] [details])
> Why the prefix "pc"?
> 65     const BFont* pcDefaultFont = be_plain_font;

Good question. :)

> count_font_families() sounds expensive. :)
>  71     for(int i = 0; i < count_font_families(); i++) {

count_font_families() sounds indeed a bit expensive,
but it's a C function from the Haiku API.

> Doesn't ICU, and other parts of WebCore already have code for dealign with
> this:
>          if(isUtf16) {
>  50             UChar lead = characterBuffer[i * 2];
>  51             UChar trail = characterBuffer[i * 2 + 1];
>  52             character = U16_GET_SUPPLEMENTARY(lead, trail);
>  53         } else {
>  54             character = characterBuffer[i];
>  55         } 

I know that the wx port handles this as it,
and the gtk port doesn't support Unicode supplementary characters...
------- Comment #18 From 2009-07-07 03:17:18 PST -------
Created an attachment (id=32371) [details]
Patch to add Haiku-specific font files for WebCore/platform/graphics/.
------- Comment #19 From 2009-07-15 11:52:55 PST -------
(From update of attachment 32228 [details])
Who owns the BView in the GraphicsContextPlatformPrivate?  I'm surprised that GCPlatformPrivate doesnt' need to retain it?

style:
npoints should be numberOfPoints or pointsLength or similar.

Style:
8     if (rects.size() > 1)
 229     {

spacing:
 235         m_data->view->StrokeRect(region.Frame(),B_MIXED_COLORS);

Naming:
 257 FloatRect GraphicsContext::roundToDevicePixels(const FloatRect& frect)

Naming:
 287 void GraphicsContext::strokeRect(const FloatRect& r, float width)
 298 void GraphicsContext::setLineCap(LineCap lc)

No need to wrap lines like this:
void GraphicsContext::addInnerRoundedRectClip(const IntRect& rect,
 451                                               int thickness)
 452 

I'm surprised your compiler can't convert from "return 0":
    return PassRefPtr<Icon>(0);

Why are you adding the empy ImageBufferData class?

I think you mean something like "if the image hasn't fully loaded":
 130     if (!image) // If it's too early we won't have an image yet.

But I'm still confused as to why we'd be calling pattern drawing routines with an image w/o a first frame.

I think there are functions to do this for you:
 140     ctxt->clip(IntRect(dstRect.x(), dstRect.y(), dstRect.width(), dstRect.height()));

IntRect(dstRect) probably does what you want.  Although you might want enclosingIntRect() there.

c++ style casts please:
 57     const unsigned char* uContents = (const unsigned char*) data.data();


I'm surprised:
 50 ImageDecoder* createDecoder(const Vector<char>& data)

isn't shared between teh ports?

Spacing:
 45     return BRect(BPoint(x(), y()), BSize(width(),height()));

It's better to break patches down by size instead of directory.

r- for all the nits above.
------- Comment #20 From 2009-07-16 01:03:54 PST -------
(From update of attachment 32371 [details])

> +// Fix temp routine to check the conversion works, please move it ASAP	
> +int charUnicodeToUtf8(unsigned short glyph, char* out)
> +{
> +    int i = 0;
> +
> +    if (glyph < 0x0080)
> +        out[i++] = (char)glyph;
C++ style casts please :D

> +    else if (glyph < 0x0800) { //two bytes
> +        out[i++] = 0xc0 | (( glyph ) >> 6 );
> +        out[i++] = 0x80 | (( glyph ) & 0x3F );
> +    } else if (glyph > 0x0800) { //3 bytes
> +        out[i++] = 0xe0 | (( glyph ) >> 12 );
> +        out[i++] = 0x80 | ( (( glyph ) >> 6 ) & 0x3F );
> +        out[i++] = 0x80 | (( glyph ) & 0x3F);
> +    }
Should have excess braces, eg.
out[i++] = 0x80 | ( (( glyph ) >> 6 ) & 0x3F ); => out[i++] = 0x80 | ((glyph >> 6 ) & 0x3F );
Shouldn't have spaces around the content of parenthesis, eg.
out[i++] = 0x80 | ( (( glyph ) >> 6 ) & 0x3F ); => out[i++] = 0x80 | ((glyph >> 6) & 0x3F);


> +FontPlatformData::FontPlatformData(const FontDescription& fontDescription, const AtomicString& familyName)
> +    : m_font(0)
> +{
> +    bool bFound = false;
> +    bool bItalic = fontDescription.italic();
> +    bool bBold = fontDescription.weight() == FontWeightBold;
> +    font_family family;
> +
> +    for (int i = 0; i < count_font_families(); i++) {
> +        get_font_family(i, &family, 0);
> +        if (!BString(familyName.string()).Compare(family)) {
> +            m_font = new BFont();
> +            m_font->SetFamilyAndStyle(family, 0);
> +            m_font->SetSize(fontDescription.computedSize());
> +
> +            bFound = true;
> +            break;
> +        }
> +    }
> +
> +    if (bFound) {
> +        int32 numStyles = count_font_styles(family);
> +        font_style fontStyle;
> +        uint32 flags;
> +        bFound = false;
> +
> +        for (int i = 0; i < numStyles; i++) {
> +            if (get_font_style(family, i, &fontStyle, &flags) == B_OK) {
> +                String style(fontStyle);
> +                if ((bItalic && bBold)
> +                    && (style == "BoldItalic" || style == "Bold Oblique")) {
There's no reason for this line break, we don't have an 80-cloumn limit in our style guidelines :D

> +                     bFound = true;
> +                     break;
> +                 }
> +                if ((bItalic && !bBold)
> +                    && (style == "Italic" || style == "Oblique")) {
ditto

...
> +                 }
> +                if ((!bItalic && !bBold)
> +                    && (style == "Roma"
> +                        || style == "Book" || style == "Condensed"
> +                        || style == "Regular" || style == "Medium")) {
You may want multiple lines here for clarity, but 4 lines seems excessive
------- Comment #21 From 2009-07-16 01:55:44 PST -------
> Why are you adding the empy ImageBufferData class?

It seems that all ports add it. And it's required because ImageBuffer.h
include it. :)

> I'm surprised:
>  50 ImageDecoder* createDecoder(const Vector<char>& data)
> 
> isn't shared between teh ports?

No it isn't. Every port defines its own.
------- Comment #22 From 2009-07-16 02:24:47 PST -------
Created an attachment (id=32846) [details]
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/.
------- Comment #23 From 2009-07-16 04:17:33 PST -------
Created an attachment (id=32856) [details]
Patch to add Haiku-specific font files for WebCore/platform/graphics/.
------- Comment #24 From 2009-07-16 06:27:14 PST -------
Created an attachment (id=32867) [details]
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/.
------- Comment #25 From 2009-07-16 11:53:06 PST -------
(From update of attachment 32867 [details])
You may want to consider using the (alpha version) lint tool for WebKit.

I downloaded this patch and ran on your files in this patch like this:
   python WebKitTools/Scripts/modules/cpplint.py WebCore/platform/graphics/haiku/*.cpp

It found these errors:
WebCore/platform/graphics/haiku/FloatRectHaiku.cpp:44:  Missing space after ,  [whitespace/comma] [3]
WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:51:  This { should be at the end of the previous line  [whitespace/braces] [4]
WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:229:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:495:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:495:  Missing space before {  [whitespace/braces] [5]
WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:511:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:516:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp:541:  Missing space after ,  [whitespace/comma] [3]
WebCore/platform/graphics/haiku/ImageHaiku.cpp:109:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/graphics/haiku/ImageHaiku.cpp:127:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/graphics/haiku/ImageSourceHaiku.cpp:78:  Tests for true/false, zero/non-zero, etc. should be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/haiku/ImageSourceHaiku.cpp:91:  Extra space before )  [whitespace/parens] [2]
WebCore/platform/graphics/haiku/ImageSourceHaiku.cpp:124:  Missing space before ( in if(  [whitespace/parens] [5]
WebCore/platform/graphics/haiku/IntRectHaiku.cpp:45:  Missing space after ,  [whitespace/comma] [3]

When glancing through the code, I also noticed the following issues.

In WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp

> 56 BView* view;
"Prefix C++ data members with "m_"."So it should be m_view 

> 59 GraphicsContextPlatformPrivate::GraphicsContextPlatformPrivate(BView* p_view)
p_view is not the correct format for a variable.

> 210     // creatin the region.
typo: creatin

> 318 void GraphicsContext::setLineJoin(LineJoin lj)
Use full words, except in the rare case where an abbreviation would be more canonical and easier to understand.
s/lj/lineJoin/


In WebCore/platform/graphics/haiku/ImageBufferData.h
#include "OwnPtr.h" should be #include <wtf/OwnPtr.h>


In WebCore/platform/graphics/haiku/ImageSourceHaiku.cpp
> 65     if (uContents[0] == 0x89 &&
> 66         uContents[1] == 0x50 &&
Boolean expressions at the same nesting level that span multiple lines should have their operators on the left side of the line instead of the right side.

So it should look like this:
     if (uContents[0] == 0x89
         && uContents[1] == 0x50

> 72     // if (uContents[0] == 0xFF &&
Commented out code isn't checked in.  If you want to note the need to address JPEG, you could put a 
  // FIXME: Implement JPEG support.
comment here.
------- Comment #26 From 2009-07-16 22:46:27 PST -------
(From update of attachment 32220 [details])
Committing to http://svn.webkit.org/repository/webkit/trunk ...
    M    WebCore/ChangeLog
    A    WebCore/platform/text/haiku/StringHaiku.cpp
    A    WebCore/platform/text/haiku/TextBreakIteratorInternalICUHaiku.cpp
Committed r46008
    M    WebCore/ChangeLog
    A    WebCore/platform/text/haiku/TextBreakIteratorInternalICUHaiku.cpp
    A    WebCore/platform/text/haiku/StringHaiku.cpp
r46008 = 8630eaa003c63558969f14b36e91f5e32d8a506e (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46008
------- Comment #27 From 2009-07-16 22:46:43 PST -------
(From update of attachment 32221 [details])
Committing to http://svn.webkit.org/repository/webkit/trunk ...
    M    WebCore/ChangeLog
    A    WebCore/bindings/js/ScriptControllerHaiku.cpp
Committed r46009
    M    WebCore/ChangeLog
    A    WebCore/bindings/js/ScriptControllerHaiku.cpp
r46009 = a884a839568b38c02917163be46d2b7046d7d82e (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46009
------- Comment #28 From 2009-07-16 22:46:58 PST -------
(From update of attachment 32222 [details])
Committing to http://svn.webkit.org/repository/webkit/trunk ...
    M    WebCore/ChangeLog
    A    WebCore/editing/haiku/EditorHaiku.cpp
Committed r46010
    A    WebCore/editing/haiku/EditorHaiku.cpp
    M    WebCore/ChangeLog
r46010 = c2dae2a1c3654e83314d76f79f67fb856f927061 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46010
------- Comment #29 From 2009-07-16 22:47:14 PST -------
(From update of attachment 32223 [details])
Committing to http://svn.webkit.org/repository/webkit/trunk ...
    M    WebCore/ChangeLog
    A    WebCore/page/haiku/DragControllerHaiku.cpp
    A    WebCore/page/haiku/EventHandlerHaiku.cpp
    A    WebCore/page/haiku/FrameHaiku.cpp
Committed r46011
    M    WebCore/ChangeLog
    A    WebCore/page/haiku/EventHandlerHaiku.cpp
    A    WebCore/page/haiku/DragControllerHaiku.cpp
    A    WebCore/page/haiku/FrameHaiku.cpp
r46011 = c459ecc2d08dd92a3d1d9c259067fad664cda3b1 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46011
------- Comment #30 From 2009-07-16 22:47:30 PST -------
(From update of attachment 32364 [details])
Committing to http://svn.webkit.org/repository/webkit/trunk ...
    M    WebCore/ChangeLog
    A    WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp
Committed r46012
    M    WebCore/ChangeLog
    A    WebCore/platform/image-decoders/haiku/ImageDecoderHaiku.cpp
r46012 = 0d02984b5f636d37396785e694037c7a96e6f977 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46012
------- Comment #31 From 2009-07-16 22:47:34 PST -------
All reviewed patches landed, closing.
------- Comment #32 From 2009-07-16 22:49:32 PST -------
Bugzilla-tool closed this bug, but it should stay open.
------- Comment #33 From 2009-07-17 01:29:35 PST -------
(In reply to comment #25)
> (From update of attachment 32867 [details] [details])
> You may want to consider using the (alpha version) lint tool for WebKit.
> 
> I downloaded this patch and ran on your files in this patch like this:
>    python WebKitTools/Scripts/modules/cpplint.py
> WebCore/platform/graphics/haiku/*.cpp

Thanks a lot for the tip.
This tool should really ease the work for reviewers.
And in the same time it will avoid people like me to fill too much patches. :)
------- Comment #34 From 2009-07-17 03:12:58 PST -------
Created an attachment (id=32923) [details]
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/.
------- Comment #35 From 2009-07-17 03:25:50 PST -------
Created an attachment (id=32924) [details]
Patch to add Haiku-specific font files for WebCore/platform/graphics/.

I made some style cleanup (thanks to the cpplint.py tool).
------- Comment #36 From 2009-07-17 05:09:59 PST -------
Created an attachment (id=32931) [details]
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/.

Sorry, in the previous patch for the purpose of style cleanup I renamed a variable, but it makes the compilation to fail.
------- Comment #37 From 2009-07-19 05:54:51 PST -------
Created an attachment (id=33042) [details]
Patch to add Haiku-specific drawing files for WebCore/platform/graphics/.

The latest modifications on the patch "Modifications on WebKit source code to allow Haiku port." ( https://bugs.webkit.org/show_bug.cgi?id=26620#c59 ) require that I add "#include <stdio.h>" in 2 files of WebCore/platform/graphics/haiku/.
------- Comment #38 From 2009-08-07 11:05:03 PST -------
(From update of attachment 32924 [details])
What is this and why?
 // Fix temp routine to check the conversion works, please move it ASAP    
 44 int charUnicodeToUtf8(unsigned short glyph, char* out)

It's not mentioned in the ChangeLog.  I needs a more decriptive name if it's just a hack.

convertUTF16ToUTF8RenderingHACK or something.

We don't use argument names in headers when they don't add value:
47         FontPlatformData(const FontDescription& fontDescription, const AtomicString& family);
 48         FontPlatformData(const FontDescription& fontDescription, BFont* font);
 49         FontPlatformData(float size, bool bold, bool italic);
 50         FontPlatformData(const FontPlatformData& fontPlatformData);
"family" can stay.  but the fontDescription at least shoudl go.

No hungarian notation here ;)
 66     bool bFound = false;
 67     bool bItalic = fontDescription.italic();
 68     bool bBold = fontDescription.weight() == FontWeightBold;
We ain't no microsoft...

Why isn't this an OwnPtr<BFont>:
 63         BFont* m_font;

Your leaking BFonts.

Why is this correct?
        if (isUtf16) {
 50             UChar lead = characterBuffer[i * 2];
 51             UChar trail = characterBuffer[i * 2 + 1];
 52             character = U16_GET_SUPPLEMENTARY(lead, trail);
 53         } else

Not all characters will be composed characters.

Use the height of the X, can't you ask the font for the height of a glyph at a certain size?
 52         m_xHeight = font->Size(); // Not sure about this

This information should be on the BFont:
 53         m_unitsPerEm = 1; // FIXME!

You'd have more luck posting this in smaller pices.  Some of this was just fine, but some of the font code is wrong here too.  If you broke it out, I coudl more easily approve the OK parts.
------- Comment #39 From 2009-08-07 11:13:24 PST -------
(From update of attachment 33042 [details])
Better if m_data could be an OwnPtr for haiku:
 79     delete m_data;

Why?
setFocusRingColorChangeFunction

Spacing:
 230         BRegion    region;

It's better to have conversion funtions outside of these setters:
302     cap_mode mode = B_BUTT_CAP;
 303     switch (lineCap) {
 304     case RoundCap:

I think CG follows that model.

So remove it:
61         // FIXME: Not sure why be_app is checked
 62         if (be_app)
 63             delete m_frame;

Please break this into smaller patches.  AGain, parts of hsi woudl be super-easy to approve.  Other parts no.  If you posted a patch with just the empty implementatins that would be an easy r+, the harder stuff can e done in separate patches.
------- Comment #40 From 2009-08-07 12:00:41 PST -------
> What is this and why?
>  // Fix temp routine to check the conversion works, please move it ASAP    
>  44 int charUnicodeToUtf8(unsigned short glyph, char* out)
> 
> It's not mentioned in the ChangeLog.  I needs a more decriptive name if it's
> just a hack.

I'm not sure about this function. I would like Ryan to explain it. (If he wrote it.)

> convertUTF16ToUTF8RenderingHACK or something.

This may be a possibility.

> No hungarian notation here ;)
>  66     bool bFound = false;
>  67     bool bItalic = fontDescription.italic();
>  68     bool bBold = fontDescription.weight() == FontWeightBold;
> We ain't no microsoft...

Erk, it comes from the Syllable port. Ryan tried to avoid them as much as possible but there may have some remains.

> Use the height of the X, can't you ask the font for the height of a glyph at a
> certain size?
>  52         m_xHeight = font->Size(); // Not sure about this

This was an issue I avoided when improving the port.
But I should update this patch.

> This information should be on the BFont:
>  53         m_unitsPerEm = 1; // FIXME!

The BFont (from Haiku) don't handle this.

> You'd have more luck posting this in smaller pices.  Some of this was just
> fine, but some of the font code is wrong here too.  If you broke it out, I
> coudl more easily approve the OK parts.

I think I will fill a new bug with multiple patches from this one.
------- Comment #41 From 2009-08-15 03:12:43 PST -------
No need to keep this bug opened.