Closed
Bug 1210733
Opened 9 years ago
Closed 9 years ago
JS_CODE_COVERAGE_OUTPUT_DIR returns too few sources with no empty code coverage when running firefox.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: nbp, Assigned: nbp)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
6.30 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
Bug 1204554 works fine in the JS shell, but it does seems that only a few functions are being reported, and that among most of the functions being reported, the code coverage information got deleted before the serialization phase. Apparently, one of the problem is that the source does not seems to be finalized first in the browser. Which cause a lot of JSScript to be finalized without having any attached reports.
Assignee | ||
Comment 1•9 years ago
|
||
This patch modify the code such that it works even if the source objects are finalized after the scripts. It works by always adding the source object or reusing the existing one if it is already present.
Attachment #8668899 -
Flags: review?(terrence)
Comment 2•9 years ago
|
||
Comment on attachment 8668899 [details] [diff] [review] Record source filenames independently of the script coverage. Review of attachment 8668899 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/CodeCoverage.cpp @@ +349,5 @@ > // Skip any operation if we already some out-of memory issues. > if (outTN_.hadOutOfMemory()) > return; > > + // Lookup if there is already one entry. This comment is either extremely ambiguous or flat-out disagrees with the source below it. The most probable meaning of this sentence, taken alone, is: "do a lookup if and only if exactly one entry exists". I could also generously interpret this sentence as saying: "lookup whether one and exactly one entry exists". But that also doesn't look right. Maybe you mean: "Get the existing source annotations, or create a new one." @@ +354,5 @@ > + LCovSource* source = lookupOrAdd(comp, sso); > + if (!source) > + return; > + > + // Write code coverage data into the allocated LCovSource. s/allocated// I don't think "allocated" adds anything here, given the null check above.
Attachment #8668899 -
Flags: review?(terrence) → review+
Comment 4•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8966326bc731
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in
before you can comment on or make changes to this bug.
Description
•