Bug 21833

Summary: Put Web Inspector's JS debugger hooks under ENABLE(JAVASCRIPT_DEBUGGER)
Product: WebKit Reporter: Peter Kasting <pkasting>
Component: Web Inspector (Deprecated)Assignee: Peter Kasting <pkasting>
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Description Flags
patch v1 aroben: review+

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.