176893 2017-09-14 01:03:30 -0700 Web Inspector: REGRESSION(r221901): Single frame recordings don't reset the recording navigation item 2017-09-27 12:30:10 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 176762 173807 1 hi hi commit-queue inspector-bugzilla-changes mattbaker webkit-bug-importer oldest_to_newest 1348691 0 hi 2017-09-14 01:03:30 -0700 * Steps to reproduce: 1. Open WebInspector on a page with a 2D <canvas> 2. Select the 2D <canvas> in the Resources tab 3. Shift-click the recording navigation item to record a single frame 4. Wait for the recording tab to appear once the recording has finished 5. Switch back to the Resources tab => The recording navigation item still shows the grey "stop" box, even though the recording has finished 1348692 1 320744 hi 2017-09-14 01:05:13 -0700 Created attachment 320744 Patch 1348815 2 320744 mattbaker 2017-09-14 09:50:09 -0700 Comment on attachment 320744 Patch r-, because we should be able to add test coverage for this. In recording2d.html whenever RecordingFinished is received, we should check that WI.CanvasManager.recordingCanvas is null. I should have added this test when introducing this change. 1349968 3 hi 2017-09-18 10:23:29 -0700 (In reply to Matt Baker from comment #2) > Comment on attachment 320744 [details] > Patch > > r-, because we should be able to add test coverage for this. In > recording2d.html whenever RecordingFinished is received, we should check > that WI.CanvasManager.recordingCanvas is null. > > I should have added this test when introducing this change. None of the tests use `CanvasManager.prototype.startRecording`. All the recording tests directly call `requestRecording`/`cancelRecording` on `CanvasAgent`. The issue is that the `CanvasManager` functions don't expose the `error` object or allow for a callback, both of which are needed in the tests. 1350008 4 mattbaker 2017-09-18 11:26:39 -0700 (In reply to Devin Rousso from comment #3) > (In reply to Matt Baker from comment #2) > > Comment on attachment 320744 [details] > > Patch > > > > r-, because we should be able to add test coverage for this. In > > recording2d.html whenever RecordingFinished is received, we should check > > that WI.CanvasManager.recordingCanvas is null. > > > > I should have added this test when introducing this change. > > None of the tests use `CanvasManager.prototype.startRecording`. All the > recording tests directly call `requestRecording`/`cancelRecording` on > `CanvasAgent`. The issue is that the `CanvasManager` functions don't expose > the `error` object or allow for a callback, both of which are needed in the > tests. If CanvasManager isn't testable, then we should take steps to address that. Updating it to be promise-based would be one solution. We could also rely more of event dispatched from the manager. Currently, when requestRecording fails CanvasManager dispatches a RecordingFinished event with a null recording. This is enough information to write a test, as long as you don't care about the actual error message. 1350011 5 320744 mattbaker 2017-09-18 11:27:11 -0700 Comment on attachment 320744 Patch r=me 1350028 6 320744 commit-queue 2017-09-18 11:56:42 -0700 Comment on attachment 320744 Patch Clearing flags on attachment: 320744 Committed r222171: <http://trac.webkit.org/changeset/222171> 1350029 7 commit-queue 2017-09-18 11:56:44 -0700 All reviewed patches have been landed. Closing bug. 1353431 8 webkit-bug-importer 2017-09-27 12:30:10 -0700 <rdar://problem/34693389> 320744 2017-09-14 01:05:13 -0700 2017-09-18 11:56:42 -0700 Patch bug-176893-20170914010512.patch text/plain 1844 hi ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGI4YmVlNzI2MjIxZTg2NjVhM2EyY2M5NDgzY2Rm MWQwYzU1N2Y2MWQuLjljZDYxYjYzMDg0YTljOGZiMmI4NDE2YWUxY2FhMTE0YjU1MjY0YWEgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dl Ykluc3BlY3RvclVJL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE3LTA5LTE0ICBEZXZp biBSb3Vzc28gIDx3ZWJraXRAZGV2aW5yb3Vzc28uY29tPgorCisgICAgICAgIFdlYiBJbnNwZWN0 b3I6IFJFR1JFU1NJT04ocjIyMTkwMSk6IFNpbmdsZSBmcmFtZSByZWNvcmRpbmdzIGRvbid0IHJl c2V0IHRoZSByZWNvcmRpbmcgbmF2aWdhdGlvbiBpdGVtCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzY4OTMKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVzZXJJbnRlcmZhY2UvQ29udHJvbGxlcnMvQ2Fu dmFzTWFuYWdlci5qczoKKyAgICAgICAgKFdJLkNhbnZhc01hbmFnZXIucHJvdG90eXBlLnJlY29y ZGluZ0ZpbmlzaGVkKToKKyAgICAgICAgU2V0IHRoZSBgX3JlY29yZGluZ0NhbnZhc2AgdG8gbnVs bCB3aGVuZXZlciBhIHJlY29yZGluZyBoYXMgZmluaXNoZWQgYW5kIGJlZW4gc2VudCB0byB0aGUK KyAgICAgICAgZnJvbnRlbmQuIEl0IGlzIGFsc28gbmVjZXNzYXJ5IHRvIGRvIHRoaXMgaW5zaWRl IGBzdG9wUmVjb3JkaW5nYCwgYXMgaXQgaXMgcG9zc2libGUgZm9yIHRoZQorICAgICAgICB1c2Vy IHRvIHN0YXJ0IGFuZCBzdG9wIGEgcmVjb3JkaW5nIGJlZm9yZSBhbnkgYWN0aW9ucyBhcmUgcmVj b3JkZWQsIG1lYW5pbmcgdGhhdCBubworICAgICAgICByZWNvcmRpbmcgd291bGQgYmUgc2VudCB0 byB0aGUgZnJvbnRlbmQgYW5kIHRoaXMgZnVuY3Rpb24gd2lsbCBuZXZlciBnZXQgY2FsbGVkLgor CiAyMDE3LTA5LTEzICBEZXZpbiBSb3Vzc28gIDx3ZWJraXRAZGV2aW5yb3Vzc28uY29tPgogCiAg ICAgICAgIFdlYiBJbnNwZWN0b3I6IGFsbG93IHJlY29yZGluZ3MgdG8gYmUgc2F2ZWQgdmlhIOKM mC1TIGluIHRoZSBSZWNvcmRpbmcgdGFiCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvVXNlckludGVyZmFjZS9Db250cm9sbGVycy9DYW52YXNNYW5hZ2VyLmpzIGIvU291cmNlL1dl Ykluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvQ29udHJvbGxlcnMvQ2FudmFzTWFuYWdlci5qcwpp bmRleCA1NzdkZjIxMzcyZDc3NTA0ODk1ZDlkNTU0NDAzZWRkOWJlZTViMDBmLi43NzY1Y2FiOTJj ZTZmMDEyZjY5NWMxMGQ1MzM5Mzk5OWJhYTUzZDA1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5z cGVjdG9yVUkvVXNlckludGVyZmFjZS9Db250cm9sbGVycy9DYW52YXNNYW5hZ2VyLmpzCisrKyBi L1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL0NvbnRyb2xsZXJzL0NhbnZhc01h bmFnZXIuanMKQEAgLTE0OCw2ICsxNDgsOCBAQCBXSS5DYW52YXNNYW5hZ2VyID0gY2xhc3MgQ2Fu dmFzTWFuYWdlciBleHRlbmRzIFdJLk9iamVjdAogICAgIHsKICAgICAgICAgLy8gQ2FsbGVkIGZy b20gV0kuQ2FudmFzT2JzZXJ2ZXIuCiAKKyAgICAgICAgdGhpcy5fcmVjb3JkaW5nQ2FudmFzID0g bnVsbDsKKwogICAgICAgICBsZXQgY2FudmFzID0gdGhpcy5fY2FudmFzSWRlbnRpZmllck1hcC5n ZXQoY2FudmFzSWRlbnRpZmllcik7CiAgICAgICAgIGNvbnNvbGUuYXNzZXJ0KGNhbnZhcyk7CiAg ICAgICAgIGlmICghY2FudmFzKQo=