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

remoting/host/chromoting_messages.h

 // Copyright (c) 2012 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.
 
-// 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
 
 //-----------------------------------------------------------------------------
 // Chromoting messages sent from the daemon to the network process.
 
 // Requests the network process to crash producing a crash dump. The daemon
 // sends this message when a fatal error has been detected indicating that
 // the network process misbehaves. The daemon passes the location of the code
 // that detected the error.
@@ skipping 56 matching lines @@
 // Chromoting messages sent from the desktop to the daemon process.
 
 // Notifies the daemon that a desktop integration process has been initialized.
 // |desktop_pipe| specifies the client end of the desktop pipe. It is to be
 // forwarded to the desktop environment stub.
 //
 // Windows only: |desktop_pipe| has to be duplicated from the desktop process by
 // 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.