[Webkit-unassigned] [Bug 156462] [GTK] Rework scrollbars theming code for GTK+ 3.20

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 11 20:31:28 PDT 2016


Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
 Attachment #276138|review?                     |review+
              Flags|                            |

--- Comment #2 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 276138
  --> https://bugs.webkit.org/attachment.cgi?id=276138

View in context: https://bugs.webkit.org/attachment.cgi?id=276138&action=review

> Source/WebCore/ChangeLog:9
> +        needed for scrollbars, this patch uses the RenderthemGadget classes introduced in r199292 to render the native


> Source/WebCore/platform/gtk/RenderThemeGadget.cpp:70
> +    // Scrollbars need to use its GType to be able to get non-CSS style properties.

Wow, I thought the type was ignored....

> Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:86
> +static GRefPtr<GtkStyleContext> createChildStyleContext(GtkStyleContext* parent, const char* name)

I'd use className instead of name to make it more clear what that parameter is for.

> Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:157
> +        FALLTHROUGH;

Didn't know we had this. Cool.

> Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:309
> +    // painting the thumb can be skipped. We don't have to be exact here.

I would name the method something like ScrollbarThemeGtk::probablyHasThumb.

> Source/WebCore/platform/gtk/ScrollbarThemeGtk.h:65
> +    bool m_hasForwardButtonStartPart : 1;

I don't pretend to know if using a bitfield here would improve performance because due to better cache locality, or hurt performance due to the nature of bitfields (seems more likely to me?), but it only saves three bytes, and it's not like this structure is going to be allocated frequently (should be just once, right?) so I would personally use normal bools here.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160412/ef116a27/attachment.html>

More information about the webkit-unassigned mailing list