Remote Assistance on Chrome OS Part I - Aura Desktop Capturer

remoting/host/chromeos/aura_desktop_capturer.h

Index: remoting/host/chromeos/aura_desktop_capturer.h
diff --git a/remoting/host/chromeos/aura_desktop_capturer.h b/remoting/host/chromeos/aura_desktop_capturer.h
new file mode 100644
index 0000000000000000000000000000000000000000..15b272cfcea43ba28605570894e993b683453923
--- /dev/null
+++ b/remoting/host/chromeos/aura_desktop_capturer.h
@@ -0,0 +1,56 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef REMOTING_HOST_CHROMEOS_AURA_DESKTOP_CAPTURER_H_
+#define REMOTING_HOST_CHROMEOS_AURA_DESKTOP_CAPTURER_H_
+
+#include "third_party/webrtc/modules/desktop_capture/screen_capturer.h"
+#include "ui/aura/window.h"
+
+namespace cc {
+class CopyOutputResult;
+}  // namespace
+
+namespace remoting {
+
+// A webrtc::DesktopCapturer that captures pixels from the root window of the
+// Aura Shell.
+class AuraDesktopCapturer : public webrtc::DesktopCapturer {
+ public:
+  AuraDesktopCapturer();
+  virtual ~AuraDesktopCapturer();
+
+  // webrtc::DesktopCapturer implementation.
+  virtual void Start(webrtc::DesktopCapturer::Callback* callback) OVERRIDE;
+
+  // Captures the next frame. In Aura, this is implemented by requesting the
+  // layer and its substree to be rendered to a given data structure. |region|
+  // specifies region of the capture target that should be fresh in the
+  // resulting frame. Since Aura doesn't support partial capturing, the entire
+  // region of the captured frame will always be fresh. Pending capture
+  // operations are cancelled when DesktopCapturer is deleted.

[Wez, 2014/09/10 01:41:11]

I'd suggest removing this comment and moving any relevant details either to the
class-level comment, or into the implementation itself.

In particular the cancellation on deletion is part of the interface contract,
and the Capture(region) API is only required to guarantee that |region| is
valid; it's not a problem if _more_ than region is valid.

[kelvinp, 2014/09/10 22:33:58]

Done.

+  virtual void Capture(const webrtc::DesktopRegion& region) OVERRIDE;
+
+  // Sets the window to be excluded from the captured image in the future
+  // Capture calls. Not supported in Aura.

[Wez, 2014/09/10 01:41:11]

Similarly, remove this comment; you can document it with NOTIMPLEMENTED() in the
implementation. :)

[kelvinp, 2014/09/10 22:33:59]

Done.

+  virtual void SetExcludedWindow(webrtc::WindowId window) OVERRIDE {}
+
+ private:
+  friend class AuraDesktopCapturerTest;
+
+  // Called when a copy of the layer is captured.
+  void OnFrameCaptured(scoped_ptr<cc::CopyOutputResult> result);
+
+  // Points to the callback passed to webrtc::DesktopCapturer::Start().
+  webrtc::DesktopCapturer::Callback* callback_;
+
+  // The root window of the Aura Shell.
+  aura::Window* desktop_window_;

[Wez, 2014/09/10 01:41:10]

Where does this ever get set?

[kelvinp, 2014/09/10 22:33:59]

Good catch. It should be set in Start, which got accidentally removed in my last
iteration.

+
+  base::WeakPtrFactory<AuraDesktopCapturer> weak_factory_;
+};

[Wez, 2014/09/10 01:41:10]

DISALLOW_COPY_AND_ASSIGN

[kelvinp, 2014/09/10 22:33:58]

Done.

+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_CHROMEOS_AURA_DESKTOP_CAPTURER_H_