Making the me2me host compiling and running on Windows. This includes making it a window applicatio…

remoting/host/host_event_logger.h

Index: remoting/host/host_event_logger.h
diff --git a/remoting/host/host_event_logger.h b/remoting/host/host_event_logger.h
index 3c92bd638f07609726ece26f48a62fb5abe462aa..b039b3b5e50d9ffe634e3cb1a62dc4043c68ffea 100644
--- a/remoting/host/host_event_logger.h
+++ b/remoting/host/host_event_logger.h
@@ -9,38 +9,25 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
-#include "remoting/host/host_status_observer.h"
 
 namespace remoting {
 
 class ChromotingHost;
-class SystemEventLogger;
 
-class HostEventLogger : public HostStatusObserver {
+class HostEventLogger {

[Lambros, 2012/03/01 21:15:41]

It's unfortunate that it has to be refactored this way to accommodate the
Windows ReportEvent() API.  I originally had this as a cross-platform class
which passed the strings to a platform-specific SystemEventLogger class.  I
think it would be nicer if the SystemEventLogger API could be widened to accept
a list of strings (log-type, log-severity, list of messages, whatever), and then
hide the ugly Windows-specific details behind that API.

I guess this will have to do for now.  The downside is that it becomes painful
to add new event types to the HostStatusObserver for logging - you now have to
do this separately for all platforms (Win, Linux, Mac), including hand-editing
of the .mc file to match whatever is logged on the other platforms.  I would
prefer if that could be avoided.

[alexeypa (please no reviews), 2012/03/01 22:32:41]

Yes, I saw SystemEventLogger. Unfortunately it does not work nicely with the the
recommended way events should be reported to Windows Event Log. Basically each
message has to come from the message file and the format string should be
defined in the messages file. It is of course possible to define a single event
"%1" - a placeholder for a string and report each and every message using this
event. The cons of this approach are:

1. It is against Windows guidelines for apps.
2. It doesn't allow Windows style localization of events reported to the event
log. Strange but Windows whats that.
3. It inhibits filtering capabilities of Event Log Viewer which are not that
advanced. 

Let's discuss it face to face because I'm not sure what is the best way myself.

  public:
-  HostEventLogger(ChromotingHost* host, const std::string& application_name);
-  virtual ~HostEventLogger();
-
-  // HostStatusObserver implementation.  These methods will be called from the
-  // network thread.
-  virtual void OnClientAuthenticated(const std::string& jid) OVERRIDE;
-  virtual void OnClientDisconnected(const std::string& jid) OVERRIDE;
-  virtual void OnAccessDenied(const std::string& jid) OVERRIDE;
-  virtual void OnClientRouteChange(
-      const std::string& jid,
-      const std::string& channel_name,
-      const net::IPEndPoint& remote_end_point,
-      const net::IPEndPoint& local_end_point) OVERRIDE;
-  virtual void OnShutdown() OVERRIDE;
+  virtual ~HostEventLogger() {}

[Jamie, 2012/03/01 18:22:32]

Be sure to run this by the clang bots. It has some rules about in-line ctor and
dtor, though I can't remember exactly what they are.

[alexeypa (please no reviews), 2012/03/01 20:33:58]

Will do.

[Lambros, 2012/03/01 21:15:41]

Should be OK (and preferred!) for a pure interface class.  +1 to running it
through clang :)

 
- private:
-  void Log(const std::string& message);
+  // Creates an event-logger that monitors host status changes and logs
+  // corresponding events to the OS-specific log (syslog/EventLog).
+  static scoped_ptr<HostEventLogger> Create(
+      ChromotingHost* host, const std::string& application_name);
 
-  scoped_refptr<ChromotingHost> host_;
-  scoped_ptr<SystemEventLogger> system_event_logger_;
+ protected:
+  HostEventLogger() {}

[Lambros, 2012/03/01 21:15:41]

nit: Maybe get rid of this ctor, and just make the dtor protected?

[alexeypa (please no reviews), 2012/03/01 22:32:41]

It is required by the compiler and it is protected (as you have noticed already.
:-)).

 
+ private:
   DISALLOW_COPY_AND_ASSIGN(HostEventLogger);
 };