1253125 - [telemetry] Toolbox panels should not call toolOpened manually

Status: VERIFIED FIXED

(DevTools :: General, defect, P1)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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. :(

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1253125 - Stop duplicating telemetry from inside tools. r=mratcliffe

Review commit: https://reviewboard.mozilla.org/r/37925/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37925/

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

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.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(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?

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4)
> `toolOpened("scratchpad-window")` for the separate.

for the separate *window*.

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

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 ;)

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/ad2c21a89e6e

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ad2c21a89e6e

Comment 9

[Mihai Boldan, Desktop QA [:mboldan]]

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.