RESOLVED FIXED 32024
[GTK] WebKit does not compile without JAVASCRIPT_DEBUGGER
https://bugs.webkit.org/show_bug.cgi?id=32024
Summary [GTK] WebKit does not compile without JAVASCRIPT_DEBUGGER
Gyuyoung Kim
Reported 2009-12-01 08:50:46 PST
Created attachment 44085 [details] patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk When I try to build webkit without ENABLE(JAVASCRIPT_DEBUGGER), there are some problems. First, even though I run './configure --enable-javascript-debugger=no', the ENABLE(JAVASCRIPT_DEBUGGER) is still enabled. I'm going to report the reason. Second, when I defined 'ENABLE_JAVASCRIPT_DEBUGGER 0' as IPHONE, ANDROID in wtf/Platform.h, there are two build errors as below, -f "WebKit/gtk/webkit/.deps/libwebkit_1_0_la-webkitwebinspector.Tpo"; exit 1; fi WebKit/gtk/webkit/webkitwebinspector.cpp: In function 'void webkit_web_inspector_set_property(GObject*, guint, const GValue*, GParamSpec*)': WebKit/gtk/webkit/webkitwebinspector.cpp:353: error: 'class WebCore::InspectorController' has no member named 'enableProfiler' WebKit/gtk/webkit/webkitwebinspector.cpp:355: error: 'class WebCore::InspectorController' has no member named 'disableProfiler' WebKit/gtk/webkit/webkitwebinspector.cpp: In function 'void webkit_web_inspector_get_property(GObject*, guint, GValue*, GParamSpec*)': WebKit/gtk/webkit/webkitwebinspector.cpp:388: error: 'class WebCore::InspectorController' has no member named 'profilerEnabled' make[1]: *** [WebKit/gtk/webkit/libwebkit_1_0_la-webkitwebinspector.lo] Error 1 make[1]: Leaving directory `/root/workspace/WebKit' make: *** [all] Error 2 Tpo" "WebKit/gtk/webkit/.deps/libwebkit_1_0_la-webkitwebinspector.Plo"; else rm -f "WebKit/gtk/webkit/.deps/libwebkit_1_0_la-webkitwebinspector.Tpo"; exit 1; fi WebKit/gtk/webkit/webkitwebinspector.cpp: In function 'void webkit_web_inspector_get_property(GObject*, guint, GValue*, GParamSpec*)': WebKit/gtk/webkit/webkitwebinspector.cpp:388: error: 'class WebCore::InspectorController' has no member named 'profilerEnabled' make[1]: *** [WebKit/gtk/webkit/libwebkit_1_0_la-webkitwebinspector.lo] Error 1 make[1]: Leaving directory `/root/workspace/WebKit' make: *** [all] Error 2 The reason of upper build errors is that webkitwebinspector.cpp still invokes the enableProfiler(), disableProfiler() and profilerEnabled() defined in WebCore/inspector/InspectorController.h. In my opinion, it seems embedded device based on gtk doesn't need to use JAVASCRIPT_DEBUGGER feature. I make a try a patch for webkitwebinspector.cpp. If there are any nits or problems, please let me know. Thanks.
Attachments
patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk (1.10 KB, patch)
2009-12-01 08:50 PST, Gyuyoung Kim
eric: review-
patch with Changelog (1.72 KB, patch)
2009-12-01 19:09 PST, Gyuyoung Kim
gustavo: review-
Patch with g_message (1.97 KB, patch)
2009-12-02 16:18 PST, Gyuyoung Kim
no flags
patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk (3.08 KB, patch)
2009-12-04 11:05 PST, Gyuyoung Kim
eric: review-
patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk-2 (3.05 KB, patch)
2009-12-04 19:04 PST, Gyuyoung Kim
no flags
patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk-3 (3.05 KB, patch)
2009-12-04 23:06 PST, Gyuyoung Kim
no flags
WebKit Review Bot
Comment 1 2009-12-01 17:04:13 PST
style-queue ran check-webkit-style on attachment 44085 [details] without any errors.
Eric Seidel (no email)
Comment 2 2009-12-01 17:08:31 PST
Comment on attachment 44085 [details] patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk I'm surprised that these case statements would be outisde the #if. Does the UI still show JS profiling stuff, and does the console object still expose profiling JS commands? This looks OK as a build fix, but is probably not the full fix.
Eric Seidel (no email)
Comment 3 2009-12-01 17:09:23 PST
Comment on attachment 44085 [details] patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk Actually, nevermind. This has no ChangeLog. see http://webkit.org/coding/contributing.html
Gyuyoung Kim
Comment 4 2009-12-01 19:09:22 PST
Created attachment 44126 [details] patch with Changelog Thank you for your comment. When I define "ENABLE_JAVASCRIPT_DEBUGGER 0" in Platform.h, there are no build errors on this patch. However, I also think that this patch is not full fix. I will report full patch to this bug.
Gustavo Noronha (kov)
Comment 5 2009-12-02 10:26:31 PST
Comment on attachment 44126 [details] patch with Changelog Hrm. Yeah, I think the case statements should indeed be outside the #if clauses, because it would be kind of an API breakage otherwise, but you should perhaps add a g_message to the #else case, and make sure get_property already sets false to the value, just to be sure.
Gyuyoung Kim
Comment 6 2009-12-02 16:18:04 PST
Created attachment 44192 [details] Patch with g_message I add a g_message to #else case. However, I'm not sure it is you want. If there are problems, please let me know. Thanks.
Gyuyoung Kim
Comment 7 2009-12-04 11:05:33 PST
Created attachment 44323 [details] patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk I have tried to solve the build breaks. As I mentioned in description, there are two cases of error. First, there are build errors as below when the javascript-debugger is disabled in configure(./configure --enable-javascript-debugger=no). WebCore/bindings/js/JSConsoleCustom.cpp:42: error: no ‘JSC::JSValue WebCore::JSConsole::profiles(JSC::ExecState*) const’ member function declared in class ‘WebCore::JSConsole’ WebCore/bindings/js/JSConsoleCustom.cpp:54: error: no ‘JSC::JSValue WebCore::JSConsole::profile(JSC::ExecState*, const JSC::ArgList&)’ member function declared in class ‘WebCore::JSConsole’ WebCore/bindings/js/JSConsoleCustom.cpp:62: error: no ‘JSC::JSValue WebCore::JSConsole::profileEnd(JSC::ExecState*, const JSC::ArgList&)’ member function declared in class ‘WebCore::JSConsole’ make[1]: *** [WebCore/bindings/js/libWebCore_la-JSConsoleCustom.lo] Error 1 make[1]: Leaving directory `/home/gyuyoung/workspace/WebKit' make: *** [all] Error 2 I think that JSConsole.h of DerivedSources doesn't make the JSConsole::profiles() because of the configure setting, but JSConsoleCustom.cpp still compiles the JSConsole::profiles(), which are wrapped by JAVASCRIPT_DEBUGGER as below, #if ENABLE(JAVASCRIPT_DEBUGGER) typedef Vector<RefPtr<JSC::Profile> > ProfilesArray; JSValue JSConsole::profiles(ExecState* exec) const { const ProfilesArray& profiles = impl()->profiles(); MarkedArgumentBuffer list; ProfilesArray::const_iterator end = profiles.end(); for (ProfilesArray::const_iterator iter = profiles.begin(); iter != end; ++iter) list.append(toJS(exec, iter->get())); return constructArray(exec, list); } ... #endif The reason is that JAVASCRIPT_DEBUGGER macro is enabled by Platform.h as below, 679 #if !defined(ENABLE_JAVASCRIPT_DEBUGGER) 680 #define ENABLE_JAVASCRIPT_DEBUGGER 1 681 #endif Upper condition means that if ENABLE_JAVASCRIPT_DEBUGGER is not defined, the ENABLE_JAVASCRIPT_DEBUGGER should be enabled. (It seems the ENABLE_JAVASCRIPT_DEBUGGER is enabled by WebCore/GNUmakefile.am when it is set as "yes" in configure) I think this make the build errors even though the "javascript-debugger" is disabled by configure. IMO, why don't we make the ENABLE_JAVASCRIPT_DEBUGGER set as 0(disable) when it is not defined ? If user sets the javascript-debugger as "no", it means that user doesn't want to use it. So, I think the feature can be disabled as default. BTW, even if the macro is set as 0 basically, as mentioned in description, there are still build breaks as below, -f "WebKit/gtk/webkit/.deps/libwebkit_1_0_la-webkitwebinspector.Tpo"; exit 1; fi WebKit/gtk/webkit/webkitwebinspector.cpp: In function 'void webkit_web_inspector_set_property(GObject*, guint, const GValue*, GParamSpec*)': WebKit/gtk/webkit/webkitwebinspector.cpp:353: error: 'class WebCore::InspectorController' has no member named 'enableProfiler' WebKit/gtk/webkit/webkitwebinspector.cpp:355: error: 'class WebCore::InspectorController' has no member named 'disableProfiler' WebKit/gtk/webkit/webkitwebinspector.cpp: In function 'void webkit_web_inspector_get_property(GObject*, guint, GValue*, GParamSpec*)': WebKit/gtk/webkit/webkitwebinspector.cpp:388: error: 'class WebCore::InspectorController' has no member named 'profilerEnabled' make[1]: *** [WebKit/gtk/webkit/libwebkit_1_0_la-webkitwebinspector.lo] Error 1 make[1]: Leaving directory `/root/workspace/WebKit' make: *** [all] Error 2 Tpo" "WebKit/gtk/webkit/.deps/libwebkit_1_0_la-webkitwebinspector.Plo"; else rm -f "WebKit/gtk/webkit/.deps/libwebkit_1_0_la-webkitwebinspector.Tpo"; exit 1; fi WebKit/gtk/webkit/webkitwebinspector.cpp: In function 'void webkit_web_inspector_get_property(GObject*, guint, GValue*, GParamSpec*)': WebKit/gtk/webkit/webkitwebinspector.cpp:388: error: 'class WebCore::InspectorController' has no member named 'profilerEnabled' make[1]: *** [WebKit/gtk/webkit/libwebkit_1_0_la-webkitwebinspector.lo] Error 1 make[1]: Leaving directory `/root/workspace/WebKit' make: *** [all] Error 2 The webkitwebinspector.cpp invokes some functions related to JAVASCRIPT_DEBUGGER macro regardless of enabling/disabling it. I make a patch for it. Please review it.
WebKit Review Bot
Comment 8 2009-12-04 11:10:06 PST
style-queue ran check-webkit-style on attachment 44323 [details] without any errors.
Gyuyoung Kim
Comment 9 2009-12-04 17:20:00 PST
Dear Eric : Could you review this patch ? (In reply to comment #7) > Created an attachment (id=44323) [details] > patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk > > I have tried to solve the build breaks. As I mentioned in description, there > are two cases of error. First, there are build errors as below when the > javascript-debugger is disabled in configure(./configure > --enable-javascript-debugger=no). > > WebCore/bindings/js/JSConsoleCustom.cpp:42: error: no ‘JSC::JSValue > WebCore::JSConsole::profiles(JSC::ExecState*) const’ member function declared > in class ‘WebCore::JSConsole’ > WebCore/bindings/js/JSConsoleCustom.cpp:54: error: no ‘JSC::JSValue > WebCore::JSConsole::profile(JSC::ExecState*, const JSC::ArgList&)’ member > function declared in class ‘WebCore::JSConsole’ > WebCore/bindings/js/JSConsoleCustom.cpp:62: error: no ‘JSC::JSValue > WebCore::JSConsole::profileEnd(JSC::ExecState*, const JSC::ArgList&)’ member > function declared in class ‘WebCore::JSConsole’ > make[1]: *** [WebCore/bindings/js/libWebCore_la-JSConsoleCustom.lo] Error 1 > make[1]: Leaving directory `/home/gyuyoung/workspace/WebKit' > make: *** [all] Error 2 > > I think that JSConsole.h of DerivedSources doesn't make the > JSConsole::profiles() because of the configure setting, but JSConsoleCustom.cpp > still compiles the JSConsole::profiles(), which are wrapped by > JAVASCRIPT_DEBUGGER as below, > > #if ENABLE(JAVASCRIPT_DEBUGGER) > > typedef Vector<RefPtr<JSC::Profile> > ProfilesArray; > > JSValue JSConsole::profiles(ExecState* exec) const > { > const ProfilesArray& profiles = impl()->profiles(); > MarkedArgumentBuffer list; > > ProfilesArray::const_iterator end = profiles.end(); > for (ProfilesArray::const_iterator iter = profiles.begin(); iter != end; > ++iter) > list.append(toJS(exec, iter->get())); > > return constructArray(exec, list); > } > ... > #endif > > The reason is that JAVASCRIPT_DEBUGGER macro is enabled by Platform.h as below, > > 679 #if !defined(ENABLE_JAVASCRIPT_DEBUGGER) > 680 #define ENABLE_JAVASCRIPT_DEBUGGER 1 > 681 #endif > > Upper condition means that if ENABLE_JAVASCRIPT_DEBUGGER is not defined, the > ENABLE_JAVASCRIPT_DEBUGGER should be enabled. > (It seems the ENABLE_JAVASCRIPT_DEBUGGER is enabled by WebCore/GNUmakefile.am > when it is set as "yes" in configure) > I think this make the build errors even though the "javascript-debugger" is > disabled by configure. > > IMO, why don't we make the ENABLE_JAVASCRIPT_DEBUGGER set as 0(disable) when it > is not defined ? If user sets the javascript-debugger as "no", it means that > user doesn't want to use it. So, I think the feature can be disabled as > default. > > BTW, even if the macro is set as 0 basically, as mentioned in description, > there are still build breaks as below, > > -f "WebKit/gtk/webkit/.deps/libwebkit_1_0_la-webkitwebinspector.Tpo"; exit 1; > fi > WebKit/gtk/webkit/webkitwebinspector.cpp: In function 'void > webkit_web_inspector_set_property(GObject*, guint, const GValue*, > GParamSpec*)': > WebKit/gtk/webkit/webkitwebinspector.cpp:353: error: 'class > WebCore::InspectorController' has no member named 'enableProfiler' > WebKit/gtk/webkit/webkitwebinspector.cpp:355: error: 'class > WebCore::InspectorController' has no member named 'disableProfiler' > WebKit/gtk/webkit/webkitwebinspector.cpp: In function 'void > webkit_web_inspector_get_property(GObject*, guint, GValue*, GParamSpec*)': > WebKit/gtk/webkit/webkitwebinspector.cpp:388: error: 'class > WebCore::InspectorController' has no member named 'profilerEnabled' > make[1]: *** [WebKit/gtk/webkit/libwebkit_1_0_la-webkitwebinspector.lo] Error 1 > make[1]: Leaving directory `/root/workspace/WebKit' > make: *** [all] Error 2 > > Tpo" "WebKit/gtk/webkit/.deps/libwebkit_1_0_la-webkitwebinspector.Plo"; else rm > -f "WebKit/gtk/webkit/.deps/libwebkit_1_0_la-webkitwebinspector.Tpo"; exit 1; > fi > WebKit/gtk/webkit/webkitwebinspector.cpp: In function 'void > webkit_web_inspector_get_property(GObject*, guint, GValue*, GParamSpec*)': > WebKit/gtk/webkit/webkitwebinspector.cpp:388: error: 'class > WebCore::InspectorController' has no member named 'profilerEnabled' > make[1]: *** [WebKit/gtk/webkit/libwebkit_1_0_la-webkitwebinspector.lo] Error 1 > make[1]: Leaving directory `/root/workspace/WebKit' > make: *** [all] Error 2 > > The webkitwebinspector.cpp invokes some functions related to > JAVASCRIPT_DEBUGGER macro regardless of enabling/disabling it. > > I make a patch for it. Please review it.
Eric Seidel (no email)
Comment 10 2009-12-04 17:48:57 PST
Comment on attachment 44323 [details] patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk The platform change is wrong. I do not think you should change the default value for ENABLE_JAVASCRIPT_DEBUGGER.
Gyuyoung Kim
Comment 11 2009-12-04 19:04:22 PST
Created attachment 44345 [details] patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk-2 I add '-DENABLE_JAVASCRIPT_DEBUGGER 0' to WebCore/GNUmakefile.am as below, =========================================================== if ENABLE_JAVASCRIPT_DEBUGGER FEATURE_DEFINES_JAVASCRIPT += ENABLE_JAVASCRIPT_DEBUGGER=1 webcore_cppflags += \ -DENABLE_JAVASCRIPT_DEBUGGER=1 else webcore_cppflags += \ -DENABLE_JAVASCRIPT_DEBUGGER=0 endif # END ENABLE_JAVASCRIPT_DEBUGGER =========================================================== I find that some features ,for example ENABLE_DATABASE, ENABLE_ICONDATABASE, ENABLE_WEB_SOCKETS, already use this way.
Gyuyoung Kim
Comment 12 2009-12-04 23:06:35 PST
Created attachment 44348 [details] patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk-3 Oops, I added an unnecessary blank line to Changelog. I make a patch again. Thanks.
WebKit Review Bot
Comment 13 2009-12-04 23:08:38 PST
style-queue ran check-webkit-style on attachment 44348 [details] without any errors.
Gyuyoung Kim
Comment 14 2009-12-07 07:33:05 PST
Dear Eric, Sorry for frequent review request. Could you review my latest patch again ? I want to fix this defect.
Eric Seidel (no email)
Comment 15 2009-12-07 10:35:15 PST
Comment on attachment 44348 [details] patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk-3 I don't know really anything about the GNUmakefile.am, but this looks sane enough. Seems strange that whatever build system is using that file doesn't have a cleaner/more consistent way to set feature defines.
WebKit Commit Bot
Comment 16 2009-12-07 11:22:57 PST
Comment on attachment 44348 [details] patch for the build error when JAVASCRIPT_DEBUGGER is disabled on gtk-3 Clearing flags on attachment: 44348 Committed r51783: <http://trac.webkit.org/changeset/51783>
WebKit Commit Bot
Comment 17 2009-12-07 11:23:03 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.