remoting/host/window_capturer_screen_wrapper.h - Issue 422503004: Adding ability to stream windows and inject events to them

Side by Side Diff: remoting/host/window_capturer_screen_wrapper.h

Issue 422503004: Adding ability to stream windows and inject events to them (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: I updated the code with almost all of the comments Lambros had. The exceptions are the redesign of … Created 6 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« remoting/host/remoting_me2me_host.cc ('K') | « remoting/host/remoting_me2me_host.cc ('k') | remoting/host/window_capturer_screen_wrapper.cc » ('j') | remoting/host/window_capturer_screen_wrapper.cc » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 // Copyright (c) 2014 The Chromium Authors. All rights reserved.
	Wez 2014/08/01 23:41:55 Lose the (c), please. Lose the (c), please. ronakvora do not use 2014/08/05 19:54:51 Done. Show quoted text On 2014/08/01 23:41:55, Wez wrote: > Lose the (c), please. Done.
	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #include "third_party/webrtc/modules/desktop_capture/screen_capturer.h"

	6 #include "third_party/webrtc/modules/desktop_capture/window_capturer.h"
	Wez 2014/08/01 23:41:55 You don't need this include, I don't think; just f You don't need this include, I don't think; just forward-declare webrtc::WindowCapturer. ronakvora do not use 2014/08/05 19:54:51 Done. Added the include to the cc file. Show quoted text On 2014/08/01 23:41:55, Wez wrote: > You don't need this include, I don't think; just forward-declare > webrtc::WindowCapturer. Done. Added the include to the cc file.
	7

	8 namespace remoting {

	9

	10 // This class wraps a WindowCapturer instance. The reason we do this

	11 // is becaues the remoting_me2me_host uses a screen capturer interface.

	12 // Instead of adding support for a window_capturer_interface, which

	13 // is not really needed, we simply wrap a WindowCapturer inside of a

	14 // ScreenCapturer and delegate messages sent to this class to the

	15 // wrapped WindowCapturer.
	Wez 2014/08/01 23:41:55 Do you actually need to proxy these calls through? Do you actually need to proxy these calls through? Couldn't you just have the caller create the underlying webrtc::WindowCapturer and construct this class to wrap it, thereby avoiding this class needing any WindowCapturer-like methods? Wez 2014/08/01 23:41:55 You shouldn't need this class at all, in fact; the You shouldn't need this class at all, in fact; the only reason the Chromoting host requires a ScreenCapturer is so that it can register for mouse-shape change notifications - that's something we're in the process of cleaning up, though, at which point you'll be able to return a DesktopCapturer from DesktopEnvironment rather than a ScreenCapturer, so you can then return the WindowCapturer directly. ronakvora do not use 2014/08/05 19:54:51 Yep. The original plan was to change the interface Show quoted text On 2014/08/01 23:41:55, Wez wrote: > You shouldn't need this class at all, in fact; the only reason the Chromoting > host requires a ScreenCapturer is so that it can register for mouse-shape change > notifications - that's something we're in the process of cleaning up, though, at > which point you'll be able to return a DesktopCapturer from DesktopEnvironment > rather than a ScreenCapturer, so you can then return the WindowCapturer > directly. Yep. The original plan was to change the interface to a DesktopCapturer, rather than ScreenCapturer, but then I realized that the SetMouseShapeObserver method was needed for ScreenCapturer, and so Sergey and I discussed this solution. ronakvora do not use 2014/08/05 19:54:51 Done. Show quoted text On 2014/08/01 23:41:55, Wez wrote: > Do you actually need to proxy these calls through? > > Couldn't you just have the caller create the underlying webrtc::WindowCapturer > and construct this class to wrap it, thereby avoiding this class needing any > WindowCapturer-like methods? Done.
	16 class WindowCapturerScreenWrapper : public ::webrtc::ScreenCapturer {
	Wez 2014/08/01 23:41:55 Lose the :: before webrtc. Lose the :: before webrtc. ronakvora do not use 2014/08/05 19:54:51 Done. Show quoted text On 2014/08/01 23:41:55, Wez wrote: > Lose the :: before webrtc. Done.
	17 public:

	18 WindowCapturerScreenWrapper();

	19 virtual ~WindowCapturerScreenWrapper();

	20

	21 // WindowCapturer interface.
	Wez 2014/08/01 23:41:55 This class doesn't provide the WindowCapturer, int This class doesn't provide the WindowCapturer, interface, though? ronakvora do not use 2014/08/05 19:54:51 Removed. I used to have all the methods of WindowC Show quoted text On 2014/08/01 23:41:55, Wez wrote: > This class doesn't provide the WindowCapturer, interface, though? Removed. I used to have all the methods of WindowCapturer here. When I realized I didn't need them, I removed them. I removed the comment.
	22 bool SelectWindow(webrtc::WindowId id);

	23 void SetWindow(const webrtc::DesktopCaptureOptions& options);
	Wez 2014/08/01 23:41:55 What's the differencing between "selecting" and "s What's the differencing between "selecting" and "setting" the window? Why does SetWindow actually take a set of DesktopCaptureOptions? ronakvora do not use 2014/08/05 19:54:51 The SetWindow just creates the wrapped WindowCaptu Show quoted text On 2014/08/01 23:41:55, Wez wrote: > What's the differencing between "selecting" and "setting" the window? > Why does SetWindow actually take a set of DesktopCaptureOptions? The SetWindow just creates the wrapped WindowCapturer instance. I updated this method to be called CreateWindowCapturerInstance.
	24

	25 // DesktopCapturer interface.
	Wez 2014/08/01 23:41:55 nit: webrtc::DesktopCapturer for consistency, or j nit: webrtc::DesktopCapturer for consistency, or just fold this under the webrtc::ScreenCapturer interface comment. ronakvora do not use 2014/08/05 19:54:51 Done. Show quoted text On 2014/08/01 23:41:55, Wez wrote: > nit: webrtc::DesktopCapturer for consistency, or just fold this under the > webrtc::ScreenCapturer interface comment. Done.
	26 virtual void Start(DesktopCapturer::Callback* callback) OVERRIDE;

	27 virtual void Capture(const webrtc::DesktopRegion& region) OVERRIDE;

	28

	29 // webrtc::ScreenCapturer interface.

	30 virtual void SetMouseShapeObserver(

	31 MouseShapeObserver* mouse_shape_observer) OVERRIDE;

	32 virtual bool GetScreenList(ScreenList* screens) OVERRIDE;

	33 virtual bool SelectScreen(webrtc::ScreenId id) OVERRIDE;

	34

	35 private:

	36 webrtc::WindowCapturer* window_capturer_;
	Wez 2014/08/01 23:41:55 scoped_ptr<WindowCapturer> scoped_ptr<WindowCapturer> ronakvora do not use 2014/08/05 19:54:51 Done. Show quoted text On 2014/08/01 23:41:55, Wez wrote: > scoped_ptr<WindowCapturer> Done.
	37

	38 DISALLOW_COPY_AND_ASSIGN(WindowCapturerScreenWrapper);

	39 };

	40

	41

	42 } // namespace remoting

OLD	NEW