120880 2013-09-06 12:00:20 -0700 [MediaStream API] Updating NavigatorUserMediaError to match the spec 2013-09-11 16:52:01 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED BlinkMergeCandidate P2 Normal --- 121101 1 eric.carlson eric.carlson cdumez commit-queue eric.carlson esprehn+autocc glenn hta jer.noble kondapallykalyan syoichi tommyw oldest_to_newest 925762 0 eric.carlson 2013-09-06 12:00:20 -0700 Consider merging https://chromium.googlesource.com/chromium/blink/+/f07305e49ddebaa166f5dee514bcc881a8efd341 Adding support for CONSTRAINT_NOT_SATISFIED as well as uppdating the error callback to match the spec. 927994 1 211357 eric.carlson 2013-09-11 15:01:24 -0700 Created attachment 211357 Proposed patch 928004 2 211357 darin 2013-09-11 15:12:09 -0700 Comment on attachment 211357 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=211357&action=review > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:163 > + RefPtr<NavigatorUserMediaError> error = NavigatorUserMediaError::create(ASCIILiteral("PERMISSION_DENIED"), description, String()); > + m_errorCallback->handleEvent(error.get()); I sometimes like the way these read without a local variable. The single line would be no longer than the first of these two lines of code. 928005 3 211357 jer.noble 2013-09-11 15:12:51 -0700 Comment on attachment 211357 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=211357&action=review Apart from a minor nit (which could, if anything, be fixed in a future patch), LGTM. r=me. > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:162 > + RefPtr<NavigatorUserMediaError> error = NavigatorUserMediaError::create(ASCIILiteral("PERMISSION_DENIED"), description, String()); Nit: Should this and... > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:174 > + RefPtr<NavigatorUserMediaError> error = NavigatorUserMediaError::create(ASCIILiteral("CONSTRAINT_NOT_SATISFIED"), description, constraintName); ...this be static keywords on NavigatorUserMediaError? 928038 4 eric.carlson 2013-09-11 16:46:25 -0700 (In reply to comment #2) > (From update of attachment 211357 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=211357&action=review > > > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:163 > > + RefPtr<NavigatorUserMediaError> error = NavigatorUserMediaError::create(ASCIILiteral("PERMISSION_DENIED"), description, String()); > > + m_errorCallback->handleEvent(error.get()); > > I sometimes like the way these read without a local variable. The single line would be no longer than the first of these two lines of code. Done 928040 5 eric.carlson 2013-09-11 16:48:08 -0700 (In reply to comment #3) > (From update of attachment 211357 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=211357&action=review > > Apart from a minor nit (which could, if anything, be fixed in a future patch), LGTM. r=me. > > > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:162 > > + RefPtr<NavigatorUserMediaError> error = NavigatorUserMediaError::create(ASCIILiteral("PERMISSION_DENIED"), description, String()); > > Nit: Should this and... > > > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:174 > > + RefPtr<NavigatorUserMediaError> error = NavigatorUserMediaError::create(ASCIILiteral("CONSTRAINT_NOT_SATISFIED"), description, constraintName); > > ...this be static keywords on NavigatorUserMediaError? Great idea. It turns out that the spec has changed and both of those error strings are incorrect, so I will fix this when I update. 928042 6 eric.carlson 2013-09-11 16:51:50 -0700 Committed r155579: https://trac.webkit.org/r155579 211357 2013-09-11 15:01:24 -0700 2013-09-11 16:52:01 -0700 Proposed patch user_media_error_2.txt text/plain 5920 eric.carlson SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE1NTU2NSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI5IEBACisyMDEzLTA5LTExICBFcmljIENh cmxzb24gIDxlcmljLmNhcmxzb25AYXBwbGUuY29tPgorCisgICAgICAgIFtNZWRpYVN0cmVhbSBB UEldIFVwZGF0aW5nIE5hdmlnYXRvclVzZXJNZWRpYUVycm9yIHRvIG1hdGNoIHRoZSBzcGVjCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjA4ODAKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKyAgICAgICAgCisgICAgICAgIE1l cmdlIGh0dHBzOi8vY2hyb21pdW0uZ29vZ2xlc291cmNlLmNvbS9jaHJvbWl1bS9ibGluay8rL2Yw NzMwNWU0OWRkZWJhYTE2NmY1ZGVlNTE0YmNjODgxYThlZmQzNDEKKyAgICAgICAgYnkgVG9tbXkg V2lkZW5mbHljaHQuCisKKyAgICAgICAgVGhlIEJsaW5rIGNoYW5nZSBkaWQgbm90IGhhdmUgYW55 IHRlc3QgY2hhbmdlcyBiZWNhdXNlIHRoZXkgZG9uJ3QgYXBwZWFyIHRvIGhhdmUgYW55IHRlc3Rz IAorICAgICAgICBmb3IgTmF2aWdhdG9yVXNlck1lZGlhRXJyb3IuIEkgY2FuJ3QgYWRkIHRlc3Rz IG5vdyBiZWNhdXNlIHRoZSBNZWRpYVN0cmVhbSBmZWF0dXJlIGlzbid0IGVuYWJsZWQgCisgICAg ICAgIHlldCwgc28gaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyMTE4 MiB0cmFja3MgYWRkaW5nIHRlc3RzLgorCisgICAgICAgICogTW9kdWxlcy9tZWRpYXN0cmVhbS9O YXZpZ2F0b3JVc2VyTWVkaWFFcnJvci5oOgorICAgICAgICAoV2ViQ29yZTo6TmF2aWdhdG9yVXNl ck1lZGlhRXJyb3I6OmNyZWF0ZSk6CisgICAgICAgIChXZWJDb3JlOjpOYXZpZ2F0b3JVc2VyTWVk aWFFcnJvcjo6bmFtZSk6CisgICAgICAgIChXZWJDb3JlOjpOYXZpZ2F0b3JVc2VyTWVkaWFFcnJv cjo6bWVzc2FnZSk6CisgICAgICAgIChXZWJDb3JlOjpOYXZpZ2F0b3JVc2VyTWVkaWFFcnJvcjo6 Y29uc3RyYWludE5hbWUpOgorICAgICAgICAoV2ViQ29yZTo6TmF2aWdhdG9yVXNlck1lZGlhRXJy b3I6Ok5hdmlnYXRvclVzZXJNZWRpYUVycm9yKToKKyAgICAgICAgKiBNb2R1bGVzL21lZGlhc3Ry ZWFtL05hdmlnYXRvclVzZXJNZWRpYUVycm9yLmlkbDoKKyAgICAgICAgKiBNb2R1bGVzL21lZGlh c3RyZWFtL1VzZXJNZWRpYVJlcXVlc3QuY3BwOgorICAgICAgICAoV2ViQ29yZTo6VXNlck1lZGlh UmVxdWVzdDo6ZmFpbCk6CisgICAgICAgIChXZWJDb3JlOjpVc2VyTWVkaWFSZXF1ZXN0OjpmYWls Q29uc3RyYWludCk6CisgICAgICAgICogTW9kdWxlcy9tZWRpYXN0cmVhbS9Vc2VyTWVkaWFSZXF1 ZXN0Lmg6CisKIDIwMTMtMDktMTEgIFpvbHRhbiBIb3J2YXRoICA8em9sdGFuQHdlYmtpdC5vcmc+ CiAKICAgICAgICAgTW92ZSBMaW5lV2lkdGggb3V0IG9mIFJlbmRlckJsb2NrTGluZUxheW91dApJ bmRleDogU291cmNlL1dlYkNvcmUvTW9kdWxlcy9tZWRpYXN0cmVhbS9OYXZpZ2F0b3JVc2VyTWVk aWFFcnJvci5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL01vZHVsZXMvbWVkaWFzdHJl YW0vTmF2aWdhdG9yVXNlck1lZGlhRXJyb3IuaAkocmV2aXNpb24gMTU1NTYxKQorKysgU291cmNl L1dlYkNvcmUvTW9kdWxlcy9tZWRpYXN0cmVhbS9OYXZpZ2F0b3JVc2VyTWVkaWFFcnJvci5oCSh3 b3JraW5nIGNvcHkpCkBAIC0zNCwyNCArMzQsMjggQEAgbmFtZXNwYWNlIFdlYkNvcmUgewogCiBj bGFzcyBOYXZpZ2F0b3JVc2VyTWVkaWFFcnJvciA6IHB1YmxpYyBSZWZDb3VudGVkPE5hdmlnYXRv clVzZXJNZWRpYUVycm9yPiB7CiBwdWJsaWM6Ci0gICAgLy8gU2hvdWxkIGJlIGtlcHQgaW4gc3lu YyB3aXRoIHRoZSB2YWx1ZXMgaW4gdGhlIGlkbCBmaWxlLgotICAgIGVudW0gRXJyb3JDb2RlIHsK LSAgICAgICAgUEVSTUlTU0lPTl9ERU5JRUQgPSAxCi0gICAgfTsKLQotICAgIHN0YXRpYyBQYXNz UmVmUHRyPE5hdmlnYXRvclVzZXJNZWRpYUVycm9yPiBjcmVhdGUoRXJyb3JDb2RlIGNvZGUpCisg ICAgc3RhdGljIFBhc3NSZWZQdHI8TmF2aWdhdG9yVXNlck1lZGlhRXJyb3I+IGNyZWF0ZShjb25z dCBTdHJpbmcmIG5hbWUsIGNvbnN0IFN0cmluZyYgbWVzc2FnZSwgY29uc3QgU3RyaW5nJiBjb25z dHJhaW50TmFtZSkKICAgICB7Ci0gICAgICAgIHJldHVybiBhZG9wdFJlZihuZXcgTmF2aWdhdG9y VXNlck1lZGlhRXJyb3IoY29kZSkpOworICAgICAgICByZXR1cm4gYWRvcHRSZWYobmV3IE5hdmln YXRvclVzZXJNZWRpYUVycm9yKG5hbWUsIG1lc3NhZ2UsIGNvbnN0cmFpbnROYW1lKSk7CiAgICAg fQogCiAgICAgdmlydHVhbCB+TmF2aWdhdG9yVXNlck1lZGlhRXJyb3IoKSB7IH0KIAotICAgIEVy cm9yQ29kZSBjb2RlKCkgY29uc3QgeyByZXR1cm4gbV9jb2RlOyB9CisgICAgY29uc3QgU3RyaW5n JiBuYW1lKCkgY29uc3QgeyByZXR1cm4gbV9uYW1lOyB9CisgICAgY29uc3QgU3RyaW5nJiBtZXNz YWdlKCkgY29uc3QgeyByZXR1cm4gbV9tZXNzYWdlOyB9CisgICAgY29uc3QgU3RyaW5nJiBjb25z dHJhaW50TmFtZSgpIGNvbnN0IHsgcmV0dXJuIG1fY29uc3RyYWludE5hbWU7IH0KIAogcHJpdmF0 ZToKLSAgICBleHBsaWNpdCBOYXZpZ2F0b3JVc2VyTWVkaWFFcnJvcihFcnJvckNvZGUgY29kZSkg OiBtX2NvZGUoY29kZSkgeyB9CisgICAgTmF2aWdhdG9yVXNlck1lZGlhRXJyb3IoY29uc3QgU3Ry aW5nJiBuYW1lLCBjb25zdCBTdHJpbmcmIG1lc3NhZ2UsIGNvbnN0IFN0cmluZyYgY29uc3RyYWlu dE5hbWUpCisgICAgICAgIDogbV9uYW1lKG5hbWUpCisgICAgICAgICwgbV9tZXNzYWdlKG1lc3Nh Z2UpCisgICAgICAgICwgbV9jb25zdHJhaW50TmFtZShjb25zdHJhaW50TmFtZSkKKyAgICB7Cisg ICAgfQogCi0gICAgRXJyb3JDb2RlIG1fY29kZTsKKyAgICBTdHJpbmcgbV9uYW1lOworICAgIFN0 cmluZyBtX21lc3NhZ2U7CisgICAgU3RyaW5nIG1fY29uc3RyYWludE5hbWU7CiB9OwogCiB9IC8v IG5hbWVzcGFjZSBXZWJDb3JlCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL21lZGlhc3Ry ZWFtL05hdmlnYXRvclVzZXJNZWRpYUVycm9yLmlkbAo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9Nb2R1bGVzL21lZGlhc3RyZWFtL05hdmlnYXRvclVzZXJNZWRpYUVycm9yLmlkbAkocmV2 aXNpb24gMTU1NTYxKQorKysgU291cmNlL1dlYkNvcmUvTW9kdWxlcy9tZWRpYXN0cmVhbS9OYXZp Z2F0b3JVc2VyTWVkaWFFcnJvci5pZGwJKHdvcmtpbmcgY29weSkKQEAgLTI2LDcgKzI2LDggQEAK ICAgICBOb0ludGVyZmFjZU9iamVjdCwKICAgICBDb25kaXRpb25hbD1NRURJQV9TVFJFQU0KIF0g aW50ZXJmYWNlIE5hdmlnYXRvclVzZXJNZWRpYUVycm9yIHsKLSAgICBjb25zdCB1bnNpZ25lZCBz aG9ydCBQRVJNSVNTSU9OX0RFTklFRCA9IDE7Ci0gICAgcmVhZG9ubHkgYXR0cmlidXRlIHVuc2ln bmVkIHNob3J0IGNvZGU7CisgICAgcmVhZG9ubHkgYXR0cmlidXRlIERPTVN0cmluZyBuYW1lOwor ICAgIHJlYWRvbmx5IGF0dHJpYnV0ZSBET01TdHJpbmcgbWVzc2FnZTsKKyAgICByZWFkb25seSBh dHRyaWJ1dGUgRE9NU3RyaW5nIGNvbnN0cmFpbnROYW1lOwogfTsKIApJbmRleDogU291cmNlL1dl YkNvcmUvTW9kdWxlcy9tZWRpYXN0cmVhbS9Vc2VyTWVkaWFSZXF1ZXN0LmNwcAo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBTb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL21lZGlhc3RyZWFtL1VzZXJNZWRpYVJlcXVlc3Qu Y3BwCShyZXZpc2lvbiAxNTU1NjEpCisrKyBTb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL21lZGlhc3Ry ZWFtL1VzZXJNZWRpYVJlcXVlc3QuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNTMsMTMgKzE1Mywy NSBAQCB2b2lkIFVzZXJNZWRpYVJlcXVlc3Q6OnN1Y2NlZWQoUGFzc1JlZlB0CiAgICAgbV9zdWNj ZXNzQ2FsbGJhY2stPmhhbmRsZUV2ZW50KHN0cmVhbS5nZXQoKSk7CiB9CiAKLXZvaWQgVXNlck1l ZGlhUmVxdWVzdDo6ZmFpbCgpCit2b2lkIFVzZXJNZWRpYVJlcXVlc3Q6OmZhaWwoY29uc3QgU3Ry aW5nJiBkZXNjcmlwdGlvbikKIHsKICAgICBpZiAoIW1fc2NyaXB0RXhlY3V0aW9uQ29udGV4dCkK ICAgICAgICAgcmV0dXJuOwogCiAgICAgaWYgKG1fZXJyb3JDYWxsYmFjaykgewotICAgICAgICBS ZWZQdHI8TmF2aWdhdG9yVXNlck1lZGlhRXJyb3I+IGVycm9yID0gTmF2aWdhdG9yVXNlck1lZGlh RXJyb3I6OmNyZWF0ZShOYXZpZ2F0b3JVc2VyTWVkaWFFcnJvcjo6UEVSTUlTU0lPTl9ERU5JRUQp OworICAgICAgICBSZWZQdHI8TmF2aWdhdG9yVXNlck1lZGlhRXJyb3I+IGVycm9yID0gTmF2aWdh dG9yVXNlck1lZGlhRXJyb3I6OmNyZWF0ZShBU0NJSUxpdGVyYWwoIlBFUk1JU1NJT05fREVOSUVE IiksIGRlc2NyaXB0aW9uLCBTdHJpbmcoKSk7CisgICAgICAgIG1fZXJyb3JDYWxsYmFjay0+aGFu ZGxlRXZlbnQoZXJyb3IuZ2V0KCkpOworICAgIH0KK30KKwordm9pZCBVc2VyTWVkaWFSZXF1ZXN0 OjpmYWlsQ29uc3RyYWludChjb25zdCBTdHJpbmcmIGNvbnN0cmFpbnROYW1lLCBjb25zdCBTdHJp bmcmIGRlc2NyaXB0aW9uKQoreworICAgIEFTU0VSVCghY29uc3RyYWludE5hbWUuaXNFbXB0eSgp KTsKKyAgICBpZiAoIW1fc2NyaXB0RXhlY3V0aW9uQ29udGV4dCkKKyAgICAgICAgcmV0dXJuOwor ICAgIAorICAgIGlmIChtX2Vycm9yQ2FsbGJhY2spIHsKKyAgICAgICAgUmVmUHRyPE5hdmlnYXRv clVzZXJNZWRpYUVycm9yPiBlcnJvciA9IE5hdmlnYXRvclVzZXJNZWRpYUVycm9yOjpjcmVhdGUo QVNDSUlMaXRlcmFsKCJDT05TVFJBSU5UX05PVF9TQVRJU0ZJRUQiKSwgZGVzY3JpcHRpb24sIGNv bnN0cmFpbnROYW1lKTsKICAgICAgICAgbV9lcnJvckNhbGxiYWNrLT5oYW5kbGVFdmVudChlcnJv ci5nZXQoKSk7CiAgICAgfQogfQpJbmRleDogU291cmNlL1dlYkNvcmUvTW9kdWxlcy9tZWRpYXN0 cmVhbS9Vc2VyTWVkaWFSZXF1ZXN0LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvTW9k dWxlcy9tZWRpYXN0cmVhbS9Vc2VyTWVkaWFSZXF1ZXN0LmgJKHJldmlzaW9uIDE1NTU2MSkKKysr IFNvdXJjZS9XZWJDb3JlL01vZHVsZXMvbWVkaWFzdHJlYW0vVXNlck1lZGlhUmVxdWVzdC5oCSh3 b3JraW5nIGNvcHkpCkBAIC02NSw3ICs2NSw4IEBAIHB1YmxpYzoKIAogICAgIHZvaWQgc3VjY2Vl ZChjb25zdCBNZWRpYVN0cmVhbVNvdXJjZVZlY3RvciYgYXVkaW9Tb3VyY2VzLCBjb25zdCBNZWRp YVN0cmVhbVNvdXJjZVZlY3RvciYgdmlkZW9Tb3VyY2VzKTsKICAgICB2b2lkIHN1Y2NlZWQoUGFz c1JlZlB0cjxNZWRpYVN0cmVhbURlc2NyaXB0b3I+KTsKLSAgICB2b2lkIGZhaWwoKTsKKyAgICB2 b2lkIGZhaWwoY29uc3QgU3RyaW5nJiBkZXNjcmlwdGlvbik7CisgICAgdm9pZCBmYWlsQ29uc3Ry YWludChjb25zdCBTdHJpbmcmIGNvbnN0cmFpbnROYW1lLCBjb25zdCBTdHJpbmcmIGRlc2NyaXB0 aW9uKTsKIAogICAgIE1lZGlhQ29uc3RyYWludHMqIGF1ZGlvQ29uc3RyYWludHMoKSBjb25zdDsK ICAgICBNZWRpYUNvbnN0cmFpbnRzKiB2aWRlb0NvbnN0cmFpbnRzKCkgY29uc3Q7Cg==