Closed Bug 964418 Opened 6 years ago Closed 6 years ago

Update mochiperf tests to use dom utils frame time recording

Categories

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

x86_64
Windows 8.1
defect

Tracking

(firefox28 fixed, firefox29 fixed)

RESOLVED FIXED
Firefox 29
Tracking Status
firefox28 --- fixed
firefox29 --- fixed

People

(Reporter: jimm, Assigned: jimm)

References

Details

(Whiteboard: [feature] p=1 [qa-])

Attachments

(2 files)

No description provided.
Attached patch patchSplinter Review
Also added a couple apzc tests. I'm also going to point the graphs page at mc vs inbound and clear the data file.
Attachment #8366152 - Flags: review?(mbrubeck)
Hey Jim, are you taking this bug for IT#23?  If so, can you provide a point value.  Thanks.
Flags: needinfo?(jmathies)
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);
Attachment #8366152 - Flags: review?(mbrubeck) → review+
> "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.
Flags: needinfo?(jmathies)
Whiteboard: p=1
Blocks: metrov1it23
Status: NEW → ASSIGNED
Priority: -- → P2
QA Contact: jbecerra
Whiteboard: p=1 → [feature] p=1
https://hg.mozilla.org/mozilla-central/rev/598d1bac661d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Attached patch new testsSplinter Review
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.
Attachment #8366638 - Flags: review?(mbrubeck)
Attachment #8366638 - Flags: review?(mbrubeck) → review+
Could you please give some guidance in order for the QA to verify this? Thanks!
Flags: needinfo?(jmathies)
(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.
Flags: needinfo?(jmathies)
Thanks, Jim! Marking this [qa-].
Whiteboard: [feature] p=1 → [feature] p=1 [qa-]
You need to log in before you can comment on or make changes to this bug.