Bug 84942

Summary: MediaStream API: Change LocalMediaStream::stop to be synchronous
Product: WebKit Reporter: Tommy Widenflycht <tommyw>
Component: WebCore Misc.Assignee: Tommy Widenflycht <tommyw>
Status: RESOLVED FIXED    
Severity: Normal CC: adam.bergkvist, eric.carlson, feature-media-reviews, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 56459    
Attachments:
Description Flags
Patch
none
Patch none

Description Tommy Widenflycht 2012-04-26 04:02:33 PDT
Since I changed LocalMediaStream to be a ActiveDOMObject recently the stop behaviour needs to change since it is no longer a good idea to start a timer when called.
Comment 1 Tommy Widenflycht 2012-04-26 04:06:09 PDT
Created attachment 138970 [details]
Patch
Comment 2 James Robinson 2012-04-30 01:21:22 PDT
Comment on attachment 138970 [details]
Patch

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

> Source/WebCore/Modules/mediastream/LocalMediaStream.cpp:2
> + * Copyright (C) 2012 Google Inc. All rights reserved.

we don't do this in WebKit, just leave the date alone

> Source/WebCore/Modules/mediastream/LocalMediaStream.h:2
> + * Copyright (C) 2012 Google Inc. All rights reserved.

same here
Comment 3 Tommy Widenflycht 2012-04-30 01:39:50 PDT
Created attachment 139426 [details]
Patch
Comment 4 Tommy Widenflycht 2012-04-30 01:41:16 PDT
Comment on attachment 138970 [details]
Patch

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

>> Source/WebCore/Modules/mediastream/LocalMediaStream.cpp:2
>> + * Copyright (C) 2012 Google Inc. All rights reserved.
> 
> we don't do this in WebKit, just leave the date alone

Done.

>> Source/WebCore/Modules/mediastream/LocalMediaStream.h:2
>> + * Copyright (C) 2012 Google Inc. All rights reserved.
> 
> same here

Done.
Comment 5 Adam Bergkvist 2012-04-30 07:18:20 PDT
This patch changes specified (and testable) API behavior. The "ended" event will be dispatched synchronously when LocalMediaStream.stop() is called from JavaScript.

I don't think calling MediaStreamCenter::didStopLocalMediaStream() from ActiveDOMObject::stop() is the right approach. I've posted a bug to discuss this. See http://webkit.org/b/85191
Comment 6 WebKit Review Bot 2012-04-30 10:43:55 PDT
Comment on attachment 139426 [details]
Patch

Clearing flags on attachment: 139426

Committed r115649: <http://trac.webkit.org/changeset/115649>
Comment 7 WebKit Review Bot 2012-04-30 10:43:59 PDT
All reviewed patches have been landed.  Closing bug.