[webkit-reviews] review denied: [Bug 180120] REGRESSION(r218064): [GTK] Broke entering fullscreen mode : [Attachment 327811] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Michael Catanzaro
<mcatanzaro at igalia.com>'s request for review:
Bug 180120: REGRESSION(r218064): [GTK] Broke entering fullscreen mode
https://bugs.webkit.org/show_bug.cgi?id=180120

Attachment 327811: Patch

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




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

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

> Source/WebKit/ChangeLog:8
> +	   Remove the bad assertions.

Why not fix them instead?

> Source/WebKit/ChangeLog:10
> +	   This should hopefully fix /webkit2/WebKitWebView/fullscreen in debug
mode.

Hopefully? Could you check it before submitting a new patch?

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:-1308
> -    ASSERT(priv->fullScreenModeActive);
> -

This is only called by the page client when not in fullscreen mode, the
intention of the assert was to ensure that was always the case, but for some
reason I added the assert checking the opposite. So, the actual fix is changing
this assert to 

ASSERT(!priv->fullScreenModeActive);

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:-1326
> -    ASSERT(!priv->fullScreenModeActive);
> -

Same here, we want to ensure we leave fullscreen while we are in fullscreen so
the right assert is:

ASSERT(priv->fullScreenModeActive);