[webkit-reviews] review denied: [Bug 27734] WINCE PORT: font related changes : [Attachment 34297] use shared buffer in custom font data

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Joe Mason
<joe.mason at torchmobile.com>'s request for review:
Bug 27734: WINCE PORT: font related changes
https://bugs.webkit.org/show_bug.cgi?id=27734

Attachment 34297: use shared buffer in custom font data
https://bugs.webkit.org/attachment.cgi?id=34297&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
What does this mean?
STORE_FONT_CUSTOM_PLATFORM_DATA

Not sufficient explanation:
 74 // Need to use #define instead of typedef due to conflict

Not a valid variable name:
 353	 const sfntHeader* sfnt = reinterpret_cast<const
sfntHeader*>(fontData->data());

Seems like this shoudl be a static inline function:
69     size_t nameTableSize = ((offsetof(nameTable, nameRecords) +
nameRecordCount * sizeof(nameRecord) + fontName.length() * sizeof(UChar)) & ~3)
+ 4;

This function needs more commentary:
 347 bool renameFont(SharedBuffer* fontData, const String& fontName)

What is it trying to do?  re-write the in-memory font representation?

Put this in a separate static inline, please:
380	// Write the new 'name' table after the original font data.
 381	 nameTable* name = reinterpret_cast<nameTable*>(data +
originalDataSize);
 382	 name->format = 0;
 383	 name->count = nameRecordCount;
 384	 name->stringOffset = offsetof(nameTable, nameRecords) +
nameRecordCount * sizeof(nameRecord);
 385	 for (unsigned i = 0; i < nameRecordCount; ++i) {
 386	     name->nameRecords[i].platformID = 3;
 387	     name->nameRecords[i].encodingID = 1;
 388	     name->nameRecords[i].languageID = 0x0409;
 389	     name->nameRecords[i].offset = 0;
 390	     name->nameRecords[i].length = fontName.length() * sizeof(UChar);
 391	 }
 392 

This code is not legible.  Please make it more understandable to casual
readers.