DesktopSessionAgent now hosts the video capturer and provides necessary plumbing to drive it via an…

remoting/host/chromoting_messages.h

Index: remoting/host/chromoting_messages.h
diff --git a/remoting/host/chromoting_messages.h b/remoting/host/chromoting_messages.h
index 5cfa146ed813189e83bc2faf202ac95f64d0c547..1b984cba2abe7cc49c5067d52cdd81c94068ec52 100644
--- a/remoting/host/chromoting_messages.h
+++ b/remoting/host/chromoting_messages.h
@@ -2,11 +2,18 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Defines IPC messages used by Chromoting components.
+#ifndef REMOTING_HOST_CHROMOTING_MESSAGES_H_
+#define REMOTING_HOST_CHROMOTING_MESSAGES_H_

[Wez, 2012/11/16 23:37:31]

nit: Chromoting IPC messages, for clarity?

[alexeypa (please no reviews), 2012/11/19 21:46:25]

No the file name is chromoting_messages.h. Other IPC message files are also
called xxx_messages.h, so I'll follow the pattern.

+
+#include "ipc/ipc_platform_file.h"
+#include "third_party/skia/include/core/SkPoint.h"
+#include "third_party/skia/include/core/SkRect.h"
+#include "third_party/skia/include/core/SkSize.h"
+
+#endif  // REMOTING_HOST_CHROMOTING_MESSAGES_H_
 
 // Multiply-included message file, no traditional include guard.
 #include "ipc/ipc_message_macros.h"
-#include "ipc/ipc_platform_file.h"
 
 #define IPC_MESSAGE_START ChromotingMsgStart
 
@@ -83,3 +90,78 @@ IPC_MESSAGE_CONTROL3(ChromotingDaemonDesktopMsg_Crash,
 // the receiver of the message.
 IPC_MESSAGE_CONTROL1(ChromotingDesktopDaemonMsg_DesktopAttached,
                      IPC::PlatformFileForTransit /* desktop_pipe */)
+
+//-----------------------------------------------------------------------------
+// Chromoting messages sent from the desktop to the network process.
+
+// Notifies the network process that a shared buffer has been created.
+// The network process has to confirm registration by replying with
+// ChromotingNetworkDesktopMsg_SharedBufferRegistered message.
+IPC_MESSAGE_CONTROL3(ChromotingDesktopNetworkMsg_RegisterSharedBuffer,

[Wez, 2012/11/16 23:37:31]

nit: Make this & next fn Add/RemoveSharedBuffer?

[alexeypa (please no reviews), 2012/11/19 21:46:25]

Done.

+                     intptr_t /* id */,
+                     IPC::PlatformFileForTransit /* handle */,
+                     uint32 /* size */)
+
+// Request the network process to stop using a shared buffer.
+IPC_MESSAGE_CONTROL1(ChromotingDesktopNetworkMsg_DropSharedBuffer,
+                     intptr_t /* id */)
+
+IPC_STRUCT_TRAITS_BEGIN(SkIPoint)
+  IPC_STRUCT_TRAITS_MEMBER(fX)
+  IPC_STRUCT_TRAITS_MEMBER(fY)
+IPC_STRUCT_TRAITS_END()
+
+IPC_STRUCT_TRAITS_BEGIN(SkIRect)
+  IPC_STRUCT_TRAITS_MEMBER(fLeft)
+  IPC_STRUCT_TRAITS_MEMBER(fTop)
+  IPC_STRUCT_TRAITS_MEMBER(fRight)
+  IPC_STRUCT_TRAITS_MEMBER(fBottom)
+IPC_STRUCT_TRAITS_END()
+
+IPC_STRUCT_TRAITS_BEGIN(SkISize)
+  IPC_STRUCT_TRAITS_MEMBER(fWidth)
+  IPC_STRUCT_TRAITS_MEMBER(fHeight)
+IPC_STRUCT_TRAITS_END()
+
+// Serialized CaptureData structure.
+IPC_STRUCT_BEGIN(SerializedCapturedData)
+  // ID of the shared memory buffer containing the pixels.
+  IPC_STRUCT_MEMBER(intptr_t, shared_buffer_id)
+
+  // Captured region.
+  IPC_STRUCT_MEMBER(std::vector<SkIRect>, dirty_region)
+
+  // Dimentions of the buffer in pixels.

[Wez, 2012/11/16 23:37:31]

typo: dimensions

[alexeypa (please no reviews), 2012/11/19 21:46:25]

Done.

+  IPC_STRUCT_MEMBER(SkISize, dimensions)
+
+  // Pixel format.

[Wez, 2012/11/16 23:37:31]

nit: "Format of the shared pixel data buffer."?

[alexeypa (please no reviews), 2012/11/19 21:46:25]

Why? The shared buffer does not know it carries pixels. Pixel format is local to
this message. It is clear what pixels it is talking about since the whole
message is about describing an image buffer.

+  IPC_STRUCT_MEMBER(int32, pixel_format)
+
+  // Time spent in capture. Unit is in milliseconds.
+  IPC_STRUCT_MEMBER(int, capture_time_ms)

[Wez, 2012/11/16 23:37:31]

nit: I'd suggest moving this to the end of the struct and noting in the comment
that it's for debug purposes.

[alexeypa (please no reviews), 2012/11/19 21:46:25]

Right now the order of fields in this structure matches the order in
CaptureData. 

The value carried by |capture_time_ms| is used when scheduling next frame
capture, not only for debugging. There is a follow up CL in my queue that fixes
calculation of |capture_time_ms| value and takes advantage of this field.

+
+  // Sequence number supplied by client for performance tracking.
+  IPC_STRUCT_MEMBER(int64, client_sequence_number)
+
+  // DPI for this frame.
+  IPC_STRUCT_MEMBER(SkIPoint, dpi)
+IPC_STRUCT_END()
+
+// Notifies the network process that a shared buffer has been created.
+IPC_MESSAGE_CONTROL1(ChromotingDesktopNetworkMsg_CaptureCompleted,
+                     SerializedCapturedData /* capture_data */ )
+
+
+//-----------------------------------------------------------------------------
+// Chromoting messages sent from the network to the desktop process.
+
+// Notifies the desktop process that the shared memory buffer has been mapped to
+// the memory of the network process and so it can be safely dropped by
+// the network process at any time.

[Wez, 2012/11/16 23:37:31]

Do you mean it can be safely dropped by the desktop process at any time?  What
does it mean for the process to "drop" it; you mean to invalidate the
previously-shared handle?  What if the network process can't map the buffer for
some reason?

[alexeypa (please no reviews), 2012/11/19 21:46:25]

It means the process can safely unmap the memory and close the handle. The
desktop process should have created a local mapping on the buffer and local copy
of the handle by this time.
It is considered a failure. Right now captures involving this buffer will return
NULL CaptureData pointer. Yes, it needs to be handled better.

+IPC_MESSAGE_CONTROL1(ChromotingNetworkDesktopMsg_SharedBufferRegistered,
+                     intptr_t /* id */)
+
+IPC_MESSAGE_CONTROL1(ChromotingNetworkDesktopMsg_InvalidateRegion,
+                     std::vector<SkIRect> /* invalid_region */ )
+
+IPC_MESSAGE_CONTROL0(ChromotingNetworkDesktopMsg_CaptureFrame)

[Wez, 2012/11/16 23:37:31]

nit: Add comments to explain this & InvalidateRegion.

[alexeypa (please no reviews), 2012/11/19 21:46:25]

Done.