Cast Extensions API: Factory method for creating a cast session

Cast Extensions API: Factory method for creating a cast session

There are separate chrome.webrtc.castSendTransport.create and
chrome.webrtc.castUdpTransport.create methods. However these objects
are never used alone. They require a session to operate and having them
separate is confusing.

Instead unify the session creation in one factory method:
chrome.cast.streaming.session.create(
  WebMediaStreamTrack audioTrack,
  WebMediaStreamTrack videoTrack,
  CreateCallback);

The above method will construct a session and create objects for
the RTP streams and the UDP transport. These streams can then be
configured by existing APIs.

Another change is the new cast.streaming namespace. Eventually
everything in chrome.webrtc will be moved there so we have a mix for
now. But we'll unify all the namespaces to cast.streaming in a couple
patches.

To summarize:

The following two APIs are removed:
1. chrome.webrtc.castSendTransport.create
2. chorme.webrtc.castUdpTransport.create

The following API is new:
1. chrome.cast.streaming.session.create

BUG=301920
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238254

[haibinlu, 2013-11-23 02:10:00]

https://codereview.chromium.org/83043005/diff/20001/chrome/common/extensions/...
File chrome/common/extensions/api/cast_streaming.idl (right):

https://codereview.chromium.org/83043005/diff/20001/chrome/common/extensions/...
chrome/common/extensions/api/cast_streaming.idl:10: // |id| : The transport id.
document parameters.

https://codereview.chromium.org/83043005/diff/20001/chrome/common/extensions/...
chrome/common/extensions/api/cast_streaming.idl:11: callback CreateCallback =
void (long audioId,
audioId and vidoeId are unclear.

https://codereview.chromium.org/83043005/diff/20001/chrome/common/extensions/...
chrome/common/extensions/api/cast_streaming.idl:13: long transportId);
udpTransportId?

https://codereview.chromium.org/83043005/diff/20001/chrome/common/extensions/...
File chrome/common/extensions/api/webrtc_cast_send_transport.idl (right):

https://codereview.chromium.org/83043005/diff/20001/chrome/common/extensions/...
chrome/common/extensions/api/webrtc_cast_send_transport.idl:67: [nocompile]
static void create(
remove this create method too?

[haibinlu, 2013-11-23 02:11:52]

lgtm

[not at google - send to devlin, 2013-11-25 19:21:20]

Could you do the API rename before submitting this CL? What will this look like
eventually?

[Alpha Left Google, 2013-11-25 19:23:08]

On 2013/11/25 19:21:20, kalman wrote:
> Could you do the API rename before submitting this CL? What will this look
like
> eventually?

There's no renaming done in this CL. I removed old APIs from chrome.webrtc and
then added a new API in chrome.cast.streaming.

Eventually all APIs will be moved from chrome.webrtc to chrome.cast.streaming.

[Alpha Left Google, 2013-11-25 19:28:47]

> There's no renaming done in this CL. I removed old APIs from chrome.webrtc and
> then added a new API in chrome.cast.streaming.
> 
> Eventually all APIs will be moved from chrome.webrtc to chrome.cast.streaming.

So to summarize this change. I removed two APIs:

1. chrome.webrtc.castSendTransport.create
2. chrome.webrtc.castUdpTransport.create

Added a new API and namespace:

1. chrome.cast.streaming.createSession

The remaining APIs are unchanged.

Eventually the set of APIs will be:
1. chrome.cast.streaming.createSession
2. chrome.cast.streaming.rtpStream.* (Moved from
chrome.webrtc.castSendTransport)
2. chrome.cast.streaming.udpTransport.* (Moved from
chrome.webrtc.castUdpTransport)

[not at google - send to devlin, 2013-11-25 19:28:48]

On 2013/11/25 19:23:08, Alpha wrote:
> On 2013/11/25 19:21:20, kalman wrote:
> > Could you do the API rename before submitting this CL? What will this look
> like
> > eventually?
> 
> There's no renaming done in this CL. I removed old APIs from chrome.webrtc and
> then added a new API in chrome.cast.streaming.
> 
> Eventually all APIs will be moved from chrome.webrtc to chrome.cast.streaming.

I understand that, but you may have problems transitioning to the new namespace
depending on how the API is laid out.

[Alpha Left Google, 2013-11-25 19:32:50]

> I understand that, but you may have problems transitioning to the new
namespace
> depending on how the API is laid out.

I was thinking about this as well. My intention was to add a new API and
deprecate two APIs. I would have to add files for this new API anyway
(potentially under chrome.webrtc). I figured that it might be handy for me to
add this new API in the new namespace (chrome.cast.streaming) instead of
defining it under chrome.webrtc and then move it later.

[not at google - send to devlin, 2013-11-25 19:33:26]

On 2013/11/25 19:28:47, Alpha wrote:
> > There's no renaming done in this CL. I removed old APIs from chrome.webrtc
and
> > then added a new API in chrome.cast.streaming.
> > 
> > Eventually all APIs will be moved from chrome.webrtc to
chrome.cast.streaming.
> 
> So to summarize this change. I removed two APIs:
> 
> 1. chrome.webrtc.castSendTransport.create
> 2. chrome.webrtc.castUdpTransport.create
> 
> Added a new API and namespace:
> 
> 1. chrome.cast.streaming.createSession
> 
> The remaining APIs are unchanged.
> 
> Eventually the set of APIs will be:
> 1. chrome.cast.streaming.createSession
> 2. chrome.cast.streaming.rtpStream.* (Moved from
> chrome.webrtc.castSendTransport)
> 2. chrome.cast.streaming.udpTransport.* (Moved from
> chrome.webrtc.castUdpTransport)

Ok, so you're going to have problems doing this, our bindings don't support both
functions/events/properties and sub-namespaces on the same namespace. This is
because we set up stubs for every namespace which set up the full namespace on
first access.

We could lose the laziness for chrome.cast.streaming but I'd rather not.

One way to avoid this would be to put all APIs in that single cast_streaming.idl
file, and have function names like "rtpStream.foo", "udpTransport.bar", etc,
however our bindings doesn't support that either :) but it'd be much easier to
add than the alternative (fixing laziness). It's be a pretty simple change in
binding.js.

So... now that you're aware of that... and as long as you're happy with that...
I'll review properly.

[Alpha Left Google, 2013-11-25 19:37:22]

On 2013/11/25 19:33:26, kalman wrote:
> On 2013/11/25 19:28:47, Alpha wrote:
> > > There's no renaming done in this CL. I removed old APIs from chrome.webrtc
> and
> > > then added a new API in chrome.cast.streaming.
> > > 
> > > Eventually all APIs will be moved from chrome.webrtc to
> chrome.cast.streaming.
> > 
> > So to summarize this change. I removed two APIs:
> > 
> > 1. chrome.webrtc.castSendTransport.create
> > 2. chrome.webrtc.castUdpTransport.create
> > 
> > Added a new API and namespace:
> > 
> > 1. chrome.cast.streaming.createSession
> > 
> > The remaining APIs are unchanged.
> > 
> > Eventually the set of APIs will be:
> > 1. chrome.cast.streaming.createSession
> > 2. chrome.cast.streaming.rtpStream.* (Moved from
> > chrome.webrtc.castSendTransport)
> > 2. chrome.cast.streaming.udpTransport.* (Moved from
> > chrome.webrtc.castUdpTransport)
> 
> Ok, so you're going to have problems doing this, our bindings don't support
both
> functions/events/properties and sub-namespaces on the same namespace. This is
> because we set up stubs for every namespace which set up the full namespace on
> first access.
> 
> We could lose the laziness for chrome.cast.streaming but I'd rather not.
> 
> One way to avoid this would be to put all APIs in that single
cast_streaming.idl
> file, and have function names like "rtpStream.foo", "udpTransport.bar", etc,
> however our bindings doesn't support that either :) but it'd be much easier to
> add than the alternative (fixing laziness). It's be a pretty simple change in
> binding.js.
> 
> So... now that you're aware of that... and as long as you're happy with
that...
> I'll review properly.

Doh! What about I lay it out like this:

chrome.cast.streaming.session.create
chrome.cast.streaming.rtpStream.*
chrome.cast.streaming.udpTransport.*

[not at google - send to devlin, 2013-11-25 19:41:11]

Could you send out a courtesy email to apps-dev@chromium.org that you're adding
this new namespace, and the justification for why? I can't lg this
effectively-new-API without going through that first, though, it does lg.

https://codereview.chromium.org/83043005/diff/130001/chrome/renderer/extensio...
File chrome/renderer/extensions/webrtc_native_handler.cc (right):

https://codereview.chromium.org/83043005/diff/130001/chrome/renderer/extensio...
chrome/renderer/extensions/webrtc_native_handler.cc:211: 3, callback_args);
so this will definitely be async eventually, right? because you might want to
add that behaviour now, namely, post a task to MessageLoop::current() to call
this back, not do it right away.

https://codereview.chromium.org/83043005/diff/130001/chrome/renderer/media/ca...
File chrome/renderer/media/cast_udp_transport.h (right):

https://codereview.chromium.org/83043005/diff/130001/chrome/renderer/media/ca...
chrome/renderer/media/cast_udp_transport.h:19: CastUdpTransport(const
scoped_refptr<CastSession>& session);
explicit

but oops I didn't mean to go into media/* :)

[not at google - send to devlin, 2013-11-25 19:41:46]

On 2013/11/25 19:37:22, Alpha wrote:
> On 2013/11/25 19:33:26, kalman wrote:
> > On 2013/11/25 19:28:47, Alpha wrote:
> > > > There's no renaming done in this CL. I removed old APIs from
chrome.webrtc
> > and
> > > > then added a new API in chrome.cast.streaming.
> > > > 
> > > > Eventually all APIs will be moved from chrome.webrtc to
> > chrome.cast.streaming.
> > > 
> > > So to summarize this change. I removed two APIs:
> > > 
> > > 1. chrome.webrtc.castSendTransport.create
> > > 2. chrome.webrtc.castUdpTransport.create
> > > 
> > > Added a new API and namespace:
> > > 
> > > 1. chrome.cast.streaming.createSession
> > > 
> > > The remaining APIs are unchanged.
> > > 
> > > Eventually the set of APIs will be:
> > > 1. chrome.cast.streaming.createSession
> > > 2. chrome.cast.streaming.rtpStream.* (Moved from
> > > chrome.webrtc.castSendTransport)
> > > 2. chrome.cast.streaming.udpTransport.* (Moved from
> > > chrome.webrtc.castUdpTransport)
> > 
> > Ok, so you're going to have problems doing this, our bindings don't support
> both
> > functions/events/properties and sub-namespaces on the same namespace. This
is
> > because we set up stubs for every namespace which set up the full namespace
on
> > first access.
> > 
> > We could lose the laziness for chrome.cast.streaming but I'd rather not.
> > 
> > One way to avoid this would be to put all APIs in that single
> cast_streaming.idl
> > file, and have function names like "rtpStream.foo", "udpTransport.bar", etc,
> > however our bindings doesn't support that either :) but it'd be much easier
to
> > add than the alternative (fixing laziness). It's be a pretty simple change
in
> > binding.js.
> > 
> > So... now that you're aware of that... and as long as you're happy with
> that...
> > I'll review properly.
> 
> Doh! What about I lay it out like this:
> 
> chrome.cast.streaming.session.create
> chrome.cast.streaming.rtpStream.*
> chrome.cast.streaming.udpTransport.*

that would work too, I suggest you include that in your email :)

[Alpha Left Google, 2013-11-26 04:22:44]

https://codereview.chromium.org/83043005/diff/130001/chrome/renderer/extensio...
File chrome/renderer/extensions/webrtc_native_handler.cc (right):

https://codereview.chromium.org/83043005/diff/130001/chrome/renderer/extensio...
chrome/renderer/extensions/webrtc_native_handler.cc:211: 3, callback_args);
On 2013/11/25 19:41:11, kalman wrote:
> so this will definitely be async eventually, right? because you might want to
> add that behaviour now, namely, post a task to MessageLoop::current() to call
> this back, not do it right away.

Okay done.

https://codereview.chromium.org/83043005/diff/130001/chrome/renderer/media/ca...
File chrome/renderer/media/cast_udp_transport.h (right):

https://codereview.chromium.org/83043005/diff/130001/chrome/renderer/media/ca...
chrome/renderer/media/cast_udp_transport.h:19: CastUdpTransport(const
scoped_refptr<CastSession>& session);
On 2013/11/25 19:41:11, kalman wrote:
> explicit
> 
> but oops I didn't mean to go into media/* :)

Done.

[not at google - send to devlin, 2013-11-26 18:51:45]

lgtm

[Alpha Left Google, 2013-11-26 20:03:23]

+scherkus for files under chrome/renderer/media.

[Alpha Left Google, 2013-11-27 22:16:22]

On 2013/11/26 20:03:23, Alpha wrote:
> +scherkus for files under chrome/renderer/media.

ping scherkus@.

[scherkus (not reviewing), 2013-11-27 22:30:51]

lgtm

[commit-bot: I haz the power, 2013-11-27 23:39:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/83043005/420001

[Alpha Left Google, 2013-11-27 23:54:53]

+thakis for owners approval under chrome/renderer/resources

[Alpha Left Google, 2013-11-27 23:55:09]

+thakis for owners approval under chrome/renderer/resources

[Nico, 2013-11-27 23:57:11]

lgtm

https://codereview.chromium.org/83043005/diff/420001/chrome/renderer/resource...
File
chrome/renderer/resources/extensions/cast_streaming_session_custom_bindings.js
(right):

https://codereview.chromium.org/83043005/diff/420001/chrome/renderer/resource...
chrome/renderer/resources/extensions/cast_streaming_session_custom_bindings.js:14:
natives.CreateSession(audioTrack, videoTrack, callback);
nit: indent 4 more

[commit-bot: I haz the power, 2013-11-28 01:23:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/83043005/460001

[commit-bot: I haz the power, 2013-11-28 02:04:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/83043005/460001

[commit-bot: I haz the power, 2013-11-28 02:32:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/83043005/460001

[commit-bot: I haz the power, 2013-11-28 03:03:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/83043005/460001

[commit-bot: I haz the power, 2013-11-28 03:32:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/83043005/460001

[commit-bot: I haz the power, 2013-11-28 03:50:58]

Failed to trigger a try job on linux_rel
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-11-28 03:51:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/83043005/480001

[commit-bot: I haz the power, 2013-11-28 03:51:41]

Failed to apply patch for
chrome/common/extensions/permissions/chrome_api_permissions.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/common/extensions/permissions/chrome_api_permissions.cc
  Hunk #1 FAILED at 354.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/common/extensions/permissions/chrome_api_permissions.cc.rej

Patch:       chrome/common/extensions/permissions/chrome_api_permissions.cc
Index: chrome/common/extensions/permissions/chrome_api_permissions.cc
diff --git a/chrome/common/extensions/permissions/chrome_api_permissions.cc
b/chrome/common/extensions/permissions/chrome_api_permissions.cc
index
10dbaef1880e838d21d115343a69d8ad451bedc0..2e00d4f38e73a76cced1544586250e55ef271804
100644
--- a/chrome/common/extensions/permissions/chrome_api_permissions.cc
+++ b/chrome/common/extensions/permissions/chrome_api_permissions.cc
@@ -354,6 +354,7 @@ std::vector<APIPermissionInfo*>
ChromeAPIPermissions::GetAllPermissions()
     { APIPermission::kFullscreen, "fullscreen" },
     { APIPermission::kAudio, "audio" },
     { APIPermission::kWebRtc, "webrtc" },
+    { APIPermission::kCastStreaming, "cast.streaming" },
 
     // Settings override permissions.
     { APIPermission::kHomepage, "homepage",

[commit-bot: I haz the power, 2013-12-02 16:50:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/83043005/500001

[commit-bot: I haz the power, 2013-12-02 19:09:11]

Retried try job too often on mac_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-12-02 19:20:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/83043005/500001

[commit-bot: I haz the power, 2013-12-02 21:19:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/83043005/520001

[commit-bot: I haz the power, 2013-12-03 00:53:52]

Change committed as 238254