1597997 - Some JSVM coverage reports have functions with wrong names (all "top-level")

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

See https://phabricator.services.mozilla.com/D53962#1640472
The ConsoleTable component is tested through devtools/client/webconsole/test/browser/browser_webconsole_console_table.js

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:Sylvestre, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Marco,
Could you please have a look to this and set the right priority?

Comment 3

[Marco Castelluccio [:marco]]

It looks like it is covered now: https://coverage.moz.tools/#revision=latest&path=devtools%2Fclient%2Fwebconsole%2Fcomponents%2FOutput%2FConsoleTable.js&view=file.

Maybe there was some intermittent issue that caused it not to be covered in that test run.

Comment 4

[Marco Castelluccio [:marco]]

Actually, it is still in the zero coverage report, even though some functions are covered. I'll investigate.

Comment 5

[Marco Castelluccio [:marco]]

Attachment: jsvm.info

I've analyzed the coverage artifacts for 0016ade97e65a470bc97521ab8338210e74cdd02.

I looked for a coverage artifact where line https://hg.mozilla.org/mozilla-central/file/0016ade97e65a470bc97521ab8338210e74cdd02/devtools/client/webconsole/components/Output/ConsoleTable.js#l83 was covered, but its encapsulating function wasn't.

I found linux_mochitest-devtools-chrome-16_code-coverage-jsvm.zip.

The relevant part of the INFO file is attached.

:nbp, any idea what could be causing this?

Comment 6

[Nicolas B. Pierron [:nbp]]

This is likely a problem related to this function:
https://searchfox.org/mozilla-central/rev/c61720a7d0c094d772059f9d6a7844eb7619f107/js/src/vm/CodeCoverage.cpp#519-531

Ted can you have a look?

Comment 7

[Ted Campbell [:tcampbell]]

This is a regression from Bug 1585372. I had moved the name capturing from finalization to script creation, but functions don't always have their name initialized yet. This means that cases like var x = function() {}; no longer work properly. I'll see what I can do.

Comment 8

[Ted Campbell [:tcampbell]]

The intermittency is due off-thread parse and main-thread parse capturing the name at different times. I'll fix the frontend to be more consistent here.

I also opened Bug 1606960 to close the testing gap between what the jit-tests test and the browser case. Hopefully we can avoid more regressions and improve the reliability of the coverage data.

Comment 9

[Ted Campbell [:tcampbell]]

Attachment: Bug 1597997 - Initialize inferred function name before script. r?jorendorff

Move the parser's calls to setFunName to before generating the actual script
for the function. This allows code-coverage to save the correct initial
function names when they are inferred. Off-thread parsing defers computing
the coverage metadata so it already has the right result.

Depends on D58668

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11fac2b6352f
Initialize inferred function name before script. r=jorendorff

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/11fac2b6352f

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to Beta for Fx73? Please nominate if so.

Comment 13

[Ted Campbell [:tcampbell]]

This is primarily for our own devs so I don't think we should uplift. The patch does have some small risk too.