Bug 199184 – Web Inspector: Implement console.timeLog

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

Implement console.timeLog

https://developer.mozilla.org/en-US/docs/Web/API/Console/timeLog
https://console.spec.whatwg.org/#timelog

[Joseph Pecoraro]

Created attachment 372818 [details]
[PATCH] Proposed Fix

[EWS Watchlist]

Attachment 372818 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/JSGlobalObjectConsoleClient.h:57:  The parameter name "arguments" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Devin Rousso]

Comment on attachment 372818 [details]
[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 🤔

[Joseph Pecoraro]

Comment on attachment 372818 [details]
[PATCH] Proposed Fix

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

>> 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 🤔

`this._extraParameters` sometimes gets cleared based on what has been read. It is kind of tricky given how different console messages are sent across.

[Joseph Pecoraro]

Created attachment 372843 [details]
[PATCH] For Landing

[WebKit Commit Bot]

Comment on attachment 372843 [details]
[PATCH] For Landing

Clearing flags on attachment: 372843

Committed r246798: <https://trac.webkit.org/changeset/246798>

[Radar WebKit Bug Importer]

<rdar://problem/52218904>

[Yury Semikhatsky]

*** Bug 186833 has been marked as a duplicate of this bug. ***