Summary: | [MediaStream API] Updating NavigatorUserMediaError to match the spec | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||
Component: | WebCore Misc. | Assignee: | Eric Carlson <eric.carlson> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cdumez, commit-queue, eric.carlson, esprehn+autocc, glenn, hta, jer.noble, kondapallykalyan, syoichi, tommyw | ||||
Priority: | P2 | Keywords: | BlinkMergeCandidate | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 121101 | ||||||
Attachments: |
|
Description
Eric Carlson
2013-09-06 12:00:20 PDT
Created attachment 211357 [details]
Proposed patch
Comment on attachment 211357 [details] 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. Comment on attachment 211357 [details] 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? (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 (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. Committed r155579: https://trac.webkit.org/r155579 |