Bug 61537

Summary: Media Stream PeerConnection API: adding StreamEvent class
Product: WebKit Reporter: Tommy Widenflycht <tommyw>
Component: DOMAssignee: Tommy Widenflycht <tommyw>
Status: RESOLVED FIXED    
Severity: Normal CC: leandrogracia, tonyg, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 56666    
Bug Blocks: 56459    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

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.