[webkit-reviews] review granted: [Bug 149496] [GTK][WPE] Use mobile user-agent on tablet : [Attachment 403776] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Jan-Michael Brummer
<jan.brummer at tabos.org>'s request for review:
Bug 149496: [GTK][WPE] Use mobile user-agent on tablet
https://bugs.webkit.org/show_bug.cgi?id=149496

Attachment 403776: Patch

https://bugs.webkit.org/attachment.cgi?id=403776&action=review




--- Comment #33 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 403776
  --> https://bugs.webkit.org/attachment.cgi?id=403776
Patch

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

> Source/WTF/wtf/PlatformGTK.cmake:27
> +    glib/ChassisType.cpp

You have to add the files to PlatformWPE.cmake too.

> Source/WTF/wtf/glib/ChassisType.cpp:88
> +    } else {
> +	   if (!g_error_matches(error.get(), G_FILE_ERROR, G_FILE_ERROR_NOENT))

else if?

> Source/WTF/wtf/glib/ChassisType.cpp:118
> +    } else {
> +	   if (!g_error_matches(error.get(), G_FILE_ERROR, G_FILE_ERROR_NOENT))

Ditto.

> Source/WTF/wtf/glib/ChassisType.cpp:136
> +	   if (chassisType == ChassisType::Unknown)
> +	       chassisType = ChassisType::Desktop;

So, Unknown is never exposed, we always fallback to Desktop. I think it would
be better to use Optional<> instead of adding the Unknown value to the enum.
All read functions would return Optional<ChassisType>, returning WTF::nullopt
when unknown.