Closed
Bug 1333024
Opened 7 years ago
Closed 7 years ago
Scalars record even when not allowed in the process when using the C++ API
Categories
(Toolkit :: Telemetry, defect, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: Dexter, Assigned: Dexter)
References
Details
(Whiteboard: [measurement:client])
Attachments
(1 file)
17.88 KB,
patch
|
chutten
:
review+
|
Details | Diff | Splinter Review |
When using the C++ API to change scalars from child processes, we don't check if we're expected to record from the process [1]. This is happening for all the scalars operations, but it's not happening for the parent process, which looks ok as internal_GetRecordableScalar checks for that too [2]. [1] - http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/toolkit/components/telemetry/TelemetryScalar.cpp#1397 [2] - http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/toolkit/components/telemetry/TelemetryScalar.cpp#1031
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → alessio.placitelli
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•7 years ago
|
||
Chris, any chance you could review this? No rush though :) The background here is that the C++ API paths were not checking if we were allowed to record scalar data in the current process. The JS API was. Part of the problem was that the function checking for the "permissions" were different: I've refactored the code a bit to use the same code in both places. I've also filed bug 1333026 to extend the C++ API test coverage to content scalars as well.
Attachment #8829453 -
Flags: review?(chutten)
Comment 2•7 years ago
|
||
Comment on attachment 8829453 [details] [diff] [review] bug1333024.patch Review of attachment 8829453 [details] [diff] [review]: ----------------------------------------------------------------- I will be even happier when this is tested :)
Attachment #8829453 -
Flags: review?(chutten) → review+
Assignee | ||
Comment 3•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8d59ab73676
Pushed by alessio.placitelli@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4ab1592a1034 Don't record scalars with the C++ API if not allowed in the current process. r=chutten
Assignee | ||
Comment 5•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ab1592a10342f9ba513e0ffb3a04d1df67bd3aa Bug 1333024 - Don't record scalars with the C++ API if not allowed in the current process. r=chutten
Assignee | ||
Updated•7 years ago
|
Whiteboard: [measurement:client] → [measurement:client][measurement:client:uplift]
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4ab1592a1034
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Assignee | ||
Updated•7 years ago
|
Whiteboard: [measurement:client][measurement:client:uplift] → [measurement:client]
You need to log in
before you can comment on or make changes to this bug.
Description
•