Bug 97673 - MediaStream API: Allow RTCPeerConnection to pass down its Document reference to the embedder
Summary: MediaStream API: Allow RTCPeerConnection to pass down its Document reference ...
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Tommy Widenflycht
Depends on:
Blocks: 56459
  Show dependency treegraph
Reported: 2012-09-26 06:32 PDT by Tommy Widenflycht
Modified: 2012-10-16 01:06 PDT (History)
8 users (show)

See Also:

Patch (37.99 KB, patch)
2012-09-26 06:40 PDT, Tommy Widenflycht
abarth: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
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]

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?