Bug 97673

Summary: MediaStream API: Allow RTCPeerConnection to pass down its Document reference to the embedder
Product: WebKit Reporter: Tommy Widenflycht <tommyw>
Component: WebCore Misc.Assignee: Tommy Widenflycht <tommyw>
Status: RESOLVED INVALID    
Severity: Normal CC: abarth, dglazkov, eric.carlson, feature-media-reviews, fishd, jamesr, tkent+wkapi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 56459    
Attachments:
Description Flags
Patch abarth: review-

Description Tommy Widenflycht 2012-09-26 06:32:05 PDT
This patch tries to accomplish the same thing as my recent UserMediaRequest patch (97095), namely to pass down a Document reference to the embedder. 
However since this code lives under WebCore/platform there isn't a clear way to do so. Creating a frame client and sending it down that way has been suggested but that would require even more cludges. In that light something like this might be a compromise?
Comment 1 Tommy Widenflycht 2012-09-26 06:40:29 PDT
Created attachment 165786 [details]
Patch
Comment 2 Tommy Widenflycht 2012-09-26 06:41:26 PDT
Note, this patch is for discussion only at this stage.
Comment 3 WebKit Review Bot 2012-09-26 06:46:18 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 4 Adam Barth 2012-09-26 09:06:20 PDT
Comment on attachment 165786 [details]
Patch

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

> Source/Platform/chromium/public/WebRTCPeerConnectionHandler.h:35
> +class WebDocument;

You can't reference WebDocument in Source/Platform.  Document is a DOM-level concept, not a Platform-level concept.

> Source/WebCore/platform/mediastream/RTCPeerConnectionHandler.cpp:50
> -    virtual bool initialize(PassRefPtr<RTCConfiguration>, PassRefPtr<MediaConstraints>) OVERRIDE;
> +    virtual bool initialize(Document*, PassRefPtr<RTCConfiguration>, PassRefPtr<MediaConstraints>) OVERRIDE;

This is also a layering violation.  Code in WebCore/platform shouldn't know anything about Document.

> Source/WebKit/chromium/src/RTCPeerConnectionHandlerChromium.cpp:249
> +namespace WebCore {
> +
> +PassOwnPtr<RTCPeerConnectionHandler> RTCPeerConnectionHandler::create(RTCPeerConnectionHandlerClient* client)
> +{
> +    return adoptPtr(new WebKit::RTCPeerConnectionHandlerChromium(client));
> +}
> +
> +} // namespace WebCore

This is wrong.  We shouldn't have code in Source/WebKit/chromium that's in the WebCore namespace.  I know other features do this, but we're trying to unwind all these layering violations so we can split WebCore and WebKit into separate DLLs.
Comment 5 Adam Barth 2012-09-26 09:07:44 PDT
Maybe we should talk in a more interactive medium to work out this issue?