[webkit-reviews] review granted: [Bug 201507] [GTK] Crash in Nicosia::GC3DLayer::makeContextCurrent due to failure in EGL display creation : [Attachment 404920] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Adrian Perez
<aperez at igalia.com>'s request for review:
Bug 201507: [GTK] Crash in Nicosia::GC3DLayer::makeContextCurrent due to
failure in EGL display creation
https://bugs.webkit.org/show_bug.cgi?id=201507

Attachment 404920: Patch

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




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

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

> Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:195
> +    default:
> +	   RELEASE_ASSERT_NOT_REACHED();

I think it's better not to add default here, since we are handling all possible
cases (supported for the given build).

> Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:248
> +	   WTFLogAlways("Cannot create surfaceless EGL context: required
extensions missing.");

I prefer not to add messages for things that are not errors. We did in the past
and people thought there were errors, reporting them as possible cause of other
bugs.

> Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:306
> +	   default:
> +	       RELEASE_ASSERT_NOT_REACHED();
> +	   }

Same here about the default, let the compiler complain.

> Source/WebCore/platform/graphics/egl/GLContextEGL.cpp:353
> +	   default:
> +	       RELEASE_ASSERT_NOT_REACHED();

Ditto.