1168726 - Use performange.getEntriesByType instead of getEntries in test if there is no clear reason

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

Once the Frame Timing API has landed, performance.getEntries() contains Frame Timing entries. So if there is no clear reason to use getEntries, we should use getEntriesByType instead of getEntries.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix

:baku, could you please review this?

I actually should have noticed this issue when I wrote test for bug 1158731.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=27c084b530ab

Comment 2

[Andrea Marchesini [:baku]]

Comment on attachment 8611040 [details] [diff] [review]
Fix

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

::: dom/tests/mochitest/general/resource_timing_iframe.html
@@ +31,5 @@
>      "http://mochi.test:8888/tests/dom/tests/mochitest/general/resource_timing_iframe.html").length,
>      "This iframe should NOT contain itself as an entry");
>  
>    // Check that there are no iframes added as entries
> +  for (var i = 0 ; i < window.performance.getEntriesByType("resource").length ; i++) {

just because window.performance.getEntriesByType("resource") != window.performance.getEntriesByType("resource")

I think it's better if you do:

var resources = window.performance.getEntriesByType("resource");
for (var i = 0; i < resources.length; ++i) {
  var entry = resources[i];
  ...

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix v2

(In reply to Andrea Marchesini (:baku) from comment #2)
> ::: dom/tests/mochitest/general/resource_timing_iframe.html
> @@ +31,5 @@
> >      "http://mochi.test:8888/tests/dom/tests/mochitest/general/resource_timing_iframe.html").length,
> >      "This iframe should NOT contain itself as an entry");
> >  
> >    // Check that there are no iframes added as entries
> > +  for (var i = 0 ; i < window.performance.getEntriesByType("resource").length ; i++) {
> 
> just because window.performance.getEntriesByType("resource") !=
> window.performance.getEntriesByType("resource")
> 
> I think it's better if you do:
> 
> var resources = window.performance.getEntriesByType("resource");
> for (var i = 0; i < resources.length; ++i) {
>   var entry = resources[i];
>   ...

Indeed! Thanks!

Carrying over review+, tested locally.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e5ae1ee8f29

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e5ae1ee8f29

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1e5ae1ee8f29