Bug 64794 - MediaStream API: Merging MediaStreamContainer and MediaStreamList
Summary: MediaStream API: Merging MediaStreamContainer and MediaStreamList
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:
Blocks: 56459
  Show dependency tree / graph
 
Reported: 2011-07-19 06:18 PDT by Tommy Widenflycht
Modified: 2011-07-21 03:05 PDT (History)
3 users (show)

See Also:

Attachments
Patch (13.92 KB, patch)
2011-07-19 06:27 PDT, Tommy Widenflycht 		no flags Details | Formatted Diff | Diff
Patch (13.76 KB, patch)
2011-07-20 07:17 PDT, Tommy Widenflycht 		no flags Details | Formatted Diff | Diff
Patch (13.68 KB, patch)
2011-07-21 01:41 PDT, Tommy Widenflycht 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tommy Widenflycht 2011-07-19 06:18:33 PDT 
Having a separate container class doesn't make any sense.
Comment 1 Tommy Widenflycht 2011-07-19 06:27:37 PDT 
Created attachment 101308 [details]
Patch
Comment 2 Tony Gentilcore 2011-07-20 06:35:24 PDT 
Comment on attachment 101308 [details]
Patch

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

Thanks for the cleanup. Just a nit about includes to address before submission.

> Source/WebCore/dom/MediaStreamList.h:30
> +#include "MediaStream.h"

Is this really necessary here? At first glance it seems like the forward declaration should suffice. If it is necessary, please remove the forward declaration, otherwise please add this in the cpp.
Comment 3 Tommy Widenflycht 2011-07-20 07:15:39 PDT 
Comment on attachment 101308 [details]
Patch

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

>> Source/WebCore/dom/MediaStreamList.h:30
>> +#include "MediaStream.h"
> 
> Is this really necessary here? At first glance it seems like the forward declaration should suffice. If it is necessary, please remove the forward declaration, otherwise please add this in the cpp.

It seems to be, the hashmap goes nuts without a real implementation.

Have removed the forward definition of MediaStream.
Comment 4 Tommy Widenflycht 2011-07-20 07:17:44 PDT 
Created attachment 101466 [details]
Patch
Comment 5 WebKit Review Bot 2011-07-20 07:57:40 PDT 
Comment on attachment 101466 [details]
Patch

Rejecting attachment 101466 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 2

Last 500 characters of output:
-14 lines).
Hunk #4 succeeded at 22238 (offset -18 lines).
2 out of 4 hunks FAILED -- saving rejects to file Source/WebCore/WebCore.xcodeproj/project.pbxproj.rej
patching file Source/WebCore/dom/MediaStreamContainer.h
rm 'Source/WebCore/dom/MediaStreamContainer.h'
patching file Source/WebCore/dom/MediaStreamList.cpp
patching file Source/WebCore/dom/MediaStreamList.h

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/9209261
Comment 6 Tommy Widenflycht 2011-07-21 01:41:02 PDT 
Created attachment 101561 [details]
Patch

Rebase
Comment 7 WebKit Review Bot 2011-07-21 03:05:54 PDT 
Comment on attachment 101561 [details]
Patch

Clearing flags on attachment: 101561

Committed r91455: <http://trac.webkit.org/changeset/91455>
Comment 8 WebKit Review Bot 2011-07-21 03:05:59 PDT 
All reviewed patches have been landed.  Closing bug.