Chromium Linux: Switch to using Skia to render widgets. In order to sandbox the Chromium renderer on Linux we need to remove the X connection. GTK cannot render without an X connection so, for now, we render widgets ourselves. Previously use didn't anti-alias fonts in order to match Windows font rendering exactly. This was helpful when bootstrapping our layout tests. Now, however, we are ready to enable it.
Created attachment 28098 [details] patch
Comment on attachment 28098 [details] patch >diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog >+2009-02-27 Adam Langley <agl@chromium.org> ... >+ WARNING: NO TEST CASES ADDED OR CHANGED nit: please remove this WARNING line... and add a link to this bug. >+ * rendering/RenderThemeChromiumLinux.cpp: Added. >+ (WebCore::): >+ (WebCore::supportsFocus): >+ (WebCore::setSizeIfAuto): >+ (WebCore::defaultGUIFont): >+ (WebCore::theme): nit: when adding a new file, please delete all of the functions that prepare-ChangeLog added. >+++ b/WebCore/platform/chromium/ScrollbarThemeChromiumLinux.cpp ... > void ScrollbarThemeChromium::paintThumb(GraphicsContext* gc, Scrollbar* scrollbar, const IntRect& rect) > { ... >+ SkPaint paint; >+ if (hovered) { >+ paint.setARGB(255, 0xff, 0xff, 0xff); >+ } else { >+ paint.setARGB(255, 0xf4, 0xf2, 0xef); >+ } nit: indentation should be 4 spaces, and you should not use brackets when there is only a single line statement. please see: http://webkit.org/coding/coding-style.html >+++ b/WebCore/platform/graphics/chromium/FontPlatformDataLinux.cpp ... >+ paint->setAntiAlias(true); Making friends in high places? >+++ b/WebCore/rendering/RenderThemeChromiumLinux.cpp It might be better to do a "svn mv" so that this diff would just show your changes. >+Color RenderThemeChromiumLinux::platformActiveSelectionBackgroundColor() const >+{ >+ return Color(0x1e, 0x90, 0xff); >+} >+ >+Color RenderThemeChromiumLinux::platformInactiveSelectionBackgroundColor() const >+{ >+ return Color(200, 200, 200); >+} sometimes hex, sometimes decimal? >+void RenderThemeChromiumLinux::systemFont(int propId, Document* document, FontDescription& fontDescription) const >+{ >+ fontDescription.firstFamily().setFamily(defaultGUIFont(NULL)); >+ fontDescription.setSpecifiedSize(12); bad indentation >+++ b/WebCore/rendering/RenderThemeChromiumLinux.h >+ virtual void adjustButtonInnerStyle(RenderStyle* style) const; nit: please remove the parameter name since it doesn't add any information.
Created attachment 28188 [details] patch
Comment on attachment 28188 [details] patch >+++ b/WebCore/platform/chromium/ScrollbarThemeChromium.cpp ... >+ const int girth = 0; >+ // On Linux, we don't use buttons >+ return IntSize(0, 0); seems like it shouldn't be necessary to define girth given the early return, or does GCC complain without it being there? >+++ b/WebCore/platform/chromium/ScrollbarThemeChromiumLinux.cpp >+ paint.setARGB(255, 0xe3, 0xdd, 0xd8); ... >+ paint.setARGB(255, 0xc5, 0xba, 0xb0); the mixing of decimal and hex really seems odd to me... is that somehow conventional? (this hurts my brain) >+++ b/WebCore/rendering/RenderThemeChromiumLinux.cpp >+ paint.setARGB(255, 0xe9, 0xe9, 0xe9); ditto (and elsewhere in this file)
Created attachment 28194 [details] patch
Have addressed all comments. It seems that replying to the bugzilla emails doesn't actually do anything, so sorry you missed the reply to the first one - but I believe I addressed everything there too. Patch interdiff follows since bugzilla doesn't seem to supply them: diff --git a/WebCore/platform/chromium/ScrollbarThemeChromium.cpp b/WebCore/platform/chromium/ScrollbarThemeChromium.cpp index 3096785..426a078 100644 --- a/WebCore/platform/chromium/ScrollbarThemeChromium.cpp +++ b/WebCore/platform/chromium/ScrollbarThemeChromium.cpp @@ -167,6 +167,11 @@ bool ScrollbarThemeChromium::shouldCenterOnThumb(Scrollbar*, const PlatformMouse IntSize ScrollbarThemeChromium::buttonSize(Scrollbar* scrollbar) { +#if defined(__linux__) + // On Linux, we don't use buttons + return IntSize(0, 0); +#endif + // Our desired rect is essentially thickness by thickness. // Our actual rect will shrink to half the available space when we have < 2 @@ -175,20 +180,13 @@ IntSize ScrollbarThemeChromium::buttonSize(Scrollbar* scrollbar) int thickness = scrollbarThickness(scrollbar->controlSize()); -#if !defined(__linux__) // In layout test mode, we force the button "girth" (i.e., the length of // the button along the axis of the scrollbar) to be a fixed size. // FIXME: This is retarded! scrollbarThickness is already fixed in layout // test mode so that should be enough to result in repeatable results, but // preserving this hack avoids having to rebaseline pixel tests. const int kLayoutTestModeGirth = 17; - int girth = ChromiumBridge::layoutTestMode() ? kLayoutTestModeGirth : thickness; -#else - const int girth = 0; - // On Linux, we don't use buttons - return IntSize(0, 0); -#endif if (scrollbar->orientation() == HorizontalScrollbar) { int width = scrollbar->width() < 2 * girth ? scrollbar->width() / 2 : girth; diff --git a/WebCore/platform/chromium/ScrollbarThemeChromiumLinux.cpp b/WebCore/platform/chromium/ScrollbarThemeChromiumLinux.cpp index 6694946..11e2472 100644 --- a/WebCore/platform/chromium/ScrollbarThemeChromiumLinux.cpp +++ b/WebCore/platform/chromium/ScrollbarThemeChromiumLinux.cpp @@ -58,12 +58,12 @@ void ScrollbarThemeChromium::paintTrackPiece(GraphicsContext* gc, Scrollbar* scr SkCanvas* const canvas = gc->platformContext()->canvas(); SkPaint paint; - paint.setARGB(255, 0xe3, 0xdd, 0xd8); + paint.setARGB(0xff, 0xe3, 0xdd, 0xd8); SkRect skrect; skrect.set(rect.x(), rect.y(), right, bottom); canvas->drawRect(skrect, paint); - paint.setARGB(255, 0xc5, 0xba, 0xb0); + paint.setARGB(0xff, 0xc5, 0xba, 0xb0); canvas->drawLine(rect.x(), rect.y(), rect.x(), bottom, paint); canvas->drawLine(right, rect.y(), right, bottom, paint); canvas->drawLine(rect.x(), rect.y(), right, rect.y(), paint); @@ -88,9 +88,9 @@ void ScrollbarThemeChromium::paintThumb(GraphicsContext* gc, Scrollbar* scrollba SkPaint paint; if (hovered) - paint.setARGB(255, 0xff, 0xff, 0xff); + paint.setARGB(0xff, 0xff, 0xff, 0xff); else - paint.setARGB(255, 0xf4, 0xf2, 0xef); + paint.setARGB(0xff, 0xf4, 0xf2, 0xef); SkRect skrect; if (vertical) { @@ -101,9 +101,9 @@ void ScrollbarThemeChromium::paintThumb(GraphicsContext* gc, Scrollbar* scrollba canvas->drawRect(skrect, paint); if (hovered) { - paint.setARGB(255, 0xf4, 0xf2, 0xef); + paint.setARGB(0xff, 0xf4, 0xf2, 0xef); } else { - paint.setARGB(255, 0xea, 0xe5, 0xe0); + paint.setARGB(0xff, 0xea, 0xe5, 0xe0); } if (vertical) { skrect.set(midx + 1, rect.y(), right, bottom); @@ -112,14 +112,14 @@ void ScrollbarThemeChromium::paintThumb(GraphicsContext* gc, Scrollbar* scrollba } canvas->drawRect(skrect, paint); - paint.setARGB(255, 0x9d, 0x96, 0x8e); + paint.setARGB(0xff, 0x9d, 0x96, 0x8e); canvas->drawLine(rect.x(), rect.y(), rect.x(), bottom, paint); canvas->drawLine(right, rect.y(), right, bottom, paint); canvas->drawLine(rect.x(), rect.y(), right, rect.y(), paint); canvas->drawLine(rect.x(), bottom, right, bottom, paint); if (rect.height() > 10 && rect.width() > 10) { - paint.setARGB(255, 0x9d, 0x96, 0x8e); + paint.setARGB(0xff, 0x9d, 0x96, 0x8e); canvas->drawLine(midx - 1, midy, midx + 3, midy, paint); canvas->drawLine(midx - 1, midy - 3, midx + 3, midy - 3, paint); canvas->drawLine(midx - 1, midy + 3, midx + 3, midy + 3, paint); diff --git a/WebCore/rendering/RenderThemeChromiumLinux.cpp b/WebCore/rendering/RenderThemeChromiumLinux.cpp index 086552d..2a078ce 100644 --- a/WebCore/rendering/RenderThemeChromiumLinux.cpp +++ b/WebCore/rendering/RenderThemeChromiumLinux.cpp @@ -204,7 +204,7 @@ static void paintButtonLike(RenderTheme* theme, RenderObject* o, const RenderObj // If the button is too small, fallback to drawing a single, solid color if (rect.width() < 5 || rect.height() < 5) { - paint.setARGB(255, 0xe9, 0xe9, 0xe9); + paint.setARGB(0xff, 0xe9, 0xe9, 0xe9); skrect.set(rect.x(), rect.y(), right, bottom); canvas->drawRect(skrect, paint); return; @@ -217,15 +217,15 @@ static void paintButtonLike(RenderTheme* theme, RenderObject* o, const RenderObj canvas->drawLine(rect.x() + 1, bottom - 1, right - 1, bottom - 1, paint); canvas->drawLine(rect.x(), rect.y() + 1, rect.x(), bottom - 1, paint); - paint.setARGB(255, 0, 0, 0); + paint.setARGB(0xff, 0, 0, 0); SkPoint p[2]; const int lightEnd = theme->isPressed(o) ? 1 : 0; const int darkEnd = !lightEnd; p[lightEnd].set(SkIntToScalar(rect.x()), SkIntToScalar(rect.y())); p[darkEnd].set(SkIntToScalar(rect.x()), SkIntToScalar(bottom - 1)); SkColor colors[2]; - colors[0] = SkColorSetARGB(255, 0xf8, 0xf8, 0xf8); - colors[1] = SkColorSetARGB(255, 0xdd, 0xdd, 0xdd); + colors[0] = SkColorSetARGB(0xff, 0xf8, 0xf8, 0xf8); + colors[1] = SkColorSetARGB(0xff, 0xdd, 0xdd, 0xdd); SkShader* s = SkGradientShader::CreateLinear( p, colors, NULL, 2, SkShader::kClamp_TileMode, NULL); @@ -237,7 +237,7 @@ static void paintButtonLike(RenderTheme* theme, RenderObject* o, const RenderObj canvas->drawRect(skrect, paint); paint.setShader(NULL); - paint.setARGB(255, 0xce, 0xce, 0xce); + paint.setARGB(0xff, 0xce, 0xce, 0xce); canvas->drawPoint(rect.x() + 1, rect.y() + 1, paint); canvas->drawPoint(right - 2, rect.y() + 1, paint); canvas->drawPoint(rect.x() + 1, bottom - 2, paint); @@ -290,7 +290,7 @@ bool RenderThemeChromiumLinux::paintMenuList(RenderObject* o, const RenderObject paintButtonLike(this, o, i, rect); SkPaint paint; - paint.setARGB(255, 0, 0, 0); + paint.setARGB(0xff, 0, 0, 0); paint.setAntiAlias(true); paint.setStyle(SkPaint::kFill_Style);
Comment on attachment 28194 [details] patch LGTM
Created attachment 28206 [details] patch Merge with trunk: collided with Ojan.
Landed as http://trac.webkit.org/changeset/41416