176017 2017-08-28 01:49:12 -0700 Speedometer 2.0: jQuery test fails occasionally 2017-08-30 20:16:58 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build PC Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=176142 InRadar P2 Normal --- 172339 1 shiyu.zhang webkit-unassigned addyo buildbot cdumez commit-queue lforschler mathias rniwa webkit-bug-importer oldest_to_newest 1342777 0 319169 shiyu.zhang 2017-08-28 01:49:12 -0700 Created attachment 319169 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. 1343031 1 addyo 2017-08-28 19:26:22 -0700 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. 1343066 2 rniwa 2017-08-28 23:45:57 -0700 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. 1343067 3 rniwa 2017-08-28 23:54:32 -0700 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. 1343383 4 319321 shiyu.zhang 2017-08-29 18:45:32 -0700 Created attachment 319321 Patch 1343385 5 shiyu.zhang 2017-08-29 18:55:15 -0700 Thanks for reviewing the bug. I have created the patch, PTAL. 1343450 6 shiyu.zhang 2017-08-29 21:37:58 -0700 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? 1343750 7 rniwa 2017-08-30 15:40:35 -0700 (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. 1343751 8 319321 rniwa 2017-08-30 15:41:38 -0700 Comment on attachment 319321 Patch In the future, please also set cq flag to ? (to ask to be landed via commit queue). 1343753 9 319321 commit-queue 2017-08-30 15:45:30 -0700 Comment on attachment 319321 Patch Clearing flags on attachment: 319321 Committed r221401: <http://trac.webkit.org/changeset/221401> 1343754 10 commit-queue 2017-08-30 15:45:31 -0700 All reviewed patches have been landed. Closing bug. 1343756 11 webkit-bug-importer 2017-08-30 15:46:29 -0700 <rdar://problem/34174394> 319169 2017-08-28 01:49:12 -0700 2017-08-29 18:45:27 -0700 A possible fix for jQuery test fix-jquery.pat text/plain 1453 shiyu.zhang SW5kZXg6IHJlc291cmNlcy90ZXN0cy5qcwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSByZXNvdXJjZXMvdGVzdHMu anMJKHJldmlzaW9uIDIyMTEzMCkKKysrIHJlc291cmNlcy90ZXN0cy5qcwkod29ya2luZyBjb3B5 KQpAQCAtMzI2LDkgKzMyNiwxMCBAQAogICAgIG5hbWU6ICdqUXVlcnktVG9kb01WQycsCiAgICAg dXJsOiAndG9kb212Yy9hcmNoaXRlY3R1cmUtZXhhbXBsZXMvanF1ZXJ5L2luZGV4Lmh0bWwnLAog ICAgIHByZXBhcmU6IGZ1bmN0aW9uIChydW5uZXIsIGNvbnRlbnRXaW5kb3csIGNvbnRlbnREb2N1 bWVudCkgewotICAgICAgICByZXR1cm4gcnVubmVyLndhaXRGb3JFbGVtZW50KCcjbmV3LXRvZG8n KS50aGVuKGZ1bmN0aW9uIChlbGVtZW50KSB7Ci0gICAgICAgICAgICBlbGVtZW50LmZvY3VzKCk7 Ci0gICAgICAgICAgICByZXR1cm4gZWxlbWVudDsKKyAgICAgICAgcmV0dXJuIHJ1bm5lci53YWl0 Rm9yRWxlbWVudCgnI2FwcElzUmVhZHknKS50aGVuKGZ1bmN0aW9uIChlbGVtZW50KSB7CisgICAg ICAgICAgICB2YXIgbmV3VG9kbyA9IGNvbnRlbnREb2N1bWVudC5nZXRFbGVtZW50QnlJZCgnbmV3 LXRvZG8nKTsKKyAgICAgICAgICAgIG5ld1RvZG8uZm9jdXMoKTsKKyAgICAgICAgICAgIHJldHVy biBuZXdUb2RvOwogICAgICAgICB9KTsKICAgICB9LAogICAgIHRlc3RzOiBbCkluZGV4OiByZXNv dXJjZXMvdG9kb212Yy9hcmNoaXRlY3R1cmUtZXhhbXBsZXMvanF1ZXJ5L2pzL2FwcC5qcwo9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSByZXNvdXJjZXMvdG9kb212Yy9hcmNoaXRlY3R1cmUtZXhhbXBsZXMvanF1ZXJ5 L2pzL2FwcC5qcwkocmV2aXNpb24gMjIxMTMwKQorKysgcmVzb3VyY2VzL3RvZG9tdmMvYXJjaGl0 ZWN0dXJlLWV4YW1wbGVzL2pxdWVyeS9qcy9hcHAuanMJKHdvcmtpbmcgY29weSkKQEAgLTQ2LDYg KzQ2LDEwIEBACiAgICAgICAgICAgICAgICAgICAgIHRoaXMucmVuZGVyKCk7CiAgICAgICAgICAg ICAgICAgfS5iaW5kKHRoaXMpCiAgICAgICAgICAgICB9KS5pbml0KCcvYWxsJyk7CisKKyAgICAg ICAgICAgIHZhciBkdW1teU5vZGVUb05vdGlmeUFwcElzUmVhZHkgPSBkb2N1bWVudC5jcmVhdGVF bGVtZW50KCdkaXYnKTsKKyAgICAgICAgICAgIGR1bW15Tm9kZVRvTm90aWZ5QXBwSXNSZWFkeS5p ZCA9ICdhcHBJc1JlYWR5JzsKKyAgICAgICAgICAgIGRvY3VtZW50LmJvZHkuYXBwZW5kQ2hpbGQo ZHVtbXlOb2RlVG9Ob3RpZnlBcHBJc1JlYWR5KTsKICAgICAgICAgfSwKICAgICAgICAgYmluZEV2 ZW50czogZnVuY3Rpb24gKCkgewogICAgICAgICAgICAgJCgnI25ldy10b2RvJykub24oJ2tleXVw JywgdGhpcy5jcmVhdGUuYmluZCh0aGlzKSk7Cg== 319321 2017-08-29 18:45:32 -0700 2017-08-30 15:45:30 -0700 Patch bug-176017-20170830174517.patch text/plain 2508 shiyu.zhang SW5kZXg6IFBlcmZvcm1hbmNlVGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFBlcmZvcm1h bmNlVGVzdHMvQ2hhbmdlTG9nCShyZXZpc2lvbiAyMjEzMzIpCisrKyBQZXJmb3JtYW5jZVRlc3Rz L0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDE3LTA4LTMwICBT aGl5dSBaaGFuZyAgPHNoaXl1LnpoYW5nQGludGVsLmNvbT4KKworICAgICAgICBTcGVlZG9tZXRl ciAyLjA6IGpRdWVyeSB0ZXN0IGZhaWxzIG9jY2FzaW9uYWxseQorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTc2MDE3CisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ3JlYXRlIGEgZHVtbXkgbm9kZSB0byBub3Rp ZnkgdGhhdCBhcHAgaXMgcmVhZHkgZm9yIGpRdWVyeSBzdWl0ZS4gSXQgcHJldmVudHMgU3BlZWRv bWV0ZXIgaW5qZWN0aW5nIGl0ZW1zIGJlZm9yZSBhcHAuanMgaXMgbG9hZGVkLgorCisgICAgICAg ICogU3BlZWRvbWV0ZXIvcmVzb3VyY2VzL3Rlc3RzLmpzOgorICAgICAgICAoU3VpdGVzLnB1c2gu cHJlcGFyZSk6CisgICAgICAgICogU3BlZWRvbWV0ZXIvcmVzb3VyY2VzL3RvZG9tdmMvYXJjaGl0 ZWN0dXJlLWV4YW1wbGVzL2pxdWVyeS9qcy9hcHAuanM6CisgICAgICAgIChqUXVlcnkuQXBwLmlu aXQpOgorCiAyMDE3LTA4LTI1ICBNYXRoaWFzIEJ5bmVucyAgPG1hdGhpYXNAcWl3aS5iZT4KIAog ICAgICAgICBTcGVlZG9tZXRlcjogUmVzdHJ1Y3R1cmUgRW1iZXIgZXhhbXBsZQpJbmRleDogUGVy Zm9ybWFuY2VUZXN0cy9TcGVlZG9tZXRlci9yZXNvdXJjZXMvdGVzdHMuanMKPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQot LS0gUGVyZm9ybWFuY2VUZXN0cy9TcGVlZG9tZXRlci9yZXNvdXJjZXMvdGVzdHMuanMJKHJldmlz aW9uIDIyMTI5NykKKysrIFBlcmZvcm1hbmNlVGVzdHMvU3BlZWRvbWV0ZXIvcmVzb3VyY2VzL3Rl c3RzLmpzCSh3b3JraW5nIGNvcHkpCkBAIC0zNTUsOSArMzU1LDEwIEBAIFN1aXRlcy5wdXNoKHsK ICAgICBuYW1lOiAnalF1ZXJ5LVRvZG9NVkMnLAogICAgIHVybDogJ3RvZG9tdmMvYXJjaGl0ZWN0 dXJlLWV4YW1wbGVzL2pxdWVyeS9pbmRleC5odG1sJywKICAgICBwcmVwYXJlOiBmdW5jdGlvbiAo cnVubmVyLCBjb250ZW50V2luZG93LCBjb250ZW50RG9jdW1lbnQpIHsKLSAgICAgICAgcmV0dXJu IHJ1bm5lci53YWl0Rm9yRWxlbWVudCgnI25ldy10b2RvJykudGhlbihmdW5jdGlvbiAoZWxlbWVu dCkgewotICAgICAgICAgICAgZWxlbWVudC5mb2N1cygpOwotICAgICAgICAgICAgcmV0dXJuIGVs ZW1lbnQ7CisgICAgICAgIHJldHVybiBydW5uZXIud2FpdEZvckVsZW1lbnQoJyNhcHBJc1JlYWR5 JykudGhlbihmdW5jdGlvbiAoZWxlbWVudCkgeworICAgICAgICAgICAgdmFyIG5ld1RvZG8gPSBj b250ZW50RG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoJ25ldy10b2RvJyk7CisgICAgICAgICAgICBu ZXdUb2RvLmZvY3VzKCk7CisgICAgICAgICAgICByZXR1cm4gbmV3VG9kbzsKICAgICAgICAgfSk7 CiAgICAgfSwKICAgICB0ZXN0czogWwpJbmRleDogUGVyZm9ybWFuY2VUZXN0cy9TcGVlZG9tZXRl ci9yZXNvdXJjZXMvdG9kb212Yy9hcmNoaXRlY3R1cmUtZXhhbXBsZXMvanF1ZXJ5L2pzL2FwcC5q cwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBQZXJmb3JtYW5jZVRlc3RzL1NwZWVkb21ldGVyL3Jlc291cmNlcy90 b2RvbXZjL2FyY2hpdGVjdHVyZS1leGFtcGxlcy9qcXVlcnkvanMvYXBwLmpzCShyZXZpc2lvbiAy MjEyOTcpCisrKyBQZXJmb3JtYW5jZVRlc3RzL1NwZWVkb21ldGVyL3Jlc291cmNlcy90b2RvbXZj L2FyY2hpdGVjdHVyZS1leGFtcGxlcy9qcXVlcnkvanMvYXBwLmpzCSh3b3JraW5nIGNvcHkpCkBA IC00Niw2ICs0NiwxMCBAQCBqUXVlcnkoZnVuY3Rpb24gKCQpIHsKICAgICAgICAgICAgICAgICAg ICAgdGhpcy5yZW5kZXIoKTsKICAgICAgICAgICAgICAgICB9LmJpbmQodGhpcykKICAgICAgICAg ICAgIH0pLmluaXQoJy9hbGwnKTsKKworICAgICAgICAgICAgdmFyIGR1bW15Tm9kZVRvTm90aWZ5 QXBwSXNSZWFkeSA9IGRvY3VtZW50LmNyZWF0ZUVsZW1lbnQoJ2RpdicpOworICAgICAgICAgICAg ZHVtbXlOb2RlVG9Ob3RpZnlBcHBJc1JlYWR5LmlkID0gJ2FwcElzUmVhZHknOworICAgICAgICAg ICAgZG9jdW1lbnQuYm9keS5hcHBlbmRDaGlsZChkdW1teU5vZGVUb05vdGlmeUFwcElzUmVhZHkp OwogICAgICAgICB9LAogICAgICAgICBiaW5kRXZlbnRzOiBmdW5jdGlvbiAoKSB7CiAgICAgICAg ICAgICAkKCcjbmV3LXRvZG8nKS5vbigna2V5dXAnLCB0aGlzLmNyZWF0ZS5iaW5kKHRoaXMpKTsK