[webkit-reviews] review granted: [Bug 136849] [GTK] Add a helper class for display system deduction : [Attachment 238195] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Zan Dobersek
<zandobersek at gmail.com>'s request for review:
Bug 136849: [GTK] Add a helper class for display system deduction
https://bugs.webkit.org/show_bug.cgi?id=136849

Attachment 238195: Patch
https://bugs.webkit.org/attachment.cgi?id=238195&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=238195&action=review


Looks like the style error is a false positive. Apologies for my annoying
review habits. Here are my final thoughts before landing.

> Source/WebCore/platform/gtk/GtkUtilities.cpp:75
> +	   ASSERT(type != Unknown);

Here you can simply do ASSERT_NOT_REACHED and return X11. By having an unknown
type, you are forcing switch statements to handle Unknown which is a fatal
error anyway and should be caught earlier.

> Source/WebCore/platform/gtk/GtkUtilities.h:29
> +class DisplaySystemType {

I'm not sure this method needs its own class. Why not just define
DisplaySystemType and a method called getDisplaySystemType?

> Source/WebCore/platform/gtk/GtkUtilities.h:32
> +	   Unknown,

I think you can omit the Unknown type.