Bug 176017 – Speedometer 2.0: jQuery test fails occasionally

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Shiyu Zhang]

Created attachment 319169 [details]
A possible fix for jQuery test

Speedometer 2.0 crashes at jQuery test with high possibility on my laptop (i5-5300U, win8, Chrome R60). 

Currently, jQuery test generates the DOM tree in its index.html and loads app.js, which binds events to the DOM nodes, at the end of the index.html. Speedometer framework starts "Adding items" action after the "new-todo" DOM node is ready. In jQuery test, there's a chance that Speedometer starts "Adding items" as soon as the "new-todo" node is ready while app.js has not been loaded completely and no event is binded to the DOM node yet. A possible fix patch is in the attachment.

[Addy Osmani]

Thanks for highlighting this as a potential issue. Imo, Speedometer attempting to inject items before app.js is loaded is a bug (however this is the first time I've come across it). My only concern with the proposed solution is whether this pattern for readiness is consistent with how developers use jQuery.

[Ryosuke Niwa]

The fix looks good. Could you create a patch in accordance with https://webkit.org/contributing-code/ ?

The easiest way to do this would be to run "./Tools/Scripts/wekbit-patch upload 176017 --commit-queue" at the top level directory, and then edit the change log with a description of your change.

[Ryosuke Niwa]

In the future, please cc me on any of Speedometer related bugs. I don't actively look for bugs so I'd totally miss a bug like this, and nobody else working on Speedometer 2.0 can legitimately review your patch since they're not a WebKit reviewer.

[Shiyu Zhang]

Created attachment 319321 [details]
Patch

[Shiyu Zhang]

Thanks for reviewing the bug. I have created the patch, PTAL.

[Shiyu Zhang]

I just noticed that BackboneJS suite may meet the same bug as well. The deleteButtons.length in resources/test.js for BackboneJS suite could be zero sometimes as I observed. This is even worse since unlike jQuery suite, BackboneJS suite will never crash even if deleteButtons.length is zero, which makes this bug difficult to be detected. Shall I file another bug to fix this issue?

[Ryosuke Niwa]

(In reply to Shiyu Zhang from comment #6)
> I just noticed that BackboneJS suite may meet the same bug as well. The
> deleteButtons.length in resources/test.js for BackboneJS suite could be zero
> sometimes as I observed. This is even worse since unlike jQuery suite,
> BackboneJS suite will never crash even if deleteButtons.length is zero,
> which makes this bug difficult to be detected. Shall I file another bug to
> fix this issue?

Yes, please file a new bug and make it a blocker to 172339.

[Ryosuke Niwa]

Comment on attachment 319321 [details]
Patch

In the future, please also set cq flag to ? (to ask to be landed via commit queue).

[WebKit Commit Bot]

Comment on attachment 319321 [details]
Patch

Clearing flags on attachment: 319321

Committed r221401: <http://trac.webkit.org/changeset/221401>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34174394>