Bug 56666 - Media Stream API: adding Stream and GeneratedStream classes.
Summary: Media Stream API: adding Stream and GeneratedStream classes.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Leandro Graciá Gil
URL:
Keywords:
Depends on: 60177
Blocks: 56459 58787 61537 61539
  Show dependency treegraph
 
Reported: 2011-03-18 12:09 PDT by Leandro Graciá Gil
Modified: 2012-09-19 07:21 PDT (History)
8 users (show)

See Also:


Attachments
Patch (64.11 KB, patch)
2011-03-18 12:42 PDT, Leandro Graciá Gil
no flags Details | Formatted Diff | Diff
Patch (114.04 KB, patch)
2011-03-18 12:57 PDT, Leandro Graciá Gil
no flags Details | Formatted Diff | Diff
Patch (63.87 KB, patch)
2011-05-17 11:36 PDT, Leandro Graciá Gil
no flags Details | Formatted Diff | Diff
Patch (64.94 KB, patch)
2011-05-20 11:13 PDT, Leandro Graciá Gil
no flags Details | Formatted Diff | Diff
Patch (64.83 KB, patch)
2011-05-23 11:18 PDT, Leandro Graciá Gil
no flags Details | Formatted Diff | Diff
Patch (64.87 KB, patch)
2011-05-24 06:12 PDT, Leandro Graciá Gil
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Leandro Graciá Gil 2011-03-18 12:09:29 PDT
This patch introduces the Stream and GeneratedStream classes according to the lastest specification and integrating to the code introduced by previous patches.
Comment 1 Leandro Graciá Gil 2011-03-18 12:42:08 PDT
Created attachment 86196 [details]
Patch
Comment 2 Leandro Graciá Gil 2011-03-18 12:57:06 PDT
Created attachment 86200 [details]
Patch
Comment 3 Leandro Graciá Gil 2011-03-18 12:57:24 PDT
No contents changed. Rebasing to solve problems with the bots.
Comment 4 Leandro Graciá Gil 2011-03-23 13:20:42 PDT
Updating the bug platform settings (had the default ones).
Comment 5 Leandro Graciá Gil 2011-05-17 11:36:01 PDT
Created attachment 93803 [details]
Patch
Comment 6 Leandro Graciá Gil 2011-05-20 11:13:39 PDT
Created attachment 94243 [details]
Patch

Some minor fixes.
Comment 7 Tony Gentilcore 2011-05-23 06:06:46 PDT
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?
Comment 8 Tommy Widenflycht 2011-05-23 06:53:01 PDT
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.
Comment 9 Leandro Graciá Gil 2011-05-23 11:18:15 PDT
Created attachment 94448 [details]
Patch
Comment 10 Leandro Graciá Gil 2011-05-23 11:21:32 PDT
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 11 WebKit Commit Bot 2011-05-24 06:00:46 PDT
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
Comment 12 Leandro Graciá Gil 2011-05-24 06:12:18 PDT
Created attachment 94598 [details]
Patch

Rebase only.
Comment 13 WebKit Commit Bot 2011-05-24 08:12:03 PDT
Comment on attachment 94598 [details]
Patch

Clearing flags on attachment: 94598

Committed r87150: <http://trac.webkit.org/changeset/87150>
Comment 14 WebKit Commit Bot 2011-05-24 08:12:09 PDT
All reviewed patches have been landed.  Closing bug.