120873 2013-09-06 11:48:25 -0700 MediaStream API: Stop means stop 2013-09-10 10:30:18 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED BlinkMergeCandidate P2 Normal --- 121101 1 eric.carlson eric.carlson commit-queue eric.carlson glenn hta jer.noble tommyw oldest_to_newest 925751 0 eric.carlson 2013-09-06 11:48:25 -0700 Consider merging https://chromium.googlesource.com/chromium/blink/+/9a97b0f3892320fa5741485e06a78183b77c3635 Removing notification functionality from MediaStreamTrack::stop because the world is being torn down. 926713 1 211044 eric.carlson 2013-09-09 08:36:47 -0700 Created attachment 211044 Proposed patch 926794 2 211044 jer.noble 2013-09-09 10:55:59 -0700 Comment on attachment 211044 Proposed patch This seems weird. MediaStreamTrack is an ActiveDOMObject, but doesn't override any of ActiveDOMObject's methods besides ::stop(), and doesn't call setPendingActivity(). Why isn't MediaStream an ActiveDOMObject? After all, it has a m_stopped ivar too. If anything, MediaStreamSource is the active object, but MediaStreamTrack::stop() doesn't try to stop it's associated source. This seems highly weird, and I'm not entirely comfortable with it. But my problems with this method don't have much to do with this change specifically, so LGTM with reservations. 926803 3 211044 commit-queue 2013-09-09 11:18:45 -0700 Comment on attachment 211044 Proposed patch Clearing flags on attachment: 211044 Committed r155364: <http://trac.webkit.org/changeset/155364> 926804 4 commit-queue 2013-09-09 11:18:48 -0700 All reviewed patches have been landed. Closing bug. 211044 2013-09-09 08:36:47 -0700 2013-09-09 11:18:45 -0700 Proposed patch stop_means_stop_1.txt text/plain 1521 eric.carlson SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE1NTM1MSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDEzLTA5LTA5ICBFcmljIENh cmxzb24gIDxlcmljLmNhcmxzb25AYXBwbGUuY29tPgorCisgICAgICAgIE1lZGlhU3RyZWFtIEFQ STogU3RvcCBtZWFucyBzdG9wCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xMjA4NzMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBNZXJnZSBibGluayBodHRwczovL2Nocm9taXVtLmdvb2dsZXNvdXJjZS5jb20v Y2hyb21pdW0vYmxpbmsvKy85YTk3YjBmMzg5MjMyMGZhNTc0MTQ4NWUwNmE3ODE4M2I3N2MzNjM1 CisgICAgICAgIGJ5IFRvbW15IFdpZGVuZmx5Y2h0LgorCisgICAgICAgIFJlbW92aW5nIG5vdGlm aWNhdGlvbiBmdW5jdGlvbmFsaXR5IGZyb20gTWVkaWFTdHJlYW1UcmFjazo6c3RvcCBiZWNhdXNl IHRoZSB3b3JsZCBpcyBiZWluZyB0b3JuIGRvd24uCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLCB0 aGUgb3JpZ2luYWwgYXV0aG9yIHdhcyB1bmFibGUgdG8gY29tZSB1cCB3aXRoIGEgcmVsaWFibGUg dGVzdCBjYXNlLgorCisgICAgICAgICogTW9kdWxlcy9tZWRpYXN0cmVhbS9NZWRpYVN0cmVhbVRy YWNrLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhU3RyZWFtVHJhY2s6OnN0b3ApOiBEb24n dCBjYWxsIGRpZEVuZFRyYWNrLCB0aGUgd29ybGQgaXMgZ29pbmcgYXdheS4KKwogMjAxMy0wOS0w OSAgQWxsYW4gU2FuZGZlbGQgSmVuc2VuICA8YWxsYW4uamVuc2VuQGRpZ2lhLmNvbT4KIAogICAg ICAgICBNb3VzZUVudGVyIGFuZCBNb3VzZUxlYXZlIG1heSBiZSBlbWl0dGVkIG9uIERvY3VtZW50 IG5vZGVzCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL21lZGlhc3RyZWFtL01lZGlhU3Ry ZWFtVHJhY2suY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL01vZHVsZXMvbWVkaWFz dHJlYW0vTWVkaWFTdHJlYW1UcmFjay5jcHAJKHJldmlzaW9uIDE1NTI0OSkKKysrIFNvdXJjZS9X ZWJDb3JlL01vZHVsZXMvbWVkaWFzdHJlYW0vTWVkaWFTdHJlYW1UcmFjay5jcHAJKHdvcmtpbmcg Y29weSkKQEAgLTE1Nyw3ICsxNTcsNiBAQCBNZWRpYVN0cmVhbUNvbXBvbmVudCogTWVkaWFTdHJl YW1UcmFjazo6CiB2b2lkIE1lZGlhU3RyZWFtVHJhY2s6OnN0b3AoKQogewogICAgIG1fc3RvcHBl ZCA9IHRydWU7Ci0gICAgZGlkRW5kVHJhY2soKTsKIH0KIAogY29uc3QgQXRvbWljU3RyaW5nJiBN ZWRpYVN0cmVhbVRyYWNrOjppbnRlcmZhY2VOYW1lKCkgY29uc3QK