Bug 224149 – [GTK] Properly use CompletionHandler when USE_OPENGL_OR_ES is set to OFF

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Charlene Wendling]

Properly use CompletionHandler when USE_OPENGL_OR_ES is set to OFF

[Charlene Wendling]

Created attachment 425093 [details]
Patch

[Charlene Wendling]

When trying to build WebKit with USE_OPENGL_OR_ES set to off, the build fails with:

LayerTreeHost.h:216:28: error: out-of-line definition of 'forceRepaintAsync'
+does not match any declaration in 'WebKit::LayerTreeHost'
LayerTreeHost.h:82:28: note: type of 1st parameter of member declaration does
not match definition ('CompletionHandler<void ()> &&' vs
'CompletionHandler<void ()> &')

It has been introduced by that change : https://trac.webkit.org/changeset/271171/webkit

The attached patch fixes that issue.

[Alex Christensen]

Comment on attachment 425093 [details]
Patch

You might consider calling it.

[Fujii Hironori]

Comment on attachment 425093 [details]
Patch

style-checker failed. Please fix it. Otherwise, commit bot will complain.

[Charlene Wendling]

Created attachment 425205 [details]
Revised patch

Here is it is. It passes check-webkit-style this time.

[Alex Christensen]

Comment on attachment 425205 [details]
Revised patch

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

> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/LayerTreeHost.h:216
> +inline void LayerTreeHost::forceRepaintAsync(CompletionHandler<void()>&&) { }

I recommend calling the completion handler.

[Fujii Hironori]

Charlene, what's your opinion on Alex's review point?
I think it's just a stub to avoid linkage errors, not called from anywhere. I think it's no problem. 
Adding ASSERT_NOT_REACHED() might be another idea.
inline void LayerTreeHost::forceRepaintAsync(CompletionHandler<void()>&&) { ASSERT_NOT_REACHED(); }

[Charlene Wendling]

(In reply to Fujii Hironori from comment #7)
> Charlene, what's your opinion on Alex's review point?
> I think it's just a stub to avoid linkage errors, not called from anywhere.
> I think it's no problem. 
> Adding ASSERT_NOT_REACHED() might be another idea.
> inline void LayerTreeHost::forceRepaintAsync(CompletionHandler<void()>&&) {
> ASSERT_NOT_REACHED(); }

In fact, while i reported the build failure and proposed a possible fix, i'm not able to define what would be the proper course of action.

[EWS]

Committed r275802 (236373@main): <https://commits.webkit.org/236373@main>

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