Bug 222625

Created attachment 422046 [details] Patch

Committed r273814: <https://commits.webkit.org/r273814> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422046 [details].

Unfortunately it looks like this broke gobject-introspection. I will try to investigate, but we need to revert in the meantime, sorry. [715/2392] Generating ../../JavaScriptCore-4.0.gir ../../../../Source/JavaScriptCore/API/glib/JSCClass.cpp:644: Warning: JavaScriptCore: jsc_class_add_constructorv: return value: Unresolved type: 'JSCValue*' ../../../../Source/JavaScriptCore/API/glib/JSCClass.cpp:688: Warning: JavaScriptCore: jsc_class_add_constructor_variadic: return value: Unresolved type: 'JSCValue*' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:620: Warning: JavaScriptCore: jsc_context_get_virtual_machine: return value: Unresolved type: 'JSCVirtualMachine*' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:635: Warning: JavaScriptCore: jsc_context_get_exception: return value: Unresolved type: 'JSCException*' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:716: Warning: JavaScriptCore: jsc_context_throw_exception: argument exception: Unresolved type: 'JSCException*' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:827: Warning: JavaScriptCore: jsc_context_evaluate: return value: Unresolved type: 'JSCValue*' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:853: Warning: JavaScriptCore: jsc_context_evaluate_with_source_uri: return value: Unresolved type: 'JSCValue*' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:868: Warning: JavaScriptCore: jsc_context_evaluate_in_object: argument object: Unresolved type: 'JSCValue**' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:885: Warning: JavaScriptCore: jsc_context_evaluate_in_object: return value: Unresolved type: 'JSCValue*' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:930: Warning: JavaScriptCore: jsc_context_check_syntax: argument exception: Unresolved type: 'JSCException**' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:1024: Warning: JavaScriptCore: jsc_context_get_global_object: return value: Unresolved type: 'JSCValue*' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:1033: Warning: JavaScriptCore: jsc_context_set_value: argument value: Unresolved type: 'JSCValue*' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:1058: Warning: JavaScriptCore: jsc_context_get_value: return value: Unresolved type: 'JSCValue*' ../../../../Source/JavaScriptCore/API/glib/JSCContext.cpp:600: Warning: JavaScriptCore: jsc_context_new_with_virtual_machine: argument vm: Unresolved type: 'JSCVirtualMachine*' ../../../../Source/JavaScriptCore/API/glib/JSCWeakValue.cpp:184: Warning: JavaScriptCore: jsc_weak_value_get_value: return value: Unresolved type: 'JSCValue*' ../../../../Source/JavaScriptCore/API/glib/JSCWeakValue.cpp:163: Warning: JavaScriptCore: jsc_weak_value_new: argument value: Unresolved type: 'JSCValue*'

(In reply to Michael Catanzaro from comment #13) > Unfortunately it looks like this broke gobject-introspection. I will try to > investigate, but we need to revert in the meantime, sorry. Don and I tried a lot of different things but couldn't figure out how to fix this. It's strange.

Re-opened since this is blocked by bug 222676

Created attachment 422228 [details] GIR

Comment on attachment 422228 [details] GIR View in context: https://bugs.webkit.org/attachment.cgi?id=422228&action=review Going through step by step to see what's going on with the .gir build. > Source/JavaScriptCore/PlatformGTK.cmake:90 > - -I${CMAKE_SOURCE_DIR}/Source > - -I${JAVASCRIPTCORE_DIR} > + -I${JAVASCRIPTCORE_DIR}/API/glib > -I${DERIVED_SOURCES_JAVASCRIPCOREGTK_DIR} > -I${FORWARDING_HEADERS_DIR}/JavaScriptCore/glib Feels like you'd just need access to the `<jsc/Foo.h>` headers which is in the `${FORWARDING_HEADERS_DIR}/JavaScriptCore/glib` and then the `${JAVASCRIPTCORE_DIR}/API/glib` which handles the `"Foo.h"` headers. The `${DERIVED_SOURCES_JAVASCRIPCOREGTK_DIR} ` is there for the `<jsc/JSCVersion.h>`. This patch built with introspection so it seems sensible.

Created attachment 422257 [details] GIR + JSC*Private API headers should not include JSC API headers

Created attachment 422259 [details] GIR + previous + use <jsc/> includes in private headers

Comment on attachment 422257 [details] GIR + JSC*Private API headers should not include JSC API headers View in context: https://bugs.webkit.org/attachment.cgi?id=422257&action=review > Source/JavaScriptCore/API/glib/JSCContextPrivate.h:33 > -#include "APICast.h" > #include "JSCContext.h" > #include "JSCValue.h" > -#include "JSContextRef.h" > #include <wtf/glib/GRefPtr.h> > > +namespace JSC { > +class JSObject; > +} > + > +typedef struct OpaqueJSClass* JSClassRef; > +typedef struct OpaqueJSContext* JSGlobalContextRef; > +typedef const struct OpaqueJSValue* JSValueRef; > +typedef struct OpaqueJSValue* JSObjectRef; This version of the patch removes any headers outside of the GLib JSC directory and instead uses forward declarations. This only affects the JSC*Private.h includes which are additional C API's used in WebKit/WebProcess/InjectedBundle/API/glib/**/*.cpp files.

Comment on attachment 422259 [details] GIR + previous + use <jsc/> includes in private headers View in context: https://bugs.webkit.org/attachment.cgi?id=422259&action=review > Source/JavaScriptCore/API/glib/JSCClassPrivate.h:24 > -#include "JSCClass.h" > -#include "JSCContext.h" > -#include "JSCValue.h" > +#include <jsc/JSCClass.h> > +#include <jsc/JSCContext.h> > +#include <jsc/JSCValue.h> This next patch goes further and includes those files as <jsc/JSC*.h> rather than "JSC*.h". This fits with how Apple's private JSC API headers work. They include <JavaScriptCore/*.h> rather than "*.h". This is because on disk the headers are stored separately. See https://github.com/WebKit/WebKit/blob/main/Source/JavaScriptCore/API/JSBasePrivate.h as an example. My personal preference is to do it this way but I am not a GLib maintainer. I think I want to split this bug along so one is focused on the API and one is focused on the CMake. Would like to know GLib maintainers preference on which patch.

Comment on attachment 422259 [details] GIR + previous + use <jsc/> includes in private headers View in context: https://bugs.webkit.org/attachment.cgi?id=422259&action=review As far as I can tell, this is a misunderstanding of how GLib private headers relate to Apple headers. You've explained to me on IRC that Apple's private headers are actually installed and used by Apple apps. GLib private headers are not. These headers are only for use by WebKit itself, when building WebKit. So there should be no restrictions on what headers can be #included. In particular, there's no reason to try to avoid #include "APICast.h". Ideally GLib API private headers should be treated exactly the same as any other random file in JavaScriptCore, say B3ArgumentRegValue.h. > Source/JavaScriptCore/PlatformGTK.cmake:73 > + --warn-error Good. :P

<rdar://problem/75238500>