Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
398873005:
Preparations for Cast Host Extension (Closed)
DescriptionPreparations for Cast Host Extension
This CL contains all the modifications to existing code that are needed
to make way for the CastHostExtension and related files. The extension
is hidden behind a command line flag and without it, no modification
affects regular functionality of the chromoting host.
Patch Set 1 #
Total comments: 3
Patch Set 2 : Added remoting_me2me_host #
Total comments: 19
Patch Set 3 : Clarified Cast Mode and Removed Me2Me Host #
Total comments: 8
Patch Set 4 : Removed enable-cast flag #Patch Set 5 : Removed unnecessary getter methods #
Total comments: 5
Messages
Total messages: 13 (0 generated)
As planned, I'm breaking down the large CL of my work into several small CLs, to make them easier for review. This one just contains the small modifications needed to allow for "switching" of video streaming from the usual way to WebRTC. This work basically fits into the large CL. The link is https://codereview.chromium.org/345463008/, just in case you want to see how the rest of the code uses some particular change here. https://codereview.chromium.org/398873005/diff/1/remoting/host/chromoting_hos... File remoting/host/chromoting_host_context.cc (right): https://codereview.chromium.org/398873005/diff/1/remoting/host/chromoting_hos... remoting/host/chromoting_host_context.cc:38: "ChromotingCaptureThread", ui_task_runner_, base::MessageLoop::TYPE_IO); Changing the capture task runner's MessageLoop to TYPE_IO was needed to accommodate peer connection, which, using capture_task_runner as a worker thread, tries to call a PortAllocator method that requires TYPE_IO MessageLoop. I'd be happy to discuss this more/point out this behavior and find other solutions if this is not acceptable. https://codereview.chromium.org/398873005/diff/1/remoting/host/client_session.h File remoting/host/client_session.h (right): https://codereview.chromium.org/398873005/diff/1/remoting/host/client_session... remoting/host/client_session.h:178: return screen_capturer_.Pass(); TODO(aiguha): Discuss with sergeyu@ a better design for sharing the screen capturer. https://codereview.chromium.org/398873005/diff/1/remoting/host/client_session... remoting/host/client_session.h:179: } CastExtensionSession's constructor receives a pointer to the current ClientSession. These methods were the shortest way to give CastExtensionSession access to these objects. The CastExtensionSession needs the two task runners to run PeerConenctionFactory on, and the ScreenCapturer to create the VideoSource used by PeerConnection.
Added a small clarifying comment. https://codereview.chromium.org/398873005/diff/20001/remoting/host/remoting_m... File remoting/host/remoting_me2me_host.cc (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/remoting_m... remoting/host/remoting_me2me_host.cc:1232: } Lines 1228-1230 are commented out only so I could break the CLs down. They are very much needed and used. This is the one of the only points of contact between the existing code and the CastExtension code.
Instead of adding enable_cast_ flag, would it be easier to add something like "ReleaseScreenCapturer()" in VideoScheduler and ClientSession. https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... File remoting/host/client_session.cc (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.cc:152: // If cast is enabled, do nothing. This doesn't look right. if enable_cast_ is true it doesn't mean that the host is always casting. I think it should indicate that the host supports casting when the client supports it and has it enabled. https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.cc:335: if (!enable_cast_) { Same as above - you still want VideoScheduler to work if the client doesn't support casting or has it disabled.
https://codereview.chromium.org/398873005/diff/20001/remoting/host/chromoting... File remoting/host/chromoting_host.h (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/chromoting... remoting/host/chromoting_host.h:160: void set_enable_cast(bool enable) { Add a comment to explain what it means to "enable_cast"; does it mean that Cast can be toggled on/off, or that we output to Cast and not to the Chromoting client? https://codereview.chromium.org/398873005/diff/20001/remoting/host/chromoting... File remoting/host/chromoting_host_context.cc (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/chromoting... remoting/host/chromoting_host_context.cc:38: "ChromotingCaptureThread", ui_task_runner_, base::MessageLoop::TYPE_IO); Why does this need to be an IO thread now? https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... File remoting/host/client_session.cc (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.cc:152: // If cast is enabled, do nothing. On 2014/07/17 19:00:43, Sergey Ulanov wrote: > This doesn't look right. if enable_cast_ is true it doesn't mean that the host > is always casting. I think it should indicate that the host supports casting > when the client supports it and has it enabled. My understanding is that |enable_cast_| means "Output video to Cast, don't output video to the Chromoting client at all" - if |enable_cast_| is true then we don't even create the VideoScheduler, below, it seems. https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.cc:382: video_scheduler_->Start(); nit: Suggest "if (video_scheduler_)" instead https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... File remoting/host/client_session.h (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.h:169: void set_enable_cast(bool enable) { enable_cast_ = enable; } As for ChromotingHost, clarify what this actually means. https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.h:177: scoped_ptr<webrtc::ScreenCapturer> RequestScreenCapturer() { This actually releases and returns the ScreenCapturer - why do we need that here? https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.h:285: // Used to capture screens. This is a class member to allow external objects, nit: "Used to capture screens" isn't very helpful ;) You might say "Used to capture desktop contents" or something along those lines. https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.h:286: // like CastExtensionSession, to request it when needed. Use by what? Does the normal Chromoting video channel use it, for example? https://codereview.chromium.org/398873005/diff/20001/remoting/host/remoting_m... File remoting/host/remoting_me2me_host.cc (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/remoting_m... remoting/host/remoting_me2me_host.cc:1230: // host_->AddExtension(extension.Pass()); You shouldn't need to do this; you can land the CLs as: 1. Add the necessary hooks into the Chromoting Host. 2. Add the CastExtension. 3. Use the CastExtension in the Me2Me host. So these changes belong in the final CL, not this base CL. https://codereview.chromium.org/398873005/diff/20001/remoting/host/remoting_m... remoting/host/remoting_me2me_host.cc:1231: host_->set_enable_cast(enable_cast_); As implemented this CL causes the host to disable Chromoting video entirely if the EnableCast switch is provided - how can that work without the extension to negotiate Cast output?
Cast-mode is now much better defined, in documentation and code. Switching from regular video streaming to WebRTC is supported on a per-session basis. This CL still contains only the hooks needed in existing code. (The actual code that makes per-session switching possible is in a different CL). https://codereview.chromium.org/398873005/diff/20001/remoting/host/chromoting... File remoting/host/chromoting_host.h (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/chromoting... remoting/host/chromoting_host.h:160: void set_enable_cast(bool enable) { On 2014/07/17 22:56:36, Wez wrote: > Add a comment to explain what it means to "enable_cast"; does it mean that Cast > can be toggled on/off, or that we output to Cast and not to the Chromoting > client? Done. https://codereview.chromium.org/398873005/diff/20001/remoting/host/chromoting... File remoting/host/chromoting_host_context.cc (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/chromoting... remoting/host/chromoting_host_context.cc:38: "ChromotingCaptureThread", ui_task_runner_, base::MessageLoop::TYPE_IO); On 2014/07/17 22:56:36, Wez wrote: > Why does this need to be an IO thread now? Discussed with wez@ offline but unresolved. To summarize, the network and capture threads are provided to PeerConnection as signalling and worker threads respectively. During its ICE establishment, PeerConnection at some point calls remoting::ChromiumSocketFactory::CreateUdpSocket() on the worker thread (capturer) and fails because of non-IO Message Loop. It looks like this method is called several times on the correct thread (i.e., this is a webrtc bug), but I could be mistaken. Wez suggested we give PeerConnection two new chromium-compatible, IO threads rather than attempt to map our network/capture to their signalling/worker. I attempted to do this, but have two concerns: 1. ChromotingHostContext is intended to manage all threads for the chromoting host process, so it seems incorrect to create threads outside, in a HostExtensionSession. I could create two here and pass them down, but that sounds messy. 2. The CastExtensionSession will need to communicate with ClientSession to request the screen capturer, which must be done on network/capture thread. (this is manageable I suppose). Any advice? https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... File remoting/host/client_session.cc (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.cc:152: // If cast is enabled, do nothing. On 2014/07/17 22:56:36, Wez wrote: > On 2014/07/17 19:00:43, Sergey Ulanov wrote: > > This doesn't look right. if enable_cast_ is true it doesn't mean that the host > > is always casting. I think it should indicate that the host supports casting > > when the client supports it and has it enabled. > > My understanding is that |enable_cast_| means "Output video to Cast, don't > output video to the Chromoting client at all" - if |enable_cast_| is true then > we don't even create the VideoScheduler, below, it seems. Thanks. Both of your comments helped me catch a bad assumption I was making, namely that if the flag was passed to the host then we're definitely casting. It should really mean, as Sergey said, that the host supports it and can switch to it if the client asks. I've rewired some things and now, the ClientSession uses default video unless the client requests cast mode. The switching is now at a per-session basis. Wez and I discussed that this would be good enough for now. Sergey? https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.cc:335: if (!enable_cast_) { On 2014/07/17 19:00:43, Sergey Ulanov wrote: > Same as above - you still want VideoScheduler to work if the client doesn't > support casting or has it disabled. Done. https://codereview.chromium.org/398873005/diff/20001/remoting/host/client_ses... remoting/host/client_session.cc:382: video_scheduler_->Start(); On 2014/07/17 22:56:36, Wez wrote: > nit: Suggest "if (video_scheduler_)" instead As discussed above, this is now (as originally) always started. https://codereview.chromium.org/398873005/diff/20001/remoting/host/remoting_m... File remoting/host/remoting_me2me_host.cc (right): https://codereview.chromium.org/398873005/diff/20001/remoting/host/remoting_m... remoting/host/remoting_me2me_host.cc:1230: // host_->AddExtension(extension.Pass()); On 2014/07/17 22:56:37, Wez wrote: > You shouldn't need to do this; you can land the CLs as: > > 1. Add the necessary hooks into the Chromoting Host. > 2. Add the CastExtension. > 3. Use the CastExtension in the Me2Me host. > > So these changes belong in the final CL, not this base CL. Done. https://codereview.chromium.org/398873005/diff/40001/remoting/host/chromoting... File remoting/host/chromoting_host_context.cc (right): https://codereview.chromium.org/398873005/diff/40001/remoting/host/chromoting... remoting/host/chromoting_host_context.cc:39: video_encode_task_runner_ = AutoThread::Create( Please see comments in previous patch set regarding IO thread. https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... File remoting/host/client_session.cc (right): https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... remoting/host/client_session.cc:501: return desktop_environment_->CreateVideoCapturer(); As of now, this method is called on the network thread. Calling video_scheduler_->Stop() here triggers the deletion of the old ScreenCapturer on the capture thread. We then createand return a new ScreenCapturer immediately after. This works in practice (on Linux atleast), but could there be a race condition between deletion and creation? Or is there no such constraint on Linux?
https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... File remoting/host/client_session.cc (right): https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... remoting/host/client_session.cc:489: if(!enable_cast_) { I still think you don't need this flag. RequestScreenCapturer() will only be called from extension and extension shouldn't exist if cast is not enabled. https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... File remoting/host/client_session.h (right): https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... remoting/host/client_session.h:182: scoped_refptr<base::SingleThreadTaskRunner> video_capture_task_runner() { I'm not sure why you need these methods here. Can you pass task runners to extension when it's created?
I've removed the unnecessary usage of the enable_cast flag. The only remaining question is regarding the task runners and how we pass them to the CastExtensionSession. https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... File remoting/host/client_session.cc (right): https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... remoting/host/client_session.cc:489: if(!enable_cast_) { On 2014/07/24 19:11:36, Sergey Ulanov wrote: > I still think you don't need this flag. RequestScreenCapturer() will only be > called from extension and extension shouldn't exist if cast is not enabled. Acknowledged. https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... File remoting/host/client_session.h (right): https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... remoting/host/client_session.h:182: scoped_refptr<base::SingleThreadTaskRunner> video_capture_task_runner() { On 2014/07/24 19:11:36, Sergey Ulanov wrote: > I'm not sure why you need these methods here. Can you pass task runners to > extension when it's created? Yes I can pass the task runners to the extension constructor and save refptrs. I thought that creating class variables just to pass them on when constructing the ExtensionSession might not be good style. If you think it's fine to do that, I'll make the change.
https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... File remoting/host/client_session.h (right): https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... remoting/host/client_session.h:182: scoped_refptr<base::SingleThreadTaskRunner> video_capture_task_runner() { On 2014/07/28 17:11:56, aiguha1 wrote: > On 2014/07/24 19:11:36, Sergey Ulanov wrote: > > I'm not sure why you need these methods here. Can you pass task runners to > > extension when it's created? > > Yes I can pass the task runners to the extension constructor and save refptrs. I > thought that creating class variables just to pass them on when constructing the > ExtensionSession might not be good style. If you think it's fine to do that, > I'll make the change. Yes - it's the same mechanism we use in other places. E.g. ChromotingHost stores video_capture_task_runner_ only to pass it to ClientSession instances.
https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... File remoting/host/client_session.h (right): https://codereview.chromium.org/398873005/diff/40001/remoting/host/client_ses... remoting/host/client_session.h:182: scoped_refptr<base::SingleThreadTaskRunner> video_capture_task_runner() { On 2014/07/28 17:19:06, Sergey Ulanov wrote: > On 2014/07/28 17:11:56, aiguha1 wrote: > > On 2014/07/24 19:11:36, Sergey Ulanov wrote: > > > I'm not sure why you need these methods here. Can you pass task runners to > > > extension when it's created? > > > > Yes I can pass the task runners to the extension constructor and save refptrs. > I > > thought that creating class variables just to pass them on when constructing > the > > ExtensionSession might not be good style. If you think it's fine to do that, > > I'll make the change. > > Yes - it's the same mechanism we use in other places. E.g. ChromotingHost stores > video_capture_task_runner_ only to pass it to ClientSession instances. Done.
https://codereview.chromium.org/398873005/diff/80001/remoting/host/client_ses... File remoting/host/client_session.cc (right): https://codereview.chromium.org/398873005/diff/80001/remoting/host/client_ses... remoting/host/client_session.cc:484: scoped_ptr<webrtc::ScreenCapturer> ClientSession::RequestScreenCapturer() { After our discussion with Wez today, I don't think you will need this method, as we decided to add ResetVideoPipeline() instead.
Hey Wez, Sergey will be unavailable for the rest of the week. Would appreciate it if you could take a look. This CL has really just boiled down to the IO MessageLoop issue. I brought it up in a previous patchset, but unanswered comments don't carry over (I wonder why). Thanks! https://codereview.chromium.org/398873005/diff/80001/remoting/host/chromoting... File remoting/host/chromoting_host_context.cc (right): https://codereview.chromium.org/398873005/diff/80001/remoting/host/chromoting... remoting/host/chromoting_host_context.cc:38: "ChromotingCaptureThread", ui_task_runner_, base::MessageLoop::TYPE_IO); (Based on comment from previous patch, but modified). Discussed with wez@ offline but unresolved. To summarize, the network and capture threads are provided to PeerConnection as signalling and worker threads respectively. During its ICE establishment, PeerConnection at some point calls remoting::ChromiumSocketFactory::CreateUdpSocket() on the worker thread (capturer) and fails because of its non-IO MessageLoop. It looks like this method is called several times on the correct (network) thread (i.e., it being called on this thread might be a webrtc bug), but I could be mistaken. Wez suggested we give PeerConnection two new chromium-compatible, IO threads rather than attempt to map our network/capture to their signalling/worker. I attempted to do this, but have three concerns: 1. ChromotingHostContext manages all threads for the chromoting host process, so it seems incorrect to create threads outside of it, in a HostExtensionSession. I could create two additional threads in ChromotingHostContext and pass them down, but that sounds messy/overreaching for a HostExtension. 2. Once CastExtensionSession gets a hold of a ScreenCapturer, it needs to use the existing capture thread (I believe) to capture frames. In the current setup, this constraint is satisfied. If we're creating new threads, I'm not sure how that would work. 3. Interaction between the ExtensionSession and ClientSession for ExtensionMessages etc. needs to be on the network thread. Again, that is true of the current setup, but not sure how it would work in the alternative one. Any advice? https://codereview.chromium.org/398873005/diff/80001/remoting/host/client_ses... File remoting/host/client_session.cc (right): https://codereview.chromium.org/398873005/diff/80001/remoting/host/client_ses... remoting/host/client_session.cc:484: scoped_ptr<webrtc::ScreenCapturer> ClientSession::RequestScreenCapturer() { On 2014/07/29 06:59:18, Sergey Ulanov wrote: > After our discussion with Wez today, I don't think you will need this method, as > we decided to add ResetVideoPipeline() instead. Agreed.
Looks like this CL shouldn't be required, then? https://codereview.chromium.org/398873005/diff/80001/remoting/host/chromoting... File remoting/host/chromoting_host_context.cc (right): https://codereview.chromium.org/398873005/diff/80001/remoting/host/chromoting... remoting/host/chromoting_host_context.cc:38: "ChromotingCaptureThread", ui_task_runner_, base::MessageLoop::TYPE_IO); On 2014/07/29 17:06:26, aiguha wrote: > (Based on comment from previous patch, but modified). > > Discussed with wez@ offline but unresolved. > To summarize, the network and capture threads are provided to PeerConnection as > signalling and worker threads respectively. During its ICE establishment, > PeerConnection at some point calls > remoting::ChromiumSocketFactory::CreateUdpSocket() on the worker thread > (capturer) and fails because of its non-IO MessageLoop. It looks like this > method is called several times on the correct (network) thread (i.e., it being > called on this thread might be a webrtc bug), but I could be mistaken. It may be possible to check in CreateUdpSocket() whether we're on an IO thread and have it immediately fail. Better still, let's get a call-stack for that case so we can determine whether it's a WebRTC bug. :) > Wez suggested we give PeerConnection two new chromium-compatible, IO threads > rather than attempt to map our network/capture to their signalling/worker. I > attempted to do this, but have three concerns: > 1. ChromotingHostContext manages all threads for the chromoting host process, so > it seems incorrect to create threads outside of it, in a HostExtensionSession. I > could create two additional threads in ChromotingHostContext and pass them down, > but that sounds messy/overreaching for a HostExtension. It's OK to have the extension create its own threads, if it really needs to. > 2. Once CastExtensionSession gets a hold of a ScreenCapturer, it needs to use > the existing capture thread (I believe) to capture frames. In the current setup, > this constraint is satisfied. If we're creating new threads, I'm not sure how > that would work. When you "steal" the ScreenCapturer via the shiny new OnCreateScreenCapturer hook, you can use it on whichever thread you like so long as you also delete it on that thread. > 3. Interaction between the ExtensionSession and ClientSession for > ExtensionMessages etc. needs to be on the network thread. Again, that is true of > the current setup, but not sure how it would work in the alternative one. Should be identical. There's nothing magical about the ChromotingHostContext's set of threads; it's really just a convenience class. > > Any advice? https://codereview.chromium.org/398873005/diff/80001/remoting/host/client_ses... File remoting/host/client_session.h (right): https://codereview.chromium.org/398873005/diff/80001/remoting/host/client_ses... remoting/host/client_session.h:171: scoped_ptr<webrtc::ScreenCapturer> RequestScreenCapturer(); We don't need this with the New Shiny OnCreateVideoCapturer hook for extensions.
Yup, this CL is no longer needed. I'm now creating a worker thread inside the CastExtensionSession itself. Still a few kinks to work out, but I think it's clear that I need not modify the existing capture thread's MessageLoopType. I'll go ahead and close this CL. On Thu, Jul 31, 2014 at 6:14 PM, <wez@chromium.org> wrote: > Looks like this CL shouldn't be required, then? > > > > https://codereview.chromium.org/398873005/diff/80001/ > remoting/host/chromoting_host_context.cc > File remoting/host/chromoting_host_context.cc (right): > > https://codereview.chromium.org/398873005/diff/80001/ > remoting/host/chromoting_host_context.cc#newcode38 > remoting/host/chromoting_host_context.cc:38: "ChromotingCaptureThread", > ui_task_runner_, base::MessageLoop::TYPE_IO); > On 2014/07/29 17:06:26, aiguha wrote: > >> (Based on comment from previous patch, but modified). >> > > Discussed with wez@ offline but unresolved. >> To summarize, the network and capture threads are provided to >> > PeerConnection as > >> signalling and worker threads respectively. During its ICE >> > establishment, > >> PeerConnection at some point calls >> remoting::ChromiumSocketFactory::CreateUdpSocket() on the worker >> > thread > >> (capturer) and fails because of its non-IO MessageLoop. It looks like >> > this > >> method is called several times on the correct (network) thread (i.e., >> > it being > >> called on this thread might be a webrtc bug), but I could be mistaken. >> > > It may be possible to check in CreateUdpSocket() whether we're on an IO > thread and have it immediately fail. Better still, let's get a > call-stack for that case so we can determine whether it's a WebRTC bug. > :) > > > Wez suggested we give PeerConnection two new chromium-compatible, IO >> > threads > >> rather than attempt to map our network/capture to their >> > signalling/worker. I > >> attempted to do this, but have three concerns: >> 1. ChromotingHostContext manages all threads for the chromoting host >> > process, so > >> it seems incorrect to create threads outside of it, in a >> > HostExtensionSession. I > >> could create two additional threads in ChromotingHostContext and pass >> > them down, > >> but that sounds messy/overreaching for a HostExtension. >> > > It's OK to have the extension create its own threads, if it really needs > to. > > > 2. Once CastExtensionSession gets a hold of a ScreenCapturer, it needs >> > to use > >> the existing capture thread (I believe) to capture frames. In the >> > current setup, > >> this constraint is satisfied. If we're creating new threads, I'm not >> > sure how > >> that would work. >> > > When you "steal" the ScreenCapturer via the shiny new > OnCreateScreenCapturer hook, you can use it on whichever thread you like > so long as you also delete it on that thread. > > > 3. Interaction between the ExtensionSession and ClientSession for >> ExtensionMessages etc. needs to be on the network thread. Again, that >> > is true of > >> the current setup, but not sure how it would work in the alternative >> > one. > > Should be identical. There's nothing magical about the > ChromotingHostContext's set of threads; it's really just a convenience > class. > > > Any advice? >> > > https://codereview.chromium.org/398873005/diff/80001/ > remoting/host/client_session.h > File remoting/host/client_session.h (right): > > https://codereview.chromium.org/398873005/diff/80001/ > remoting/host/client_session.h#newcode171 > remoting/host/client_session.h:171: scoped_ptr<webrtc::ScreenCapturer> > RequestScreenCapturer(); > We don't need this with the New Shiny OnCreateVideoCapturer hook for > extensions. > > https://codereview.chromium.org/398873005/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
