[Webkit-unassigned] [Bug 183448] [GTK][WPE] Add API to convert between DOM and JSCValue

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=183448

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #336280|review?                     |review+
              Flags|                            |

--- Comment #8 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 336280
  --> https://bugs.webkit.org/attachment.cgi?id=336280
Patch

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

> Source/WebKit/PlatformWPE.cmake:169
> +    ${WEBKIT_DIR}/WebProcess/InjectedBundle/API/wpe/DOM/WebKitDOMDocument.h
> +    ${WEBKIT_DIR}/WebProcess/InjectedBundle/API/wpe/DOM/WebKitDOMElement.h
> +    ${WEBKIT_DIR}/WebProcess/InjectedBundle/API/wpe/DOM/WebKitDOMNode.h
> +    ${WEBKIT_DIR}/WebProcess/InjectedBundle/API/wpe/DOM/WebKitDOMObject.h

So these are the DOM APIs that will live on undeprecated. I think it worked out OK for WebKitDOMDocument, WebKitDOMNode, and WebKitDOMObject, but I'm quite uncertain about WebKitDOMElement. The former WebKitDOMHTMLInputElement functions that you are providing there seem like hacked-on afterthoughts. Maybe it would make sense to keep WebKitDOMHTMLInputElement undeprecated as well? Are there any other ways we could provide the required functionality?

> Source/WebKit/WebProcess/InjectedBundle/API/glib/DOM/WebKitDOMElement.cpp:55
> +#if PLATFORM(GTK)
> +    return WEBKIT_DOM_ELEMENT(g_object_new(WEBKIT_DOM_TYPE_ELEMENT, "core-object", coreObject, nullptr));
> +#else
> +    auto* element = WEBKIT_DOM_ELEMENT(g_object_new(WEBKIT_DOM_TYPE_ELEMENT, nullptr));
> +    webkitDOMNodeSetCoreObject(WEBKIT_DOM_NODE(element), static_cast<WebCore::Node*>(coreObject));
> +    return element;
> +#endif

My main concern with this patch is that removing the deprecated stuff from the GTK+ 4 API will be quite difficult. It will be much easier to prepare for that now, rather than later. I would change all the #if PLATFORM(GTK) conditions to #if PLATFORM(GTK) && GTK_CHECK_VERSION(3, 90, 0). That way, our GTK+ 4 API will automatically get the WPE version of the code, which will save us the trouble of updating all these conditions in the future.

> Source/WebKit/WebProcess/InjectedBundle/API/glib/DOM/WebKitDOMElement.cpp:66
> +#if PLATFORM(GTK)
> +G_GNUC_BEGIN_IGNORE_DEPRECATIONS;
> +G_DEFINE_TYPE_WITH_CODE(WebKitDOMElement, webkit_dom_element, WEBKIT_DOM_TYPE_NODE, G_IMPLEMENT_INTERFACE(WEBKIT_DOM_TYPE_EVENT_TARGET, webkitDOMElementDOMEventTargetInit))
> +G_GNUC_END_IGNORE_DEPRECATIONS;
> +#else
> +G_DEFINE_TYPE(WebKitDOMElement, webkit_dom_element, WEBKIT_DOM_TYPE_NODE)
> +#endif

Ditto.

> Source/WebKit/WebProcess/InjectedBundle/API/glib/DOM/WebKitDOMElement.cpp:73
> +#if PLATFORM(GTK)
> +    GObjectClass* gobjectClass = G_OBJECT_CLASS(elementClass);
> +    webkitDOMElementInstallProperties(gobjectClass);
> +#endif

Ditto.

> Source/WebKit/WebProcess/InjectedBundle/API/glib/DOM/WebKitDOMElement.cpp:99
> +    if (is<WebCore::HTMLInputElement>(node))
> +        return downcast<WebCore::HTMLInputElement>(*node).lastChangeWasUserEdit();
> +
> +    if (is<WebCore::HTMLTextAreaElement>(node))
> +        return downcast<WebCore::HTMLTextAreaElement>(*node).lastChangeWasUserEdit();

I'd use g_return_val_if_fail(is<WebCore::HTMLInputElement>(node) || is<WebCore::HTMLTextAreaElement(node)). That would make debugging easier. It's programmer error, right?

> Source/WebKit/WebProcess/InjectedBundle/API/glib/DOM/WebKitDOMElement.cpp:120
> +    if (!is<WebCore::HTMLInputElement>(node))
> +        return false;

Ditto. etc.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180323/06d95a95/attachment-0001.html>