Bug 50820 – [GTK] Port to GtkStyleContext

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

GtkStyle is now deprecated in gtk+ 3, GtkStyleContext is the replacement that allows us painting the widgets without using real widgets.

[Carlos Garcia Campos]

Created attachment 76209 [details]
Patch to port gtk3 code to GtkStyleContext

[Carlos Garcia Campos]

Created attachment 76518 [details]
Fork RenderThemeGtk and ScrollbarThemeGtk

I'm splitting the patch as requested by Martin to make it easier for reviewers, please don't expect individual patches to work (not even build), they are only for review. The idea is to merge this as a single patch to make sure it builds and works and it won't break any future bisection. I'm also commenting here the changes made in every file.

This first patch forksRenderThemeGtk and ScrollbarThemeGtk moving the code that is specific to gtk2 from RenderThemeGtk and ScrollbarThemeGtk to RenderThemeGtk2 and ScrollbarThemeGtk2 leaving the common code in the original files.

RenderThemeGtk: paintStockIcon has been splitted into getStockIcon, that has different implementations in gtk2 and gtk3, and paintStockIcon() which is common. Some methods that were static and private to RenderThemeGtk are now static methods of the class so that they can be used in RenderThemeGtk2 and RenderThemeGtk3. Some public methods returning or using gtk enums are using integers now, because we don't have forward declarations for enums. 

RenderThemeGtk2: m_themeParts and m_themePartsHaveRGBAColormap have been removed. Mozilla code uses a global static GtkThemeParts struct, hwoever, we are allocating a new structure everytime a new RenderThemeGtk instance is created, but that new struct is not used because moz_gtk_use_theme_parts() is only called the first time. So we can allocate the GtkThemeParts only once the first time, move the HaveRGBAColormap to the struct and add an accessor to gtkdrawing to get the global struct. If using heap allocation is a problem we can use a stack allocated struct in gtkdrawing. getIndicatorMetrics() was basicly a wrapper for moz_gtk_checkbox_get_metrics() and only used by WidgetRenderingContextGtk2 that already knows about mozilla code, so it has been removed and moz_gtk_checkbox_get_metrics() is used directly when needed. The rest of the code in this file is copy-pasted from RenderThemeGtk.

ScrollbarThemeGtk: It uses a private static HashSet to store the registered scrollbars, since both ScrollbarThemeGtk2 and ScrollbarThemeGtk3 need to use it, I've added a private accessor ScrollbarThemeGtk::getScrollbars(). Boolean attributes (m_troughUnderSteppers, m_hasForwardButtonStartPart, m_hasBackButtonEndPart) are now gboolean instead of bool, because they are used directly (well, the mem dir actually) as arguments of gtk_style_context_get_style(), in ScrollbarThemeGtk3, that expects gboolean (an integer) and using bool causes mem overflows. 

WidgetRenderingContextGtk2: It uses the new accessor moz_gtk_get_theme_parts() to use the global GtkThemeParts struct and moz_gtk_checkbox_get_metrics() instead of RenderThemeGtk::getIndicatorMetrics().

gtk2drawing: moz_gtk_get_theme_parts() has been added.

gtkdrawing.h: non gtk2 stuff has been removed and haveRGBAColormap was moved from RenderThemeGtk to the GtkThemeParts struct.

ScrollbarThemeGtk2: All the code is copy-pasted from ScrollbarThemeGtk, just changed a bit to use the new accessor getScrollbars().

Removed files are not included in the patch: WidgetRenderingContextGtk3.cpp and gtk3drawing.c

[Carlos Garcia Campos]

Created attachment 76519 [details]
Port gtk2 code to GtkStyleContext

This second patch is the real port to GtkStyleContext. All gtk3 code is in RenderThemeGtk3 and ScrollbarThemeGtk3, mozilla code is not used anymore in gtk3 which also means we don't need fake widgets either \o/

RenderThemeGtk3: gtk3 provides GtkStyleContext that don't need a real widget to draw, they only need widget paths (GtkWidgetPath). Instead of creating a big struct to cache every style context I'm using a global static HashMap that dynamically stores the style context for every widget type, using the GType (which is an integer) of the widget to render as the hash key. A private static method getStyleContext() allows getting the style context for any widget type. GtkStyleContext has a signal similar to the old style-set of GtkStyle to know when a new style has been set (because the theme has changed, for example), but it only works for style contexts associated to a widget. Now we connect to gtk-theme-name and gtk-color-scheme (properties of GtkSettings) notify signal to invalidate our style contexts and call platformColorsDidChange(). The rest of the file is the specific code needed to render the widgets using GtkStyleContext, it's based on gtk2 implementations and current gtk3 code.

ScrollbarThemeGtk3: The style context for the scrollbars is stored in the global HashMap in RenderThemeGtk3, so we use an accessor to get it in the constructor to cache it. When the theme changes, all the style contexts are invalidated in RenderThemeGtk3, and when a GtkStyleContext is invalidated the changed signal is emitted, so here we only need to connect to the changed signal for the scrollbar context to react to theme changes. The rest of the file is the specific code needed to render the scrollbars using GtkStyleContext.

[Carlos Garcia Campos]

(In reply to comment #2)
> Created an attachment (id=76518) [details]
> RenderThemeGtk2: m_themeParts and m_themePartsHaveRGBAColormap have been removed. Mozilla code uses a global static GtkThemeParts struct, hwoever, we are allocating a new structure everytime a new RenderThemeGtk instance is created, but that new struct is not used because moz_gtk_use_theme_parts() is only called the first time. So we can allocate the GtkThemeParts only once the first time, move the HaveRGBAColormap to the struct and add an accessor to gtkdrawing to get the global struct. If using heap allocation is a problem we can use a stack allocated struct in gtkdrawing. 

I've just noticed that I forgot to free the global GtkThemeParts struct in moz_gtk_shutdown(), so I can add the free there, or just use a stack allocated struct.

[Carlos Garcia Campos]

Created attachment 76746 [details]
Updated patch rebased to current git master

Patch rebased to current git master and two more changes:

 - Allocate theme parts struct in the stackto fix the memory leak I mentioned in previous comment
 - Implement juntion sides for stepper buttons in gtk3 scrollbars code

[Martin Robinson]

Comment on attachment 76746 [details]
Updated patch rebased to current git master

Removing review flag, because we're going to land this in pieces.