Long term we would like to allow RenderThemeChromiumLinux to run on both windows and linux. However the font lookup should remain platform specific. We need to refactor font code out of RenderThemeChromiumSkia into platform specific methods.
Created attachment 173669 [details] Fix
Attachment 173669 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/rendering/RenderThemeChromiumFontProviderWin.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/rendering/RenderThemeChromiumFontProviderWin.cpp:50: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/rendering/RenderThemeChromiumFontProviderWin.cpp:56: Use 0 or null instead of NULL (even in *comments*). [readability/null] [4] Source/WebCore/rendering/RenderThemeChromiumFontProviderWin.cpp:75: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/rendering/RenderThemeChromiumFontProviderWin.cpp:101: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/rendering/RenderThemeChromiumFontProviderWin.cpp:102: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/rendering/RenderThemeChromiumFontProviderWin.cpp:147: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/rendering/RenderThemeChromiumFontProvider.cpp:34: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Total errors found: 8 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 173669 [details] Fix R=me, but you need to fix up the style warnings.
Created attachment 173739 [details] Fixes style issues
Comment on attachment 173739 [details] Fixes style issues View in context: https://bugs.webkit.org/attachment.cgi?id=173739&action=review > Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.cpp:51 > +void RenderThemeChromiumFontProvider::getFont(int propId, FontDescription& fontDescription) Nit: It's not that common to prefix a method with 'get' in WebKit. Something like initializeSystemFont might be more descriptive.
Created attachment 174007 [details] Patch for landing
Created attachment 174008 [details] Patch for landing
Comment on attachment 174008 [details] Patch for landing Clearing flags on attachment: 174008 Committed r134524: <http://trac.webkit.org/changeset/134524>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by bug 102177
It looks like this patch caused the following build error: third_party/WebKit/Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.cpp:37:24: error: declaration requires a global constructor [-Werror,-Wglobal-constructors] static FontDescription smallSystemFont; ^~~~~~~~~~~~~~~ third_party/WebKit/Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.cpp:37:24: error: declaration requires an exit-time destructor [-Werror,-Wexit-time-destructors] third_party/WebKit/Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.cpp:37:24: error: declaration requires a global destructor [-Werror,-Wglobal-constructors] third_party/WebKit/Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.cpp:38:24: error: declaration requires a global constructor [-Werror,-Wglobal-constructors] static FontDescription menuFont; ^~~~~~~~ third_party/WebKit/Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.cpp:38:24: error: declaration requires an exit-time destructor [-Werror,-Wexit-time-destructors] third_party/WebKit/Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.cpp:38:24: error: declaration requires a global destructor [-Werror,-Wglobal-constructors] third_party/WebKit/Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.cpp:39:24: error: declaration requires a global constructor [-Werror,-Wglobal-constructors] static FontDescription labelFont; ^~~~~~~~~ third_party/WebKit/Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.cpp:39:24: error: declaration requires an exit-time destructor [-Werror,-Wexit-time-destructors] third_party/WebKit/Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.cpp:39:24: error: declaration requires a global destructor [-Werror,-Wglobal-constructors] 9 errors generated. make: *** [out/Release/obj.target/webcore_rendering/third_party/WebKit/Source/WebCore/rendering/RenderThemeChromiumFontProviderLinux.o] Error 1 make: *** Waiting for unfinished jobs....
Created attachment 174165 [details] Patch
Comment on attachment 174165 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174165&action=review > Source/WebCore/rendering/RenderThemeChromiumFontProviderWin.cpp:47 > +static FontDescription smallSystemFont; > +static FontDescription menuFont; > +static FontDescription labelFont; These look like static initializers too, although MSVC won't complain about it. Hmm, maybe make these pointers and initialize them lazily in systemFont()?
Created attachment 174184 [details] Patch
I was worried about call order, so I went with DEFINE_STATIC_LOCAL.
Comment on attachment 174184 [details] Patch Clearing flags on attachment: 174184 Committed r134662: <http://trac.webkit.org/changeset/134662>