Nicolas B. Pierron [:nbp] Assignee
	Description • 9 years ago

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.

	Nicolas B. Pierron [:nbp] 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.

	Terrence Cole [: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.

	Pulsebot
	Comment 3 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/8966326bc731

	Carsten Book [:Tomcat]
	Comment 4 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/8966326bc731