Bug 227951 – [WPE] Implement fullscreen in WPEView

• Status: RESOLVED
• Resolution: FIXED
• Component: WPE WebKit

[Imanol Fernandez]

Implement fullscreen in WPEView:
- Notify DOM fullscreen enter request to libwpe
- Handle exit fullscreen from libwpe

[Imanol Fernandez]

Created attachment 433505 [details]
Patch

wip

[Imanol Fernandez]

Created attachment 433792 [details]
Patch

Update to latest libwpe API

[Adrian Perez]

As for the WPE build failure, we will need to land the new API
in libwpe first, and then update the Flatpak SDK to build a newer
revision of libwpe. We should do that in a separate patch that only
touches the SDK build recipes :)

[Adrian Perez]

Comment on attachment 433792 [details]
Patch

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

> Source/WebKit/UIProcess/API/wpe/WPEView.cpp:260
> +#if ENABLE(FULLSCREEN_API)

It would be good to make guards check also WPE_CHECK_VERSION(1, 11, 1)
to make it possible to build WebKit using older libwpe versions.

> Source/WebKit/UIProcess/API/wpe/WPEView.cpp:291
> +    wpe_view_backend_set_dom_fullscreen_client(m_backend, &s_fullscreenClient, this);

You will need to adapt this to the function name changes in the version
of the libwpe PR that we got landed.

[Adrian Perez]

Also I expect that implementing this will make a few more layout
tests pass. It would be good to look into enabling them, if so.
We might need some work in WKTR (Tools/WebKitTestRunner/wpe/) to
provide a fullscreening handler there and also it would be great
to make it work in the MiniBrowser (Tools/MiniBrowser/wpe/).

[Imanol Fernandez]

Created attachment 434998 [details]
Patch

Update to API function name changes. Add WPE version checks

[Lauro Moura]

(In reply to Adrian Perez from comment #4)
> 
> You will need to adapt this to the function name changes in the version
> of the libwpe PR that we got landed.

The SDK with libwpe 1.11.1 has just landed in the repo.

[Imanol Fernandez]

Created attachment 435062 [details]
Patch

Implement fullscreen in the minibrowser. Reset state if WPEView->setFullscreen fails

[Imanol Fernandez]

(In reply to Adrian Perez from comment #5)
> Also I expect that implementing this will make a few more layout
> tests pass. It would be good to look into enabling them, if so.
> We might need some work in WKTR (Tools/WebKitTestRunner/wpe/) to
> provide a fullscreening handler there and also it would be great
> to make it work in the MiniBrowser (Tools/MiniBrowser/wpe/).

I have looked into the WKTR tests and confirmed that they are not using the platform PageClientImpl->enterFullScreen path (which calls WPEView->setFullscreen). Instead the WKTR is using fullscreen code from InjectedBundlePage which sets DOM fullscreen state without calling platform fullscreen handlers. I understant that WKTR uses headless testing so it probably doesn't make sense to use real fullscreening here. Adding a WPE fullscreen handler in WKTR would not have any effect. The fullscreen related tests have the same results as before the pathc.

(In reply to Lauro Moura from comment #7)
> (In reply to Adrian Perez from comment #4)
> > 
> > You will need to adapt this to the function name changes in the version
> > of the libwpe PR that we got landed.
> 
> The SDK with libwpe 1.11.1 has just landed in the repo.

Great, I already tested with it instead of the local flatpak-sdk build :)

[Adrian Perez]

(In reply to Imanol Fernandez from comment #9)
> (In reply to Adrian Perez from comment #5)
> > Also I expect that implementing this will make a few more layout
> > tests pass. It would be good to look into enabling them, if so.
> > We might need some work in WKTR (Tools/WebKitTestRunner/wpe/) to
> > provide a fullscreening handler there and also it would be great
> > to make it work in the MiniBrowser (Tools/MiniBrowser/wpe/).
> 
> I have looked into the WKTR tests and confirmed that they are not using the
> platform PageClientImpl->enterFullScreen path (which calls
> WPEView->setFullscreen). Instead the WKTR is using fullscreen code from
> InjectedBundlePage which sets DOM fullscreen state without calling platform
> fullscreen handlers. I understant that WKTR uses headless testing so it
> probably doesn't make sense to use real fullscreening here. Adding a WPE
> fullscreen handler in WKTR would not have any effect. The fullscreen related
> tests have the same results as before the pathc.
> 
> (In reply to Lauro Moura from comment #7)
> > (In reply to Adrian Perez from comment #4)
> > > 
> > > You will need to adapt this to the function name changes in the version
> > > of the libwpe PR that we got landed.
> > 
> > The SDK with libwpe 1.11.1 has just landed in the repo.
> 
> Great, I already tested with it instead of the local flatpak-sdk build :)

And the EWS passed, too!

[Adrian Perez]

Comment on attachment 435062 [details]
Patch

The Mac EWS failures are unrelated, so let's land this ^_^

[EWS]

Committed r280774 (240358@main): <https://commits.webkit.org/240358@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 435062 [details].