Bug 71458

Summary:

[chromium] MediaStream API: Add WebMediaStreamRegistry

Product:

WebKit

Reporter:

Tommy Widenflycht <tommyw>

Component:

DOM

Assignee:

Tommy Widenflycht <tommyw>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

donggwan.kim, fishd, grunell, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

56459

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Tommy Widenflycht

Reported 2011-11-03 04:23:05 PDT

Add WebMediaStreamRegistry to the MediaStream api.

Attachments
Patch (6.14 KB, patch) 2011-11-03 05:29 PDT, Tommy Widenflycht	no flags	Details Formatted Diff Diff
Patch (6.09 KB, patch) 2011-11-04 02:59 PDT, Tommy Widenflycht	no flags	Details Formatted Diff Diff
Patch (6.03 KB, patch) 2011-11-08 02:22 PST, Tommy Widenflycht	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Tommy Widenflycht

Comment 1 2011-11-03 05:29:28 PDT

Created attachment 113467 [details] Patch

WebKit Review Bot

Comment 2 2011-11-03 05:32:41 PDT

Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.

Darin Fisher (:fishd, Google)

Comment 3 2011-11-03 09:13:40 PDT

Comment on attachment 113467 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113467&action=review > Source/WebKit/chromium/public/WebMediaStreamRegistry.h:36 > + // Returns an empty string if the url doesn't correspond to a Stream object. empty or null? seems like null (WebString::isNull) is what it actually returns. > Source/WebKit/chromium/src/WebMediaStreamRegistry.cpp:44 > + if (streamDescriptor) it seems like you should return WebMediaStreamDescriptor here instead. WebMediaStreamDescriptor has a label() method. it creates an API that is a more lightweight wrapper around what WebCore actually does.

Tommy Widenflycht

Comment 4 2011-11-04 02:59:58 PDT

Created attachment 113640 [details] Patch

Tommy Widenflycht

Comment 5 2011-11-04 03:02:28 PDT

Comment on attachment 113467 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113467&action=review >> Source/WebKit/chromium/public/WebMediaStreamRegistry.h:36 >> + // Returns an empty string if the url doesn't correspond to a Stream object. > > empty or null? seems like null (WebString::isNull) is what it actually returns. Implicitly fixed :) >> Source/WebKit/chromium/src/WebMediaStreamRegistry.cpp:44 >> + if (streamDescriptor) > > it seems like you should return WebMediaStreamDescriptor here instead. WebMediaStreamDescriptor has a label() method. > it creates an API that is a more lightweight wrapper around what WebCore actually does. Fixed, we don't need more than the label right now but this is more future proof with less code. Win/Win :)

Darin Fisher (:fishd, Google)

Comment 6 2011-11-07 09:32:08 PST

Comment on attachment 113640 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113640&action=review > Source/WebKit/chromium/src/WebMediaStreamRegistry.cpp:44 > + return WebMediaStreamDescriptor(MediaStreamRegistry::registry().lookupMediaStreamDescriptor(kurl.string())); nit: it is perhaps a bit more conventional in webkit code to just use KURL(url).string() instead of creating the named temporary variable.

Tommy Widenflycht

Comment 7 2011-11-08 02:22:43 PST

Created attachment 114017 [details] Patch

Tommy Widenflycht

Comment 8 2011-11-08 02:23:12 PST

Comment on attachment 113640 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113640&action=review >> Source/WebKit/chromium/src/WebMediaStreamRegistry.cpp:44 >> + return WebMediaStreamDescriptor(MediaStreamRegistry::registry().lookupMediaStreamDescriptor(kurl.string())); > > nit: it is perhaps a bit more conventional in webkit code to just use KURL(url).string() instead of creating the named temporary variable. Fixed.

WebKit Review Bot

Comment 9 2011-11-09 18:25:25 PST

Comment on attachment 114017 [details] Patch Clearing flags on attachment: 114017 Committed r99796: <http://trac.webkit.org/changeset/99796>

WebKit Review Bot

Comment 10 2011-11-09 18:25:31 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.