[webkit-reviews] review granted: [Bug 173698] Web Inspector: Pausing with a deep call stack can be very slow, avoid eagerly generating object previews : [Attachment 313593] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Matt Baker <mattbaker at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 173698: Web Inspector: Pausing with a deep call stack can be very slow,
avoid eagerly generating object previews
https://bugs.webkit.org/show_bug.cgi?id=173698

Attachment 313593: [PATCH] Proposed Fix

https://bugs.webkit.org/attachment.cgi?id=313593&action=review




--- Comment #6 from Matt Baker <mattbaker at apple.com> ---
Comment on attachment 313593
  --> https://bugs.webkit.org/attachment.cgi?id=313593
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=313593&action=review

Nice work, r=me with a few nits/comments.

> Source/JavaScriptCore/ChangeLog:10
> +	   the `thisObject` of each of the call frame's. In some cases, this
could be more

No apostrophe in frames.

> Source/JavaScriptCore/inspector/InjectedScriptSource.js:240
> +	   var object = this._objectForId(parsedObjectId);

Nit: The rest of the file still uses var, but why not use let for new code?

> Source/JavaScriptCore/inspector/InjectedScriptSource.js:475
> +	   var result = evalFunction.call(object, expression, commandLineAPI);

Nit: let

> Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp:170
> +    muteConsole();

This is such a common pattern in InspectorRuntimeAgent, I wonder if it would be
worth making into a RAII-style guard.