Bug 61537 - Media Stream PeerConnection API: adding StreamEvent class
Summary: Media Stream PeerConnection API: adding StreamEvent class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Tommy Widenflycht
URL:
Keywords:
Depends on: 56666
Blocks: 56459
  Show dependency treegraph
 
Reported: 2011-05-26 09:08 PDT by Tommy Widenflycht
Modified: 2011-06-08 06:25 PDT (History)
3 users (show)

See Also:


Attachments
Patch (25.50 KB, patch)
2011-05-26 09:22 PDT, Tommy Widenflycht
no flags Details | Formatted Diff | Diff
Patch (25.54 KB, patch)
2011-05-30 05:57 PDT, Tommy Widenflycht
no flags Details | Formatted Diff | Diff
Patch (25.75 KB, patch)
2011-06-08 04:31 PDT, Tommy Widenflycht
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tommy Widenflycht 2011-05-26 09:08:03 PDT
This patch introduces the StreamEvent class according to the lastest specification.
Comment 1 Tommy Widenflycht 2011-05-26 09:22:52 PDT
Created attachment 94987 [details]
Patch
Comment 2 Leandro Graciá Gil 2011-05-26 11:35:27 PDT
(In reply to comment #1)
> Created an attachment (id=94987) [details]
> Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=94987&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

Same as mentioned in bug 61539 review.

> Source/WebCore/dom/StreamEvent.h:42
> +    static PassRefPtr<StreamEvent> create(const AtomicString& type, bool canBubble, bool cancelable, PassRefPtr<Stream>);

Are you sure that canBubble and cancelable are required here? I'm not completely sure but it sounds to me like all stream events can't bubble and are not cancelable by definition. Take a look to dom/DeviceOrientationEvent.h/cpp for example.
Comment 3 Tommy Widenflycht 2011-05-30 05:50:08 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > Created an attachment (id=94987) [details] [details]
> > Patch
> 
> View in context: https://bugs.webkit.org/attachment.cgi?id=94987&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        No new tests. (OOPS!)
> 
> Same as mentioned in bug 61539 review.

Done.

> > Source/WebCore/dom/StreamEvent.h:42
> > +    static PassRefPtr<StreamEvent> create(const AtomicString& type, bool canBubble, bool cancelable, PassRefPtr<Stream>);
> 
> Are you sure that canBubble and cancelable are required here? I'm not completely sure but it sounds to me like all stream events can't bubble and are not cancelable by definition. Take a look to dom/DeviceOrientationEvent.h/cpp for example.

Just following the IDL in the specification, even though I agree that it could be simplified.
Comment 4 Tommy Widenflycht 2011-05-30 05:57:36 PDT
Created attachment 95341 [details]
Patch
Comment 5 Tony Gentilcore 2011-06-07 06:52:01 PDT
Comment on attachment 95341 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95341&action=review

> Source/WebCore/dom/StreamEvent.idl:27
> +    interface [

Might be nice to add a link to the spec here.
Comment 6 Tommy Widenflycht 2011-06-08 04:31:15 PDT
Created attachment 96404 [details]
Patch
Comment 7 Tommy Widenflycht 2011-06-08 04:33:01 PDT
Comment on attachment 95341 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95341&action=review

>> Source/WebCore/dom/StreamEvent.idl:27
>> +    interface [
> 
> Might be nice to add a link to the spec here.

Done.
Comment 8 Tommy Widenflycht 2011-06-08 04:33:26 PDT
Rebased + fixed review comment.
Comment 9 WebKit Review Bot 2011-06-08 06:25:04 PDT
Comment on attachment 96404 [details]
Patch

Clearing flags on attachment: 96404

Committed r88343: <http://trac.webkit.org/changeset/88343>
Comment 10 WebKit Review Bot 2011-06-08 06:25:09 PDT
All reviewed patches have been landed.  Closing bug.