Closed Bug 1133900 Opened 5 years ago Closed 5 years ago

Only collect JS telemetry for web content (not add-on or chrome JS)

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

Details

Attachments

(1 file)

The telemetry probe in bug 1130123 was adding console.whatever() calls (where "whatever" is an unknown property) in .html files to our __noSuchMethod__ telemetry. Even though this is likely a content bug, we don't want to count these calls as __noSuchMethod__ uses.

This patch adds back the "http" filename check. Our Nightly 38 telemetry counts have been polluted, but next week's Aurora 38 telemetry should be accurate.
Attachment #8565546 - Flags: review?(jdemooij)
Comment on attachment 8565546 [details] [diff] [review]
http-telemetry.patch

Review of attachment 8565546 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, r=me with nits below addressed.

::: js/src/frontend/Parser.cpp
@@ +8383,5 @@
>  {
>      JSContext* cx = context->maybeJSContext();
>      if (!cx)
>          return;
> +    const char* filename = getFilename();

Nit: SM style is const char *filename

::: js/src/jscompartment.cpp
@@ +860,5 @@
> +void
> +JSCompartment::addTelemetry(const char* filename, DeprecatedLanguageExtension e)
> +{
> +    // Only report telemetry for web content, not add-ons or chrome JS.
> +    if (addonId || isSystem || !filename || strcmp(filename, "http") != 0)

This strcmp will always return non-zero because it compares the full filename. I think you want strncmp (we also used this function before the refactoring):

strncmp(filename, "http", 4) != 0

::: js/src/jscompartment.h
@@ +557,5 @@
>  
>      void reportTelemetry();
>  
>    public:
> +    void addTelemetry(const char* filename, DeprecatedLanguageExtension e);

Nit: * before the name here too, also in the cpp file.
Attachment #8565546 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/d563f8e2d157
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
See Also: → 1211164
You need to log in before you can comment on or make changes to this bug.