[webkit-reviews] review granted: [Bug 21833] Put Web Inspector's JS debugger hooks under ENABLE(JAVASCRIPT_DEBUGGER) : [Attachment 24605] patch v1

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Peter Kasting
<pkasting at google.com>'s request for review:
Bug 21833: Put Web Inspector's JS debugger hooks under
ENABLE(JAVASCRIPT_DEBUGGER)
https://bugs.webkit.org/show_bug.cgi?id=21833

Attachment 24605: patch v1
https://bugs.webkit.org/attachment.cgi?id=24605&action=edit

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> @@ -1114,12 +1125,16 @@ void InspectorController::setWindowVisib
>	   populateScriptObjects();
>	   if (m_nodeToFocus)
>	       focusNode();
> +#if ENABLE(JAVASCRIPT_DEBUGGER)
>	   if (m_attachDebuggerWhenShown)
>	       startDebugging();
> +#endif
>	   if (m_showAfterVisible != CurrentPanel)
>	       showPanel(m_showAfterVisible);
>      } else {
> +#if ENABLE(JAVASCRIPT_DEBUGGER)
>	   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.

r=me