[Webkit-unassigned] [Bug 56922] Media Stream API: add the skeleton of the frame context, the page controller and the embedder client.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=56922





--- Comment #6 from Steve Block <steveblock at google.com>  2011-04-11 09:25:52 PST ---
(From update of attachment 88823)
View in context: https://bugs.webkit.org/attachment.cgi?id=88823&action=review

> Source/WebCore/page/DOMWindow.h:503
> +        OwnPtr<MediaStreamContext> m_mediaStreamContext;

Can he controller own a set of these, rather than polluting DOMWindow?

> Source/WebCore/page/MediaStreamContext.cpp:56
> +    ScriptExecutionContext* m_scriptContext;

Use full name - m_scriptExecutionContext

> Source/WebCore/page/MediaStreamContext.cpp:105
> +void MediaStreamContext::pageDetached()

As discussed, this method probably isn't needed right now.

> Source/WebCore/page/MediaStreamContext.cpp:116
> +// Called on frame destruction despite the name.

Remove

> Source/WebCore/page/MediaStreamContext.cpp:131
> +    if (pageController() == controller)

When can the controller not be our controller?

> Source/WebCore/page/MediaStreamContext.h:44
> +class MediaStreamContext {

Try to avoid generic names like Context - maybe MediaStreamFrameController ?

> Source/WebCore/page/MediaStreamContext.h:61
> +    class IdGenerator {

Why is this a separate class?

> Source/WebCore/page/MediaStreamContext.h:77
> +    void enterDetachedState();

Add a description of what it means to be detached.

> Source/WebCore/page/MediaStreamContext.h:81
> +    bool m_detachedState;

m_isInDetachedState

> Source/WebCore/page/MediaStreamController.cpp:83
> +void MediaStreamController::transferContext(MediaStreamContext* context, MediaStreamController* newController)

Given that this method isn't implemented properly yet, let's not add it yet to avoid bloat. transferToNewPage() can call contextDestroyed() directly.

> Source/WebCore/page/MediaStreamController.h:45
> +    void contextDetached(MediaStreamContext*);

Make more descriptive - removeContentForFrameController

> Source/WebCore/page/MediaStreamController.h:46
> +    void transferContext(MediaStreamContext*, MediaStreamController*);

transferContentForFrameController

> Source/WebCore/page/MediaStreamController.h:49
> +    class RequestContext {

If this class is just for internal use, I don't think there's much need for it. Otherwsie, add an id() method to MediaStreamContext.

> Source/WebCore/page/MediaStreamController.h:67
> +    typedef HashMap<int, RequestContext> RequestContextMap;

keep typedef with with first use

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.