964418 - Update mochiperf tests to use dom utils frame time recording

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Tests, defect, P2)

Comment 1

[Jim Mathies [:jimm]]

Attachment: patch

Also added a couple apzc tests. I'm also going to point the graphs page at mc vs inbound and clear the data file.

Comment 2

[Marco Mucci [:MarcoM]]

Hey Jim, are you taking this bug for IT#23?  If so, can you provide a point value.  Thanks.

Comment 3

[Matt Brubeck (:mbrubeck)]

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

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

::: browser/metro/base/tests/mochiperf/res/ripples.html
@@ +230,5 @@
>      progress = timestamp - start;
>      ripples.run(progress);
>      var time = new Date();
>      var diff = time.getTime() - now.getTime();
> +    if (diff < 5000) { // ten seconds

nit: out of date comment

::: browser/metro/base/tests/mochitest/head.js
@@ +223,5 @@
> +function hideNavBar()
> +{
> +  let promise = waitForEvent(Elements.navbar, "transitionend");
> +  if (ContextUI.navbarVisible) {
> +    ContextUI.dismissNavbar();

"let promise = waitForEvent..." should be moved inside the "if" statement.

@@ +226,5 @@
> +  if (ContextUI.navbarVisible) {
> +    ContextUI.dismissNavbar();
> +    return promise;
> +  }
> +}

In the case where the navbar is *not* visible, this should return an immediately-resolved promise instead of returning nothing (to prevent callers from failing unpredictably if they expect a promise).  For example, you could add this at the end of the function:

  return Promise.resolve(null);

Comment 4

[Jim Mathies [:jimm]]

> "let promise = waitForEvent..." should be moved inside the "if" statement.
> 
> @@ +226,5 @@
> > +  if (ContextUI.navbarVisible) {
> > +    ContextUI.dismissNavbar();
> > +    return promise;
> > +  }
> > +}
> 
> In the case where the navbar is *not* visible, this should return an
> immediately-resolved promise instead of returning nothing (to prevent
> callers from failing unpredictably if they expect a promise).  For example,
> you could add this at the end of the function:
> 
>   return Promise.resolve(null);

Thanks. I updated both show and hide. Will push to try first.

Comment 5

[Jim Mathies [:jimm]]

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

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/598d1bac661d

Comment 7

[Jim Mathies [:jimm]]

Attachment: new tests

Adding a couple tests and tweaking a few values. 

I also did some testing with these tweaking various apzc prefs, specifically the repaint_interval values. Generally I don't see a big difference there as long as the interval is within a band of values (25 -> 70 msec). So I don't think those prefs play much of a role in redraw performance. At least not on my surface pro.

Comment 8

[Jim Mathies [:jimm]]

https://tbpl.mozilla.org/?tree=Try&rev=7ba40832c61e
https://hg.mozilla.org/integration/mozilla-inbound/rev/57b0e214f693

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Landed on Aurora with Matt's blessing as a bustage fix for bug 913609.
https://hg.mozilla.org/releases/mozilla-aurora/rev/0a8bb4f60f03

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/57b0e214f693

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/555697fdea38

Comment 12

[Manuela Muntean [Away]]

Could you please give some guidance in order for the QA to verify this? Thanks!

Comment 13

[Jim Mathies [:jimm]]

(In reply to Manuela Muntean [:Manuela] [QA] from comment #12)
> Could you please give some guidance in order for the QA to verify this?
> Thanks!

nothing to verify here, these tests are working.

Comment 14

[Manuela Muntean [Away]]

Thanks, Jim! Marking this [qa-].