Chromoting: Implemented security attention sequence (SAS) emulation on Windows.

remoting/host/session_event_executor_win.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.
+
+#ifndef REMOTING_HOST_SESSION_EVENT_EXECUTOR_WIN_H_
+#define REMOTING_HOST_SESSION_EVENT_EXECUTOR_WIN_H_
+
+#include "remoting/protocol/input_stub.h"
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "ipc/ipc_channel.h"
+
+#include "remoting/host/event_executor.h"
+
+class MessageLoop;
+
+namespace base {
+
+class MessageLoopProxy;

[Wez, 2012/03/08 22:58:55]

nit: Does the style guide require that there be blank lines around these
forward-decls? If not then I'd suggest removing them to aid readability.

[alexeypa (please no reviews), 2012/03/09 01:13:54]

Done.

+
+} // namespace base
+
+namespace IPC {
+
+class ChannelProxy;
+
+} // namespace IPC
+
+namespace remoting {
+
+class SessionEventExecutor : public EventExecutor,

[Wez, 2012/03/08 22:58:55]

nit: SessionEventExecutorWin, to match the guard & filename?

[Wez, 2012/03/08 22:58:55]

public EventExecutor -> public InputStub

[Wez, 2012/03/08 22:58:55]

I'm not convinced by the name; what is "session" about this class?  It's really
more a "daemon" event-injector, surely?

[alexeypa (please no reviews), 2012/03/09 01:13:54]

Done.

[alexeypa (please no reviews), 2012/03/09 01:13:54]

Done.

[alexeypa (please no reviews), 2012/03/09 01:13:54]

This class's responsibility is handling of session-specific events (and other
session-specific stuff such as switching between desktops).

The fact the "daemon" is involved is only an implementation detail.

[Wez, 2012/03/09 22:24:34]

Except that this implementation only makes sense in the context of the daemon. 
That said, if UIAccess grants us rights to switch into the UAC desktop and/or
call SendSAS, then it that makes more sense.  The problem with "session" is that
it's used elsewhere in the remoting/ code - "WinSession" might be clearer?

+                             public IPC::Channel::Listener {
+ public:
+  SessionEventExecutor(MessageLoop* message_loop,

[Wez, 2012/03/08 22:58:55]

It's not clear what this class does with |message_loop|, or what happens to
events that have been injected, posted to |message_loop|, but not yet processed
by the SessionEventExecutor, if it gets deleted?

[alexeypa (please no reviews), 2012/03/09 01:13:54]

It does the same thing as other EventExecutor classes.

[Wez, 2012/03/09 22:24:34]

Yes, I know that.  My point is that it's not at all obvious what that is,
without a comment on the constructor to explain, or even to refer the reader to
EventExecutor::Create() (although that doesn't document the behaviour either).

+                       base::MessageLoopProxy* io_message_loop,
+                       scoped_ptr<EventExecutor> nested_executor);

[Wez, 2012/03/08 22:58:55]

scoped_ptr<EventExecutor> -> scoped_ptr<InputStub>

[alexeypa (please no reviews), 2012/03/09 01:13:54]

Done.

+  ~SessionEventExecutor();
+
+  // protocol::InputStub implementation.
+  virtual void InjectKeyEvent(const protocol::KeyEvent& event) OVERRIDE;
+  virtual void InjectMouseEvent(const protocol::MouseEvent& event) OVERRIDE;
+
+  // IPC::Channel::Listener implementation.
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+
+ private:
+  // Pointer to the next event executor.
+  scoped_ptr<EventExecutor> nested_executor_;
+
+  MessageLoop* message_loop_;
+
+  // The Chromoting IPC channel connecting the host with the service.
+  scoped_ptr<IPC::ChannelProxy> chromoting_channel_;
+
+  bool scroll_pressed_;
+
+  DISALLOW_COPY_AND_ASSIGN(SessionEventExecutor);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_SESSION_EVENT_EXECUTOR_WIN_H_