831864 - Refactor some logic to a renderSymbolicatedStacks method

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: patch

This is similar to bug 831748, but for the the symbolicated stack, not the raw one.

The main difference to bug 831748 is that right now only the hangs section uses the symbolicated stacks functionality, I am in the process of adding that to the late writes section too.

This patch also changes ChromeHangs_renderHangHeader to take only an index, which allows it to be used as a callback directly, simplifying the code a bit more.

Comment 1

[Nathan Froyd [:froydnj]]

Comment on attachment 703423 [details] [diff] [review]
patch

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

::: toolkit/content/aboutTelemetry.js
@@ +353,5 @@
>      let titleElement = document.createElement("span");
>      titleElement.className = "hang-title";
>  
> +    let hangs = Telemetry.chromeHangs;
> +    let durations = hangs.durations;

Can't you use this.hangs.durations instead?

Comment 2

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce25418d00a8

I used  Telemetry.hangs.durations. To use 'this' I would have to call bind in the two callers just for that.

Comment 3

[Vladan Djeric (:vladan)]

Comment on attachment 703423 [details] [diff] [review]
patch

>diff --git a/toolkit/content/aboutTelemetry.js b/toolkit/content/aboutTelemetry.js
>index 8b47821..6131719 100644
>--- a/toolkit/content/aboutTelemetry.js
>+++ b/toolkit/content/aboutTelemetry.js
>@@ -285,6 +285,42 @@ let StackRenderer = {
>       aRenderHeader(i);
>       this.renderStack(div, stack)
>     }
>+  },
>+  renderSymbolicatedStacks:
>+    function StackRenderer_renderSymbolicatedStacks(aPrefix, aRequest,
>+                                                    aRenderHeader) {
>+    if (aRequest.readyState != 4)
>+      return;

Why is StackRenderer.renderSymbolicatedStacks checking status codes of HTTP requests?

Comment 4

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

> Why is StackRenderer.renderSymbolicatedStacks checking status codes of HTTP
> requests?

Because it only needs to do anything when it gets the final answer and because it needs to display an error if the connection has one.

All this logic is sharable and we should share it. I can change the name to something more descriptive, but I can't think of anything more succinct than handleNetworkEventsAndThenDisplayErrorOrRenderStacksAsApppropriate :-)

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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