Summary: | Lessen Include Path Build Fragility (FontPlatformData.h) | ||
---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> |
Component: | WebCore Misc. | Assignee: | Brent Fulgham <bfulgham> |
Status: | RESOLVED FIXED | ||
Severity: | Critical | CC: | aroben, webkit.review.bot |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | PC | ||
OS: | Windows 7 | ||
Attachments: |
Description
Brent Fulgham
2010-03-16 13:57:55 PDT
Created attachment 50837 [details]
Patch to disambiguate FontPlatformData.h includes.
Comment on attachment 50837 [details]
Patch to disambiguate FontPlatformData.h includes.
My biggest complaint with this approach is that the next time someone adds #include "FontPlatformData.h" to a file, they will break Apple's Windows build. We need to find some way to avoid that.
Created attachment 50975 [details]
Address FontPlatformData.h include confusion by moving WinCairo declarations into platform/graphics/cairo version of file.
Move WinCairo implementation to platform/graphics/cairo; move cg-specific FontPlatformData.h. Remove cairo-specific stuff from the cg/FontPlatformData.h file.
Attachment 50975 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp:135: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp:136: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp:137: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp:138: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp:139: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
WARNING: Could not read file. Skipping: "WebCore/platform/graphics/win/FontPlatformData.h"
WebCore/platform/graphics/cairo/FontPlatformData.h:41: Alphabetical sorting problem. [build/include_order] [4]
WebCore/platform/graphics/win/RefCountedHFONT.h:23: #ifndef header guard has wrong style, please use: RefCountedHFONT_h [build/header_guard] [5]
WebCore/platform/graphics/win/RefCountedHFONT.h:36: More than one command on the same line in if [whitespace/parens] [4]
WebCore/platform/graphics/cg/FontPlatformData.h:24: #ifndef header guard has wrong style, please use: FontPlatformData_h [build/header_guard] [5]
WebCore/platform/graphics/cg/FontPlatformData.h:31: Alphabetical sorting problem. [build/include_order] [4]
WebCore/platform/graphics/cg/FontPlatformData.h:76: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
WebCore/platform/graphics/cg/FontPlatformData.h:77: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
WebCore/platform/graphics/cg/FontPlatformData.h:78: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
WebCore/platform/graphics/cg/FontPlatformData.h:79: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4]
Total errors found: 14 in 8 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 51044 [details]
Same as last revision, but with check-webkit-style fixes.
Minor whitespace changes to conform to check-webkit-style.
Created attachment 51045 [details] Uploaded wrong version of diff. This is the correct file. (I.e., attachment 50975 [details] with check-webkit-style fixes.) Corrects check-webkit-style warnings. Attachment 51045 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WARNING: Could not read file. Skipping: "WebCore/platform/graphics/win/FontPlatformData.h"
WebCore/platform/graphics/cairo/FontPlatformData.h:41: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 1 in 8 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 51045 [details] Uploaded wrong version of diff. This is the correct file. (I.e., attachment 50975 [details] with check-webkit-style fixes.) > - bool isHashTableDeletedValue() const { > + bool isHashTableDeletedValue() const > + { > #if defined(USE_FREETYPE) > return m_pattern == hashTableDeletedFontValue(); > #elif defined(USE_PANGO) > return m_font == hashTableDeletedFontValue(); > +#elif PLATFORM(WIN) > + return m_font.isHashTableDeletedValue(); > #endif > }; > > + > #ifndef NDEBUG > String description() const; > #endif Too much whitespace in there. (And the semicolon after the closing brace isn't needed.) > @@ -122,6 +161,13 @@ public: > > PangoContext* m_context; > PangoFont* m_font; > +#elif PLATFORM(WIN) > +private: > + void platformDataInit(HFONT font, float size, HDC hdc, WCHAR* faceName); You could remove the "font" and "hdc" parameter names here. > Index: WebCore/platform/graphics/cg/FontPlatformData.h Having this Windows-specific file in the cg/ directory seems a little confusing. Mac uses CG, too (and many files in the platform/graphics/cg directory), for example. But I guess it's OK for now. > +++ WebCore/platform/graphics/win/RefCountedHFONT.h (revision 0) > @@ -0,0 +1,59 @@ > +/* > + * This file is part of the internal font implementation. It should not be included by anyone other than > + * FontCairo.cpp, FontMac.cpp, FontWin.cpp and Font.cpp. I don't think this comment is needed anymore. r=me Landed in http://trac.webkit.org/changeset/56192. |