Summary: | Media Stream API: adding Stream and GeneratedStream classes. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Leandro Graciá Gil <leandrogracia> | ||||||||||||||
Component: | DOM | Assignee: | Leandro Graciá Gil <leandrogracia> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | adam.bergkvist, commit-queue, eric.carlson, satish, steveblock, tbassetto+bugzilla, tommyw, tonyg | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
Bug Depends on: | 60177 | ||||||||||||||||
Bug Blocks: | 56459, 58787, 61537, 61539 | ||||||||||||||||
Attachments: |
|
Description
Leandro Graciá Gil
2011-03-18 12:09:29 PDT
Created attachment 86196 [details]
Patch
Created attachment 86200 [details]
Patch
No contents changed. Rebasing to solve problems with the bots. Updating the bug platform settings (had the default ones). Created attachment 93803 [details]
Patch
Created attachment 94243 [details]
Patch
Some minor fixes.
Comment on attachment 94243 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94243&action=review Just a couple of small questions. Everything else lgtm. > Source/WebCore/dom/GeneratedStream.cpp:82 > + if (m_readyState != ENDED) Since stop() needs to be robust to ENDED streams, perhaps it would be easier to just call stop() unconditionally here at let it fail the first check? > Source/WebCore/dom/Stream.cpp:57 > + if (m_readyState == ENDED) I might be missing it, but it doesn't seem like we'd ever want to call streamEnded while m_readyState == ENDED. If not, should we ASSERT here instead? > Source/WebCore/dom/Stream.idl:36 > + attribute EventListener onreadystatechange; onreadystatechange isn't in: http://www.whatwg.org/specs/web-apps/current-work/#stream Should we have it? Looks good to me, modulo Tony's comments. View in context: https://bugs.webkit.org/attachment.cgi?id=94243&action=review >> Source/WebCore/dom/Stream.idl:36 >> + attribute EventListener onreadystatechange; > > onreadystatechange isn't in: > http://www.whatwg.org/specs/web-apps/current-work/#stream > > Should we have it? Right, it used to be in the spec but must have been recently removed. Created attachment 94448 [details]
Patch
Comment on attachment 94243 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94243&action=review >> Source/WebCore/dom/GeneratedStream.cpp:82 >> + if (m_readyState != ENDED) > > Since stop() needs to be robust to ENDED streams, perhaps it would be easier to just call stop() unconditionally here at let it fail the first check? Fixed. >> Source/WebCore/dom/Stream.cpp:57 >> + if (m_readyState == ENDED) > > I might be missing it, but it doesn't seem like we'd ever want to call streamEnded while m_readyState == ENDED. If not, should we ASSERT here instead? Fixed. >> Source/WebCore/dom/Stream.idl:36 >> + attribute EventListener onreadystatechange; > > onreadystatechange isn't in: > http://www.whatwg.org/specs/web-apps/current-work/#stream > > Should we have it? Fixed. Comment on attachment 94448 [details] Patch Rejecting attachment 94448 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'apply-..." exit_code: 2 Last 500 characters of output: e/WebCore/page/MediaStreamController.h patching file Source/WebCore/page/MediaStreamFrameController.cpp patching file Source/WebCore/page/MediaStreamFrameController.h patching file Source/WebCore/page/NavigatorUserMediaError.h patching file Source/WebCore/page/NavigatorUserMediaSuccessCallback.h patching file Source/WebCore/page/NavigatorUserMediaSuccessCallback.idl Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Tony Gentilcore', u'--..." exit_code: 1 Full output: http://queues.webkit.org/results/8734083 Created attachment 94598 [details]
Patch
Rebase only.
Comment on attachment 94598 [details] Patch Clearing flags on attachment: 94598 Committed r87150: <http://trac.webkit.org/changeset/87150> All reviewed patches have been landed. Closing bug. |