Bug 136849

Created attachment 238188 [details] Patch

Attachment 238188 [details] did not pass style-queue: ERROR: Source/WebCore/platform/gtk/DisplaySystemType.h:35: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 238188 [details] Patch You can probably just tuck this into GtkUtilities.

Created attachment 238195 [details] Patch

Attachment 238195 [details] did not pass style-queue: ERROR: Source/WebCore/platform/gtk/GtkUtilities.h:33: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 238195 [details] Patch 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.

Created attachment 238211 [details] Patch

Attachment 238211 [details] did not pass style-queue: ERROR: Source/WebCore/platform/gtk/GtkUtilities.h:30: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 238211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238211&action=review You don't have to apologize. I appreciate you taking the time and going through patches thoroughly. > Source/WebCore/platform/gtk/GtkUtilities.cpp:67 > + static DisplaySystemType type = [] { This version simplifies ('simplifies') the initialization by using a lambda. This enables a properly applied ASSERT_NOT_REACHED() at the bottom of the labmda.

Comment on attachment 238211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238211&action=review >> Source/WebCore/platform/gtk/GtkUtilities.cpp:67 >> + static DisplaySystemType type = [] { > > This version simplifies ('simplifies') the initialization by using a lambda. This enables a properly applied ASSERT_NOT_REACHED() at the bottom of the labmda. Plus there's no need for the is-this-static-var-initialized-yet check.

Comment on attachment 238211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238211&action=review >>> Source/WebCore/platform/gtk/GtkUtilities.cpp:67 >>> + static DisplaySystemType type = [] { >> >> This version simplifies ('simplifies') the initialization by using a lambda. This enables a properly applied ASSERT_NOT_REACHED() at the bottom of the labmda. > > Plus there's no need for the is-this-static-var-initialized-yet check. Does that actually delay the execution of the code until the first call? If so, that's pretty awesome.

Comment on attachment 238211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238211&action=review >>>> Source/WebCore/platform/gtk/GtkUtilities.cpp:67 >>>> + static DisplaySystemType type = [] { >>> >>> This version simplifies ('simplifies') the initialization by using a lambda. This enables a properly applied ASSERT_NOT_REACHED() at the bottom of the labmda. >> >> Plus there's no need for the is-this-static-var-initialized-yet check. > > Does that actually delay the execution of the code until the first call? If so, that's pretty awesome. That's exactly what happens. The lambda is also completely optimized away when building with optimizations, so there's no code being generated under a separate symbol.

Comment on attachment 238211 [details] Patch Clearing flags on attachment: 238211 Committed r173690: <http://trac.webkit.org/changeset/173690>

All reviewed patches have been landed. Closing bug.