38602 2010-05-05 12:35:26 -0700 Page::setCanStartMedia does not properly handle the case where a media listener is removed 2010-05-06 10:12:44 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 darin darin bdakin oldest_to_newest 221234 0 darin 2010-05-05 12:35:26 -0700 Page::setCanStartMedia does not properly handle the case where a media listener is removed 221236 1 55144 darin 2010-05-05 12:40:08 -0700 Created attachment 55144 Patch 221237 2 55144 darin 2010-05-05 12:41:18 -0700 Comment on attachment 55144 Patch Now the tricky part. I see this bug by code inspection, but I'm not sure how to make a test case. I did not mean to put this up for review. webkit-patch set the review? flag for me. 221238 3 55144 aroben 2010-05-05 12:48:45 -0700 Comment on attachment 55144 Patch > @@ -422,8 +422,11 @@ void Page::setCanStartMedia(bool canStar > m_mediaCanStartListeners.clear(); > > size_t size = listeners.size(); > - for (size_t i = 0; i < size; ++i) > - listeners[i]->mediaCanStart(); > + for (size_t i = 0; i < size; ++i) { > + MediaCanStartListener* listener = listeners[i]; > + if (m_mediaCanStartListeners.contains(listener))' Won't this test always fail, since line 422 cleared out m_mediaCanStartListeners? 221239 4 darin 2010-05-05 12:50:48 -0700 (In reply to comment #3) > (From update of attachment 55144 [details]) > > @@ -422,8 +422,11 @@ void Page::setCanStartMedia(bool canStar > > m_mediaCanStartListeners.clear(); > > > > size_t size = listeners.size(); > > - for (size_t i = 0; i < size; ++i) > > - listeners[i]->mediaCanStart(); > > + for (size_t i = 0; i < size; ++i) { > > + MediaCanStartListener* listener = listeners[i]; > > + if (m_mediaCanStartListeners.contains(listener))' > > Won't this test always fail, since line 422 cleared out > m_mediaCanStartListeners? Yes, that's right. Need a bigger change. 221245 5 55147 darin 2010-05-05 13:13:08 -0700 Created attachment 55147 Patch 221334 6 55168 darin 2010-05-05 16:10:21 -0700 Created attachment 55168 Improved version. 221772 7 darin 2010-05-06 10:12:44 -0700 Committed r58894: <http://trac.webkit.org/changeset/58894> 55144 2010-05-05 12:40:08 -0700 2010-05-05 12:57:41 -0700 Patch bug-38602-20100505124007.patch text/plain 1381 darin SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1ODgzNikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTAtMDUtMDUgIERhcmluIEFkbGVyICA8ZGFyaW5AYXBwbGUuY29t PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFBhZ2U6 OnNldENhblN0YXJ0TWVkaWEgZG9lcyBub3QgcHJvcGVybHkgaGFuZGxlIHRoZSBjYXNlIHdoZXJl IGEgbWVkaWEgbGlzdGVuZXIgaXMgcmVtb3ZlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mzg2MDIKKworICAgICAgICBObyBuZXcgdGVzdHMuIChPT1BT ISkKKworICAgICAgICAqIHBhZ2UvUGFnZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpQYWdlOjpz ZXRDYW5TdGFydE1lZGlhKTogQ2hlY2sgdGhhdCB0aGUgbGlzdGVuZXIgaXMgc3RpbGwgaW4KKyAg ICAgICAgdGhlIHNldCBiZWZvcmUgY2FsbGluZyBpdC4KKwogMjAxMC0wNS0wNSAgQWxleGV5IFBy b3NrdXJ5YWtvdiAgPGFwQGFwcGxlLmNvbT4KIAogICAgICAgICBSb2xsIG91dCByNTg4MzAgZm9y IGJyZWFraW5nIHRlc3RzLgpJbmRleDogV2ViQ29yZS9wYWdlL1BhZ2UuY3BwCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFdlYkNvcmUvcGFnZS9QYWdlLmNwcAkocmV2aXNpb24gNTg4MzUpCisrKyBXZWJDb3JlL3Bh Z2UvUGFnZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTQyMiw4ICs0MjIsMTEgQEAgdm9pZCBQYWdl OjpzZXRDYW5TdGFydE1lZGlhKGJvb2wgY2FuU3RhcgogICAgIG1fbWVkaWFDYW5TdGFydExpc3Rl bmVycy5jbGVhcigpOwogCiAgICAgc2l6ZV90IHNpemUgPSBsaXN0ZW5lcnMuc2l6ZSgpOwotICAg IGZvciAoc2l6ZV90IGkgPSAwOyBpIDwgc2l6ZTsgKytpKQotICAgICAgICBsaXN0ZW5lcnNbaV0t Pm1lZGlhQ2FuU3RhcnQoKTsKKyAgICBmb3IgKHNpemVfdCBpID0gMDsgaSA8IHNpemU7ICsraSkg eworICAgICAgICBNZWRpYUNhblN0YXJ0TGlzdGVuZXIqIGxpc3RlbmVyID0gbGlzdGVuZXJzW2ld OworICAgICAgICBpZiAobV9tZWRpYUNhblN0YXJ0TGlzdGVuZXJzLmNvbnRhaW5zKGxpc3RlbmVy KSkKKyAgICAgICAgICAgIGxpc3RlbmVyLT5tZWRpYUNhblN0YXJ0KCk7CisgICAgfQogfQogCiBz dGF0aWMgRnJhbWUqIGluY3JlbWVudEZyYW1lKEZyYW1lKiBjdXJyLCBib29sIGZvcndhcmQsIGJv b2wgd3JhcEZsYWcpCg== 55147 2010-05-05 13:13:08 -0700 2010-05-05 16:10:17 -0700 Patch bug-38602-20100505131307.patch text/plain 2156 darin SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1ODgzNikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTggQEAKKzIwMTAtMDUtMDUgIERhcmluIEFkbGVyICA8ZGFyaW5AYXBwbGUuY29t PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFBhZ2U6 OnNldENhblN0YXJ0TWVkaWEgZG9lcyBub3QgcHJvcGVybHkgaGFuZGxlIHRoZSBjYXNlIHdoZXJl IGEgbWVkaWEgbGlzdGVuZXIgaXMgcmVtb3ZlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mzg2MDIKKworICAgICAgICBObyBuZXcgdGVzdHMuIChPT1BT ISkKKworICAgICAgICAqIHBhZ2UvUGFnZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpQYWdlOjpy ZW1vdmVNZWRpYUNhblN0YXJ0TGlzdGVuZXIpOiBSZW1vdmVkIGluY29ycmVjdCBhc3NlcnRpb24u CisgICAgICAgIChXZWJDb3JlOjpQYWdlOjpzZXRDYW5TdGFydE1lZGlhKTogQ2hhbmdlIGFsZ29y aXRobSBzbyB3ZSBub3RpZnkgbGlzdGVuZXJzCisgICAgICAgIG9uZSBhdCBhIHRpbWUgYW5kIGRv bid0IG5vdGlmeSBhbnkgbGlzdGVuZXIgdGhhdCBoYXMgYWxyZWFkeSBiZWVuIHJlbW92ZWQKKyAg ICAgICAgZnJvbSB0aGUgc2V0LgorCiAyMDEwLTA1LTA1ICBBbGV4ZXkgUHJvc2t1cnlha292ICA8 YXBAYXBwbGUuY29tPgogCiAgICAgICAgIFJvbGwgb3V0IHI1ODgzMCBmb3IgYnJlYWtpbmcgdGVz dHMuCkluZGV4OiBXZWJDb3JlL3BhZ2UvUGFnZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9w YWdlL1BhZ2UuY3BwCShyZXZpc2lvbiA1ODgzNSkKKysrIFdlYkNvcmUvcGFnZS9QYWdlLmNwcAko d29ya2luZyBjb3B5KQpAQCAtNDAyLDcgKzQwMiw2IEBAIHZvaWQgUGFnZTo6YWRkTWVkaWFDYW5T dGFydExpc3RlbmVyKE1lZGkKIAogdm9pZCBQYWdlOjpyZW1vdmVNZWRpYUNhblN0YXJ0TGlzdGVu ZXIoTWVkaWFDYW5TdGFydExpc3RlbmVyKiBsaXN0ZW5lcikKIHsKLSAgICBBU1NFUlQoIW1fY2Fu U3RhcnRNZWRpYSk7CiAgICAgQVNTRVJUKG1fbWVkaWFDYW5TdGFydExpc3RlbmVycy5jb250YWlu cyhsaXN0ZW5lcikpOwogICAgIG1fbWVkaWFDYW5TdGFydExpc3RlbmVycy5yZW1vdmUobGlzdGVu ZXIpOwogfQpAQCAtNDE0LDE2ICs0MTMsMTUgQEAgdm9pZCBQYWdlOjpzZXRDYW5TdGFydE1lZGlh KGJvb2wgY2FuU3RhcgogCiAgICAgbV9jYW5TdGFydE1lZGlhID0gY2FuU3RhcnRNZWRpYTsKIAot ICAgIGlmICghbV9jYW5TdGFydE1lZGlhIHx8IG1fbWVkaWFDYW5TdGFydExpc3RlbmVycy5pc0Vt cHR5KCkpCisgICAgaWYgKCFtX2NhblN0YXJ0TWVkaWEpCiAgICAgICAgIHJldHVybjsKIAotICAg IFZlY3RvcjxNZWRpYUNhblN0YXJ0TGlzdGVuZXIqPiBsaXN0ZW5lcnM7Ci0gICAgY29weVRvVmVj dG9yKG1fbWVkaWFDYW5TdGFydExpc3RlbmVycywgbGlzdGVuZXJzKTsKLSAgICBtX21lZGlhQ2Fu U3RhcnRMaXN0ZW5lcnMuY2xlYXIoKTsKLQotICAgIHNpemVfdCBzaXplID0gbGlzdGVuZXJzLnNp emUoKTsKLSAgICBmb3IgKHNpemVfdCBpID0gMDsgaSA8IHNpemU7ICsraSkKLSAgICAgICAgbGlz dGVuZXJzW2ldLT5tZWRpYUNhblN0YXJ0KCk7CisgICAgd2hpbGUgKCFtX21lZGlhQ2FuU3RhcnRM aXN0ZW5lcnMuaXNFbXB0eSgpKSB7CisgICAgICAgIEhhc2hTZXQ8TWVkaWFDYW5TdGFydExpc3Rl bmVyKj46Oml0ZXJhdG9yIHNsb3QgPSBtX21lZGlhQ2FuU3RhcnRMaXN0ZW5lcnMuYmVnaW4oKTsK KyAgICAgICAgTWVkaWFDYW5TdGFydExpc3RlbmVyKiBsaXN0ZW5lciA9ICpzbG90OworICAgICAg ICBtX21lZGlhQ2FuU3RhcnRMaXN0ZW5lcnMucmVtb3ZlKHNsb3QpOworICAgICAgICBsaXN0ZW5l ci0+bWVkaWFDYW5TdGFydCgpOworICAgIH0KIH0KIAogc3RhdGljIEZyYW1lKiBpbmNyZW1lbnRG cmFtZShGcmFtZSogY3VyciwgYm9vbCBmb3J3YXJkLCBib29sIHdyYXBGbGFnKQo= 55168 2010-05-05 16:10:21 -0700 2010-05-05 16:11:22 -0700 Improved version. bug-38602-20100505161019.patch text/plain 2148 darin SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1ODgzNikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTggQEAKKzIwMTAtMDUtMDUgIERhcmluIEFkbGVyICA8ZGFyaW5AYXBwbGUuY29t PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFBhZ2U6 OnNldENhblN0YXJ0TWVkaWEgZG9lcyBub3QgcHJvcGVybHkgaGFuZGxlIHRoZSBjYXNlIHdoZXJl IGEgbWVkaWEgbGlzdGVuZXIgaXMgcmVtb3ZlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mzg2MDIKKworICAgICAgICBObyBuZXcgdGVzdHMuIChPT1BT ISkKKworICAgICAgICAqIHBhZ2UvUGFnZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpQYWdlOjpy ZW1vdmVNZWRpYUNhblN0YXJ0TGlzdGVuZXIpOiBSZW1vdmVkIGluY29ycmVjdCBhc3NlcnRpb24u CisgICAgICAgIChXZWJDb3JlOjpQYWdlOjpzZXRDYW5TdGFydE1lZGlhKTogQ2hhbmdlIGFsZ29y aXRobSBzbyB3ZSBub3RpZnkgbGlzdGVuZXJzCisgICAgICAgIG9uZSBhdCBhIHRpbWUgYW5kIGRv bid0IG5vdGlmeSBhbnkgbGlzdGVuZXIgdGhhdCBoYXMgYWxyZWFkeSBiZWVuIHJlbW92ZWQKKyAg ICAgICAgZnJvbSB0aGUgc2V0LgorCiAyMDEwLTA1LTA1ICBBbGV4ZXkgUHJvc2t1cnlha292ICA8 YXBAYXBwbGUuY29tPgogCiAgICAgICAgIFJvbGwgb3V0IHI1ODgzMCBmb3IgYnJlYWtpbmcgdGVz dHMuCkluZGV4OiBXZWJDb3JlL3BhZ2UvUGFnZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9w YWdlL1BhZ2UuY3BwCShyZXZpc2lvbiA1ODgzNSkKKysrIFdlYkNvcmUvcGFnZS9QYWdlLmNwcAko d29ya2luZyBjb3B5KQpAQCAtNDAyLDcgKzQwMiw2IEBAIHZvaWQgUGFnZTo6YWRkTWVkaWFDYW5T dGFydExpc3RlbmVyKE1lZGkKIAogdm9pZCBQYWdlOjpyZW1vdmVNZWRpYUNhblN0YXJ0TGlzdGVu ZXIoTWVkaWFDYW5TdGFydExpc3RlbmVyKiBsaXN0ZW5lcikKIHsKLSAgICBBU1NFUlQoIW1fY2Fu U3RhcnRNZWRpYSk7CiAgICAgQVNTRVJUKG1fbWVkaWFDYW5TdGFydExpc3RlbmVycy5jb250YWlu cyhsaXN0ZW5lcikpOwogICAgIG1fbWVkaWFDYW5TdGFydExpc3RlbmVycy5yZW1vdmUobGlzdGVu ZXIpOwogfQpAQCAtNDE0LDE2ICs0MTMsMTIgQEAgdm9pZCBQYWdlOjpzZXRDYW5TdGFydE1lZGlh KGJvb2wgY2FuU3RhcgogCiAgICAgbV9jYW5TdGFydE1lZGlhID0gY2FuU3RhcnRNZWRpYTsKIAot ICAgIGlmICghbV9jYW5TdGFydE1lZGlhIHx8IG1fbWVkaWFDYW5TdGFydExpc3RlbmVycy5pc0Vt cHR5KCkpCi0gICAgICAgIHJldHVybjsKLQotICAgIFZlY3RvcjxNZWRpYUNhblN0YXJ0TGlzdGVu ZXIqPiBsaXN0ZW5lcnM7Ci0gICAgY29weVRvVmVjdG9yKG1fbWVkaWFDYW5TdGFydExpc3RlbmVy cywgbGlzdGVuZXJzKTsKLSAgICBtX21lZGlhQ2FuU3RhcnRMaXN0ZW5lcnMuY2xlYXIoKTsKLQot ICAgIHNpemVfdCBzaXplID0gbGlzdGVuZXJzLnNpemUoKTsKLSAgICBmb3IgKHNpemVfdCBpID0g MDsgaSA8IHNpemU7ICsraSkKLSAgICAgICAgbGlzdGVuZXJzW2ldLT5tZWRpYUNhblN0YXJ0KCk7 CisgICAgd2hpbGUgKG1fY2FuU3RhcnRNZWRpYSAmJiAhbV9tZWRpYUNhblN0YXJ0TGlzdGVuZXJz LmlzRW1wdHkoKSkgeworICAgICAgICBIYXNoU2V0PE1lZGlhQ2FuU3RhcnRMaXN0ZW5lcio+Ojpp dGVyYXRvciBzbG90ID0gbV9tZWRpYUNhblN0YXJ0TGlzdGVuZXJzLmJlZ2luKCk7CisgICAgICAg IE1lZGlhQ2FuU3RhcnRMaXN0ZW5lciogbGlzdGVuZXIgPSAqc2xvdDsKKyAgICAgICAgbV9tZWRp YUNhblN0YXJ0TGlzdGVuZXJzLnJlbW92ZShzbG90KTsKKyAgICAgICAgbGlzdGVuZXItPm1lZGlh Q2FuU3RhcnQoKTsKKyAgICB9CiB9CiAKIHN0YXRpYyBGcmFtZSogaW5jcmVtZW50RnJhbWUoRnJh bWUqIGN1cnIsIGJvb2wgZm9yd2FyZCwgYm9vbCB3cmFwRmxhZykK