Bug 24917 - Finish support for disabling the JavaScript Debugger at compile time
Summary: Finish support for disabling the JavaScript Debugger at compile time
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-28 20:03 PDT by Laszlo Gombos
Modified: 2009-05-15 01:08 PDT (History)
2 users (show)

See Also:


Attachments
Fix/Enable turning off JavaScript Debugger (46.59 KB, patch)
2009-03-28 20:36 PDT, Laszlo Gombos
mrowe: review-
Details | Formatted Diff | Diff
Finish/Fix support for disabling the JavaScript Debugger and Profiler (10.93 KB, patch)
2009-05-04 07:55 PDT, Laszlo Gombos
no flags Details | Formatted Diff | Diff
Bring the patch up-to-date (11.04 KB, patch)
2009-05-11 20:57 PDT, Laszlo Gombos
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2009-03-28 20:03:56 PDT
ChangeSet 37820 (http://trac.webkit.org/changeset/37820) laid the foundation of turning off JavaScript Debugger compile time. 

This bug is to finish the work and - similarly to other feature flags - enable turning of JavaScript Debugger. 

The intention is to keep supporting JavaScript Debugger by default, but enable individual ports/builds to turn it of.
Comment 1 Laszlo Gombos 2009-03-28 20:36:19 PDT
Created attachment 29041 [details]
Fix/Enable turning off JavaScript Debugger

Made an attempt to make sure that in all ports JavaScript Debugger support remains as it was before the patch.

Introduced guards not only where the JavaScript Debugger is used, but also in JavaScript Debugger implementation itself - to stay consistent with other feature flags, like the App Cache (e.g. see http://trac.webkit.org/browser/trunk/WebCore/loader/appcache/ApplicationCacheStorage.h)

Enable the Gtk and Qt ports to turn off JavaScript Debugger support, other ports can enable turning off JavaScript Debugger support later (if there is interest).
Comment 2 Mark Rowe (bdash) 2009-03-29 10:28:41 PDT
Comment on attachment 29041 [details]
Fix/Enable turning off JavaScript Debugger

This is not something that warrants a build-webkit or ./configure option, if this is even useful at all. 

Revision 37820 was contributed by someone from Google.  My assumption is that they wanted to be able to disable debugging in the inspector as it was not easily hooked up to their JavaScript engine at that time.  I don't see the benefit in being able to disable JavaScript debugging when being used with JavaScriptCore.
Comment 3 Laszlo Gombos 2009-03-30 18:39:54 PDT
(In reply to comment #2)

Mark, thanks for the quick feedback. 

Would it be acceptable if the build system changes are local to QtWebKit port (WebCore.pro) - which is my primary interest -  and not global.

I have not found an easy way to add defaults (that are enabled by default) to generate-bindings.pl, so to stay consistent with the rest of the build system, it might make sense to change the define to NO_JAVASCRIPT_DEBUGGER so that it could be set to 0 by default (instead of forcing JAVASCRIPT_DEBUGGER to 1).

The primary motivation for this work is to be able to reduce the binary size for embedded systems (typically not need for JavaScript debugging in production). 

The secondary motivation was to make the build system more consistent (flag should either do its job properly or should be removed).

Comment 4 Laszlo Gombos 2009-05-04 07:55:33 PDT
Created attachment 29989 [details]
Finish/Fix support for disabling the JavaScript Debugger and Profiler 

ChangeSet http://trac.webkit.org/changeset/43072 introduced a global build flag for Debuggerless  builds, and also extended the cover to disable Profiler. 

This patch fixes the build if the ENABLE_JAVASCRIPT_DEBUGGER is set to 0 (changes under page directory) and also explicitly disables some of the .h|.cpp files to reduce the binary size (as it was proposed in the first patch attached to this bug).
Comment 5 Laszlo Gombos 2009-05-11 20:57:29 PDT
Created attachment 30216 [details]
Bring the patch up-to-date
Comment 6 Eric Seidel (no email) 2009-05-15 01:08:44 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/bindings/js/JSInspectorControllerCustom.cpp
	M	WebCore/bindings/js/JSJavaScriptCallFrameCustom.cpp
	M	WebCore/inspector/JavaScriptCallFrame.cpp
	M	WebCore/inspector/JavaScriptCallFrame.h
	M	WebCore/inspector/JavaScriptCallFrame.idl
	M	WebCore/inspector/JavaScriptDebugListener.h
	M	WebCore/inspector/JavaScriptDebugServer.cpp
	M	WebCore/inspector/JavaScriptDebugServer.h
	M	WebCore/inspector/JavaScriptProfile.cpp
	M	WebCore/inspector/JavaScriptProfile.h
	M	WebCore/inspector/JavaScriptProfileNode.cpp
	M	WebCore/inspector/JavaScriptProfileNode.h
	M	WebCore/page/Console.cpp
	M	WebCore/page/Console.h
	M	WebCore/page/Console.idl
Committed r43761