1150514 - In about:performance, jank level 10 is displayed as black instead of red

Status: RESOLVED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

This is due to a typo in aboutPerformance.xhtml, in which we define two styles for `jank9`, instead of one for `jank9` and one for `jank10`.

The problem appears in http://mxr.mozilla.org/mozilla-central/source/toolkit/components/aboutperformance/content/aboutPerformance.xhtml#71

The class `jank9` at line 71 should be replaced with `jank10`.

Comment 1

[wrahman0]

I will take a look at this.

Comment 2

[wrahman0]

How can I go about testing my solution?

Comment 3

[wrahman0]

Repeated message. Forgot to mark need more info.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

The first step is to build Firefox. Have you done that already?

Once you have built Firefox, the simplest way is probably to:
1. Open about:performance;
2. Open the web console;
3. In the webconsole, write the following:
  for (var start = Date.now(); Date.now() - start < 2000; Date.now());
4. Look quickly :)

This will cause the webconsole to use way too much CPU, which should result in a jank level of 10.

Comment 5

[wrahman0]

Yeah, I have already built Firefox. I just didn't know how to see this color. It makes sense and seems to be a very simple fix. I will get it up by the end of the day. 

Could you assign me this bug so it appears under "My bugs"?

Comment 6

[David Teller [:Yoric] - still alive but not very active]

The general policy is to wait until the first patch to assign a bug.

Comment 7

[wrahman0]

So I was running the about:performance page and it seems like none of the jank * are colored. Any idea?

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Best guess: there may be a syntax error in your CSS. If you wish, you can attach your patch so that I can take a look.

Comment 9

[wrahman0]

Attachment: bug1150514_jank9_tojank10.diff

jank10 is appearing red now. It would be more prominent if all of the jank* were bold. Do you think I should bold all the jank numbers as well?

Comment 10

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8588483 [details] [diff] [review]
bug1150514_jank9_tojank10.diff

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

Looks good to me.
Making jank level bold is a good idea. Do you want to submit a patch that does both?

Comment 11

[wrahman0]

Yeah Ill submit a patch for both later tonight.

Comment 12

[wrahman0]

Attachment: bug1150514_jank9_tojank10.diff

Added bolded jank text.

Comment 13

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8590589 [details] [diff] [review]
bug1150514_jank9_tojank10.diff

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

This looks good.
Could you change your commit message as follows, before we can land the patch?

Bug 1150514 - in about:performance, fixing CSS of jank levels;r=Yoric

Comment 14

[wrahman0]

Sure, I'm not currently at near by laptop so expect the change later tonight.

Comment 15

[wrahman0]

Attachment: bug1150514 - in about:performance, fixing CSS of jank levels;r=Yoric

How does that look? Let me know if you want anything else changed.

Comment 16

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8591181 [details] [diff] [review]
bug1150514 - in about:performance, fixing CSS of jank levels;r=Yoric

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

I'm afraid that your patch still doesn't have a commit message.

If you are using mq, you can set the commit message by doing
 hg qref -m "Bug 1150514 - in about:performance, fixing CSS of jank levels;r=Yoric"
if you are using heads, you can set the commit message by doing
 hg commit --amend -m "Bug 1150514 - in about:performance, fixing CSS of jank levels;r=Yoric"

Comment 17

[wrahman0]

Oh, I see. What I did was set the message when I was submitting the patch on bugzilla. I am using mercurial, so I will do what you mentioned above. Sorry about the that.

Comment 18

[wrahman0]

Attachment: bug1150514_fixing_css_for_jank_levels.diff

Updated the patch message to be "Bug 1150514 - in about:performance, fixing CSS of jank levels;r=Yoric" as per request.

Comment 19

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8592040 [details] [diff] [review]
bug1150514_fixing_css_for_jank_levels.diff

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

Yep, looks good.

Comment 20

[David Teller [:Yoric] - still alive but not very active]

Let's land this.

Comment 21

[wrahman0]

Is there anything else required from my end before this lands?

Comment 22

[David Teller [:Yoric] - still alive but not very active]

Nope, this is going to land within a few hours.

Comment 23

[David Teller [:Yoric] - still alive but not very active]

Next step is picking another bug :)

By the way, if you haven't opened an account on mozillians.org, you can do it now. I'll vouch for you.

Comment 24

[wrahman0]

Yeah, I was thinking about focusing on the about:performance side of things. I will be sending a patch later today about a temporary fix for the refresh rate of that page. It should be increased from 2000 ms to probably 10000 ms. This fix should do until we land Bug 1150863. 

Also, thanks for the vouch offer! Really motivates me to keep working hard here! Here is my profile info: https://mozillians.org/en-US/u/wrahman0/

Comment 25

[Carsten Book [:Tomcat]]

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

Comment 26

[Carsten Book [:Tomcat]]

(In reply to Carsten Book [:Tomcat] from comment #25)
> https://hg.mozilla.org/integration/fx-team/rev/58c91976be46

also thanks for contributing to mozilla!

Comment 27

[wrahman0]

Glad to help :D

Comment 28

[David Teller [:Yoric] - still alive but not very active]

(vouched)

Comment 29

[Ekanan Ketunuti]

https://hg.mozilla.org/mozilla-central/rev/58c91976be46