1427484 - The "Show raw stack data" in Browser hangs in about:telemetry does not work

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P2)

Description

[Jeremy Lempereur [:o0ignition0o]]

Attachment: chrome-hangs.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20170926100259

Steps to reproduce:

In the about:telemetry page, go to the Browser Hangs section.
Click on the "Fetch function names for stacks" link, and click on the "Show raw stack data" button. 


Actual results:

As I clicked on the "Show raw stack data" button, the link did not change, and the Browser Hangs section (on the left side) vanished.

My console reported : ` JavaScript error: , line 0: uncaught exception: TypeError: element.argument is undefined ` 


Expected results:

The link should display "Fetch function names for stacks" again, and the section should still be displayed on the navigation menu.

Comment 1

[Jeremy Lempereur [:o0ignition0o]]

Attachment: Bug 1427484 - Fix about:telemetry "Show raw stack data" button in Browser Hangs section.

The "Show raw stack data" button in the Browser Hangs section produced nothing but removing the "Browser Hangs" section from the about:telemetry navigation menu. I looked at the way the Late Writes section works and patched the Chrome Hangs render calls accordingly.

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

Comment 2

[Chris H-C :chutten]

Comment on attachment 8939253 [details]
Bug 1427484 - Fix about:telemetry "Show raw stack data" button in Browser Hangs section.

https://reviewboard.mozilla.org/r/209682/#review215432

Solid patch, but I think we'll run into a problem with non-current ping data from non-main pings.

::: toolkit/content/aboutTelemetry.js:2055
(Diff revision 1)
>      function() {
>        if (!gPingData) {
>          return;
>        }
>  
> -      ChromeHangs.render(gPingData);
> +      ChromeHangs.render(gPingData.payload.chromeHangs);

chromeHangs may not be present in each gPingData. For instance, a `health` ping does not contain chromeHangs.

Does this patch work when you select a non-main (and non-saved-session) archived ping from the ping selector at the top-left?

Comment 3

[Jeremy Lempereur [:o0ignition0o]]

Hi Chris, and thanks for your response :)
I'll try to reproduce this setup and handle it tonight :)

Comment 4

[Jeremy Lempereur [:o0ignition0o]]

Comment on attachment 8939253 [details]
Bug 1427484 - Fix about:telemetry "Show raw stack data" button in Browser Hangs section.

https://reviewboard.mozilla.org/r/209682/#review215432

> chromeHangs may not be present in each gPingData. For instance, a `health` ping does not contain chromeHangs.
> 
> Does this patch work when you select a non-main (and non-saved-session) archived ping from the ping selector at the top-left?

After a bit more investigations, I think a TypeError would be raised if and only if gPingData.payload was undefined. 
If a ping has no chromeHangs (but a payload) The render function should catch it and return early. 

I'm not confident enough with the codebase to claim that each ping has a payload (although it seems legitimate).

I could add an other check on the payload before calling the render function, but we should probably do the same in the LateWrites section too I guess? 

I'm going to enable archiving as displayed in https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/concepts/archiving.html and check how the page behaves :)

Comment 5

[Jeremy Lempereur [:o0ignition0o]]

Comment on attachment 8939253 [details]
Bug 1427484 - Fix about:telemetry "Show raw stack data" button in Browser Hangs section.

https://reviewboard.mozilla.org/r/209682/#review215432

> After a bit more investigations, I think a TypeError would be raised if and only if gPingData.payload was undefined. 
> If a ping has no chromeHangs (but a payload) The render function should catch it and return early. 
> 
> I'm not confident enough with the codebase to claim that each ping has a payload (although it seems legitimate).
> 
> I could add an other check on the payload before calling the render function, but we should probably do the same in the LateWrites section too I guess? 
> 
> I'm going to enable archiving as displayed in https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/concepts/archiving.html and check how the page behaves :)

I just tried to watch what happens with non-main and non-saved-session pings and there is just no Chrome Hangs menu being displayed, and I can't find any error in the console :/

Comment 6

[Chris H-C :chutten]

Comment on attachment 8939253 [details]
Bug 1427484 - Fix about:telemetry "Show raw stack data" button in Browser Hangs section.

https://reviewboard.mozilla.org/r/209682/#review215554

Good enough for me :)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/71b4e1ee7d99
Fix about:telemetry "Show raw stack data" button in Browser Hangs section. r=chutten

Comment 8

[Jeremy Lempereur [:o0ignition0o]]

Great thank's a lot ! Could you please vouch for me? I want to be a Mozillian too :) I'm https://mozillians.org/en-US/u/ignition/

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/71b4e1ee7d99

Comment 10

[Chris H-C :chutten]

The Mozillians form asks me if you've made "significant contributions". Cleaning up about:telemetry is a good start, but we should get you a couple of heavier pieces of work before we start the vouching parade :)

What do you think about bug 1249596 ?

Comment 11

[Jeremy Lempereur [:o0ignition0o]]

Sure, I'm taking it ! :D