Bug 21833 - Put Web Inspector's JS debugger hooks under ENABLE(JAVASCRIPT_DEBUGGER)
Summary: Put Web Inspector's JS debugger hooks under ENABLE(JAVASCRIPT_DEBUGGER)
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Peter Kasting
Depends on:
Reported: 2008-10-23 11:31 PDT by Peter Kasting
Modified: 2008-10-23 12:38 PDT (History)
0 users

See Also:

patch v1 (12.28 KB, patch)
2008-10-23 11:48 PDT, Peter Kasting
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Kasting 2008-10-23 11:31:13 PDT
In the Chromium port, we can't yet use the JavaScript Debugger.  To obviate the need to fork files, I plan to place the hooks to support the debugger under ENABLE(JAVASCRIPT_DEBUGGER), which will be on by default.  This flag can go away eventually once we can support the debugger in our port.
Comment 1 Peter Kasting 2008-10-23 11:48:35 PDT
Created attachment 24605 [details]
patch v1

Notable points to review:
* Make sure I didn't erroneously cover any non-debugger functions.
* If there are more helper functions, #includes, etc. that can also go under this #if, I may have missed them; feel free to point that out.
Comment 2 Adam Roben (:aroben) 2008-10-23 11:56:05 PDT
Comment on attachment 24605 [details]
patch v1

> @@ -1114,12 +1125,16 @@ void InspectorController::setWindowVisib
>          populateScriptObjects();
>          if (m_nodeToFocus)
>              focusNode();
>          if (m_attachDebuggerWhenShown)
>              startDebugging();
> +#endif
>          if (m_showAfterVisible != CurrentPanel)
>              showPanel(m_showAfterVisible);
>      } else {
>          stopDebugging();
> +#endif

I wonder if it would be better to make startDebugging() and stopDebugging() (and probably some of the other InspectorController functions) be no-ops when the debugger is disabled. That would allow us to add fewer #ifdefs, I think. But I guess that's inconsistent with how we normally use the ENABLE() macro.

Comment 3 Peter Kasting 2008-10-23 12:38:11 PDT
Fixed in r37820.