[webkit-reviews] review granted: [Bug 26145] WebInspector halts when opening with debugger enabled globally : [Attachment 30909] fix

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has granted Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 26145: WebInspector halts when opening with debugger enabled globally
https://bugs.webkit.org/show_bug.cgi?id=26145

Attachment 30909: fix
https://bugs.webkit.org/attachment.cgi?id=30909&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> -	       enableDebugger();
> +	       m_frontend->attachDebuggerWhenShown();

Can't you just leave this calling enableDebugger() since you fixed that
function?

>	   disableDebugger();
> +	   m_attachDebuggerWhenShown = true;

Why is this needed? Shoudln't this be m_attachDebuggerWhenShown = false? If it
is supose to be false, should disableDebugger() set it to false? If it isn't
those things, maybe a comment is needed to explain.

> -	   enableDebugger();
> +	   m_attachDebuggerWhenShown = true;

Same as before. Can't you just leave this calling enableDebugger() since you
fixed that function?

> +	       this._attachDebuggerWhenShown = false;

I like to use delete this._attachDebuggerWhenShown for cases like this, since
there is no benifit to keeping this property around.

r+, but maybe this can be a smaller change based on the comments above.