Closed Bug 1253125 Opened 5 years ago Closed 5 years ago
[telemetry] Toolbox panels should not call tool
58 bytes, text/x-review-board-request
Several panels apparently call toolOpened/Closed themselves, but they don't need to do that! The toolbox calls it for you. Only things outside of the toolbox should need to call toolOpened manually. This was spotted by QA in bug 1247985. At least the following are affected: > DEVTOOLS_SHADEREDITOR_OPENED_COUNT > DEVTOOLS_CANVASDEBUGGER_OPENED_COUNT > DEVTOOLS_MEMORY_OPENED_COUNT > DEVTOOLS_STORAGE_OPENED_COUNT > DEVTOOLS_WEBAUDIOEDITOR_OPENED_COUNT > DEVTOOLS_SCRATCHPAD_OPENED_COUNT Unfortunately, this means our opened counts and active times for these tools are not correct. :(
QA Contact: mihai.boldan
Priority: -- → P1
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=53940cc0b217 This run is noisy from a bad parent rev, but none of the failures seem related to the change here.
Review commit: https://reviewboard.mozilla.org/r/37925/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/37925/
Comment on attachment 8726332 [details] MozReview Request: Bug 1253125 - Stop duplicating telemetry from inside tools. r=mratcliffe https://reviewboard.mozilla.org/r/37925/#review34543 ::: devtools/client/scratchpad/scratchpad.js (Diff revision 1) > -telemetry.toolOpened("scratchpad"); Scratchpad can be launched from the Web Developer menu or inside the toolbox. We need to find a way to only call telemetry.toolOpened and telemetry.toolClosed when we are *not* in the toolbox. Another option would be to stop automatically sending pings for main toolbox panels... the advantage being that it would be less confusing. Your choice.
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #3) > Comment on attachment 8726332 [details] > MozReview Request: Bug 1253125 - Stop duplicating telemetry from inside > tools. r=mratcliffe > > https://reviewboard.mozilla.org/r/37925/#review34543 > > ::: devtools/client/scratchpad/scratchpad.js > (Diff revision 1) > > -telemetry.toolOpened("scratchpad"); > > Scratchpad can be launched from the Web Developer menu or inside the toolbox. The current patch will record `toolOpened("scratchpad")` for the toolbox panel, and `toolOpened("scratchpad-window")` for the separate. Is this sufficient, or are you saying `toolOpened("scratchpad")` should be record for both cases?
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4) > `toolOpened("scratchpad-window")` for the separate. for the separate *window*.
Comment on attachment 8726332 [details] MozReview Request: Bug 1253125 - Stop duplicating telemetry from inside tools. r=mratcliffe https://reviewboard.mozilla.org/r/37925/#review34545 In that case... ship it ;)
Attachment #8726332 - Flags: review+
I reproduced this issue on Firefox 47.0a1 (2016-03-01) and on Windows 10 x64. The issue is no longer reproducible on Firefox 47.0b3. Tests were performed on Windows 10 x64 OS and by using the affected counters from Comment 0. I am marking this issue Verified Fixed.
You need to log in before you can comment on or make changes to this bug.