Bug 141813 – [WK2][GTK] Fix the build after r180362

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Csaba Osztrogonác]

SSIA. I won't be online until monday morning, feel free to pick it up.

[Gyuyoung Kim]

Created attachment 246942 [details]
Hot Fix

[Gyuyoung Kim]

I don't find what is correct fix against r180362. However if we don't use API::Object::wrap | API::Object::unwrap, EFL and GTK ports are fine at the moment.

[Hunseop Jeong]

Created attachment 246943 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 246943 [details]
Patch

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

> Source/WebKit2/Shared/API/c/WKSharedAPICast.h:131
> +#if PLATFORM(COCOA)

I'm not sure whether r180362 was only related with COCOA platform. I think my previous patch looks better.

[Carlos Garcia Campos]

I'm not sure this is the correct fix. wrap/unwrapp are defined in cross-platform code and there's an implementation using static casts when !DELEGATE_REF_COUNTING_TO_COCOA

[Gyuyoung Kim]

(In reply to comment #5)
> I'm not sure this is the correct fix. wrap/unwrapp are defined in
> cross-platform code and there's an implementation using static casts when
> !DELEGATE_REF_COUNTING_TO_COCOA

Yes, that's why I just uploaded a hot fix. I'm still looking this.

[Carlos Garcia Campos]

Created attachment 246945 [details]
Fix

This fixes the build fot GTK+ let's see what happens with EFL

[WebKit Commit Bot]

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

[Carlos Garcia Campos]

Since fixes a different between ports, and I don't know how to fix the EFL build, I think it would be better to use different bugs.

[Gyuyoung Kim]

(In reply to comment #9)
> Since fixes a different between ports, and I don't know how to fix the EFL
> build, I think it would be better to use different bugs.

Ok, I'm going to file a new bug for EFL port.

[Zan Dobersek]

Comment on attachment 246945 [details]
Fix

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

> Source/WebKit2/UIProcess/API/C/gtk/WKAPICastGtk.h:48
> +inline WKViewRef toAPI(WebKitWebViewBase* view)
> +{
> +    return static_cast<WKViewRef>(static_cast<void*>(view));
> +}
> +
> +inline WebKitWebViewBase* toImpl(WKViewRef view)
> +{
> +    return static_cast<WebKitWebViewBase*>(static_cast<void*>(const_cast<OpaqueWKView*>(view)));
> +}

These should be specializations of the respective templates. They should also mimic the previous behavior of the generic toAPI<>() and toImpl<>() templates.
But ideally you'd have WebKitWebViewBase somehow inherit from API::Object, if that's even possible.

I ended up with this, built with Clang but not tested:

> template<>
> inline WKViewRef toAPI<>(WebKitWebViewBase* view)
> {
>     return reinterpret_cast<WKViewRef>(static_cast<void*>(view));
> }
> 
> template<>
> inline WebKitWebViewBase* toImpl<>(WKViewRef view)
> {
>     return static_cast<WebKitWebViewBase*>(static_cast<void*>(const_cast<typename std::remove_const<typename std::remove_pointer<WKViewRef>::type>::type*>(view)));
> }

[Carlos Garcia Campos]

(In reply to comment #11)
> Comment on attachment 246945 [details]
> Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=246945&action=review
> 
> > Source/WebKit2/UIProcess/API/C/gtk/WKAPICastGtk.h:48
> > +inline WKViewRef toAPI(WebKitWebViewBase* view)
> > +{
> > +    return static_cast<WKViewRef>(static_cast<void*>(view));
> > +}
> > +
> > +inline WebKitWebViewBase* toImpl(WKViewRef view)
> > +{
> > +    return static_cast<WebKitWebViewBase*>(static_cast<void*>(const_cast<OpaqueWKView*>(view)));
> > +}
> 
> These should be specializations of the respective templates. They should
> also mimic the previous behavior of the generic toAPI<>() and toImpl<>()
> templates.
> But ideally you'd have WebKitWebViewBase somehow inherit from API::Object,
> if that's even possible.
> 
> I ended up with this, built with Clang but not tested:
> 
> > template<>
> > inline WKViewRef toAPI<>(WebKitWebViewBase* view)
> > {
> >     return reinterpret_cast<WKViewRef>(static_cast<void*>(view));
> > }
> > 
> > template<>
> > inline WebKitWebViewBase* toImpl<>(WKViewRef view)
> > {
> >     return static_cast<WebKitWebViewBase*>(static_cast<void*>(const_cast<typename std::remove_const<typename std::remove_pointer<WKViewRef>::type>::type*>(view)));
> > }

That works here.

[Carlos Garcia Campos]

Committed r180409: <http://trac.webkit.org/changeset/180409>

[Anders Carlsson]

> > I ended up with this, built with Clang but not tested:
> > 
> > > template<>
> > > inline WKViewRef toAPI<>(WebKitWebViewBase* view)
> > > {
> > >     return reinterpret_cast<WKViewRef>(static_cast<void*>(view));
> > > }
> > > 
> > > template<>
> > > inline WebKitWebViewBase* toImpl<>(WKViewRef view)
> > > {
> > >     return static_cast<WebKitWebViewBase*>(static_cast<void*>(const_cast<typename std::remove_const<typename std::remove_pointer<WKViewRef>::type>::type*>(view)));
> > > }
> 
> That works here.

Why didn't you add traits instead?

[Carlos Garcia Campos]

(In reply to comment #14)
> > > I ended up with this, built with Clang but not tested:
> > > 
> > > > template<>
> > > > inline WKViewRef toAPI<>(WebKitWebViewBase* view)
> > > > {
> > > >     return reinterpret_cast<WKViewRef>(static_cast<void*>(view));
> > > > }
> > > > 
> > > > template<>
> > > > inline WebKitWebViewBase* toImpl<>(WKViewRef view)
> > > > {
> > > >     return static_cast<WebKitWebViewBase*>(static_cast<void*>(const_cast<typename std::remove_const<typename std::remove_pointer<WKViewRef>::type>::type*>(view)));
> > > > }
> > 
> > That works here.
> 
> Why didn't you add traits instead?

Where? Could you elaborate a bit more? The thing is WebKitWebViewBase is not an API::Object, because it's not even a C++ class, but a GObject, so adding toAPI/toImpl for it was the only way I knew how to fix the problem. If there's a better way, I'll land a follow up patch.