[Webkit-unassigned] [Bug 126907] [GTK] Hardcoded text color in input fields

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=126907

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

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

Good job! I know this was tricky. The patch looks good to me, I just have one comment.

It'd be great if Benjamin could review this version as a sanity-check. Also, Carlos Garcia should review it as well, at least the bits in RenderThemeGtk.

> Source/WebCore/platform/gtk/RenderThemeGadget.cpp:91
> +    static GRefPtr<GtkStyleContext> baseContext = createBaseStyleContext();

It looks good, but there is a nasty trick: this is an exit-time destructor. Even if it works properly right now, it's risky since we could easily wind up destroying the GtkStyleContext after it's no longer safe to do so in the future. It's better to just leak it, since we only ever create one. The way to document this is to use the NeverDestroyed template, like this:

static NeverDestroyed<GRefPtr<GtkStyleContext>> baseContext = createBaseStyleContext();

I know it might look pretty weird to put a GRefPtr into a NeverDestroyed, but that's the idiom we use, just roll with it.

Also, as a matter of style, I would remove the blank line below it.

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