Bug 176142 - Speedometer 2.0: Add dummy node to notify app is ready for Backbone suite
Summary: Speedometer 2.0: Add dummy node to notify app is ready for Backbone suite
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 172339
  Show dependency treegraph
 
Reported: 2017-08-30 20:16 PDT by Shiyu Zhang
Modified: 2017-09-27 12:47 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.37 KB, patch)
2017-08-30 20:32 PDT, Shiyu Zhang
no flags Details | Formatted Diff | Diff
Patch (3.09 KB, patch)
2017-09-03 20:41 PDT, Shiyu Zhang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shiyu Zhang 2017-08-30 20:16:58 PDT
There's a chance for BackboneJS suite that Speedometer attempted to add items before app.js was loaded. The deleteButtons.length in resources/test.js for BackboneJS suite could be zero sometimes as I observed. What's worse, BackboneJS suite will just skip complete/delete-items steps and never crash if checkboxes/deleteButtons.length is zero, which makes this issue difficult to be detected.
Comment 1 Shiyu Zhang 2017-08-30 20:32:42 PDT
Created attachment 319437 [details]
Patch
Comment 2 Shiyu Zhang 2017-08-30 20:38:19 PDT
Comment on attachment 319437 [details]
Patch

A patch to fix this bug, PTAL.
Comment 3 Ryosuke Niwa 2017-09-01 11:42:43 PDT
Comment on attachment 319437 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319437&action=review

> PerformanceTests/Speedometer/resources/tests.js:233
> +            var newTodo = contentDocument.getElementsByClassName('new-todo');
> +            newTodo[0].focus();
> +            return newTodo[0];

I think it's cleaner to use querySelector here.
Comment 4 Ryosuke Niwa 2017-09-01 11:44:08 PDT
Comment on attachment 319437 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319437&action=review

>> PerformanceTests/Speedometer/resources/tests.js:233
>> +            return newTodo[0];
> 
> I think it's cleaner to use querySelector here.

Can we also replace checkboxes.length and deleteButtons.length by numberOfItemsToAdd below
so that we'd see a console error should this bug come back?
Comment 5 Shiyu Zhang 2017-09-03 20:41:22 PDT
Created attachment 319831 [details]
Patch
Comment 6 Shiyu Zhang 2017-09-03 20:45:05 PDT
Comment on attachment 319831 [details]
Patch

Thanks for your advice! A new patch following your suggestion is here.
Comment 7 WebKit Commit Bot 2017-09-05 01:17:29 PDT
Comment on attachment 319831 [details]
Patch

Clearing flags on attachment: 319831

Committed r221611: <http://trac.webkit.org/changeset/221611>
Comment 8 WebKit Commit Bot 2017-09-05 01:17:30 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2017-09-27 12:47:54 PDT
<rdar://problem/34694038>