[Webkit-unassigned] [Bug 199184] Web Inspector: Implement console.timeLog

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=199184

Devin Rousso <drousso at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #372818|review?                     |review+
              Flags|                            |

--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 372818
  --> https://bugs.webkit.org/attachment.cgi?id=372818
[PATCH] Proposed Fix

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

r=me, nice!

> LayoutTests/inspector/console/console-time.html:17
>          name: "DefaultLabel",

Style: we usually prefix all test case names with the name of the suite: `console.time.DefaultLabel`.

(repeated below)

> LayoutTests/inspector/console/console-time.html:21
> +            const expected = 6;

Might be worth a comment saying that `console.time()` doesn't log anything, so this is the count of `console.timeLog` and `console.timeEnd`.

(repeated below)

> Source/JavaScriptCore/inspector/agents/InspectorConsoleAgent.cpp:149
> +    if (!m_injectedScriptManager.inspectorEnvironment().developerExtrasEnabled())

NIT: should we put this inside a `LIKELY`?

> Source/JavaScriptCore/runtime/ConsoleObject.cpp:322
> +    ConsoleClient* client = exec->lexicalGlobalObject()->consoleClient();

Style: `auto*`?

> Source/WebCore/inspector/InspectorInstrumentation.cpp:891
> +    if (!instrumentingAgents.inspectorEnvironment().developerExtrasEnabled())

NIT: should we put this inside a `LIKELY`?

> Source/WebCore/inspector/InspectorInstrumentation.cpp:894
> +    if (WebConsoleAgent* consoleAgent = instrumentingAgents.webConsoleAgent())

Style: `auto*`?

> Source/WebCore/inspector/InspectorInstrumentation.h:1416
> +    if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForFrame(frame))

Style: `auto*`?

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:293
> +                if (this._extraParameters)

IIUC, `this._extraParameters` is equal to `this._message.parameters`?

At some point we should probably update the other users of `this._message.parameters` in this class to use `this._extraParameters` instead 🤔

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190625/698b7b7d/attachment.html>