Bug 180120 – REGRESSION(r218064): [GTK] Broke entering fullscreen mode in debug builds

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

r218064 added a bad assert when entering fullscreen mode. It just crashes now (in debug builds).

[Michael Catanzaro]

Created attachment 327809 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Michael Catanzaro]

Created attachment 327811 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 327811 [details]
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);

[Michael Catanzaro]

OK, I'll swap the assertions. That works.

> > Source/WebKit/ChangeLog:10
> > +        This should hopefully fix /webkit2/WebKitWebView/fullscreen in debug mode.
> 
> Hopefully? Could you check it before submitting a new patch?

I thought I would need a rebuild to run API tests, but I actually don't. It works.

[Michael Catanzaro]

Created attachment 327853 [details]
Patch

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 327853 [details]:

editing/spelling/spellcheck-async.html bug 160571 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 327853 [details]
Patch

Clearing flags on attachment: 327853

Committed r225267: <https://trac.webkit.org/changeset/225267>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.