Commit 78c4c3e4 authored by Joe Downing's avatar Joe Downing Committed by Commit Bot

Fixing event logging for the Windows host

The Windows host is supposed to log events using the Windows
event logging API so they can be viewed in Event Viewer.  During a
recent bugfix validation, we discovered that this was broken as
no chromoting events were being logged.

I tracked down the root cause and it was from a little of 3 years ago:
https://chromium-review.googlesource.com/c/chromium/src/+/551035/

In that CL, the HostStatusMonitor functionality was pulled out of the
DaemonProcess and put into its own class.  The problem was that the
observer list was not removed, so instead of using |status_monitor_|
to notify listeners, the DaemonProcess continued to use
|status_observers_| which is always empty.

The fix is to remove the status_observers_ member and notify listeners
using the observer list owned by status_monitor_.

Bug: 1127439
Change-Id: I9ee00212e0e2c3646ced5711e594a60dfd2627e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2406698Reviewed-by: default avatarLambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@google.com>
Cr-Commit-Position: refs/heads/master@{#806342}
parent af57f317
...@@ -290,28 +290,28 @@ bool DaemonProcess::WasTerminalIdAllocated(int terminal_id) { ...@@ -290,28 +290,28 @@ bool DaemonProcess::WasTerminalIdAllocated(int terminal_id) {
void DaemonProcess::OnAccessDenied(const std::string& jid) { void DaemonProcess::OnAccessDenied(const std::string& jid) {
DCHECK(caller_task_runner()->BelongsToCurrentThread()); DCHECK(caller_task_runner()->BelongsToCurrentThread());
for (auto& observer : status_observers_) for (auto& observer : status_monitor_->observers())
observer.OnAccessDenied(jid); observer.OnAccessDenied(jid);
} }
void DaemonProcess::OnClientAuthenticated(const std::string& jid) { void DaemonProcess::OnClientAuthenticated(const std::string& jid) {
DCHECK(caller_task_runner()->BelongsToCurrentThread()); DCHECK(caller_task_runner()->BelongsToCurrentThread());
for (auto& observer : status_observers_) for (auto& observer : status_monitor_->observers())
observer.OnClientAuthenticated(jid); observer.OnClientAuthenticated(jid);
} }
void DaemonProcess::OnClientConnected(const std::string& jid) { void DaemonProcess::OnClientConnected(const std::string& jid) {
DCHECK(caller_task_runner()->BelongsToCurrentThread()); DCHECK(caller_task_runner()->BelongsToCurrentThread());
for (auto& observer : status_observers_) for (auto& observer : status_monitor_->observers())
observer.OnClientConnected(jid); observer.OnClientConnected(jid);
} }
void DaemonProcess::OnClientDisconnected(const std::string& jid) { void DaemonProcess::OnClientDisconnected(const std::string& jid) {
DCHECK(caller_task_runner()->BelongsToCurrentThread()); DCHECK(caller_task_runner()->BelongsToCurrentThread());
for (auto& observer : status_observers_) for (auto& observer : status_monitor_->observers())
observer.OnClientDisconnected(jid); observer.OnClientDisconnected(jid);
} }
...@@ -331,21 +331,21 @@ void DaemonProcess::OnClientRouteChange(const std::string& jid, ...@@ -331,21 +331,21 @@ void DaemonProcess::OnClientRouteChange(const std::string& jid,
CHECK(local_ip.empty() || local_ip.IsValid()); CHECK(local_ip.empty() || local_ip.IsValid());
parsed_route.local_address = net::IPEndPoint(local_ip, route.local_port); parsed_route.local_address = net::IPEndPoint(local_ip, route.local_port);
for (auto& observer : status_observers_) for (auto& observer : status_monitor_->observers())
observer.OnClientRouteChange(jid, channel_name, parsed_route); observer.OnClientRouteChange(jid, channel_name, parsed_route);
} }
void DaemonProcess::OnHostStarted(const std::string& xmpp_login) { void DaemonProcess::OnHostStarted(const std::string& xmpp_login) {
DCHECK(caller_task_runner()->BelongsToCurrentThread()); DCHECK(caller_task_runner()->BelongsToCurrentThread());
for (auto& observer : status_observers_) for (auto& observer : status_monitor_->observers())
observer.OnStart(xmpp_login); observer.OnStart(xmpp_login);
} }
void DaemonProcess::OnHostShutdown() { void DaemonProcess::OnHostShutdown() {
DCHECK(caller_task_runner()->BelongsToCurrentThread()); DCHECK(caller_task_runner()->BelongsToCurrentThread());
for (auto& observer : status_observers_) for (auto& observer : status_monitor_->observers())
observer.OnShutdown(); observer.OnShutdown();
} }
......
...@@ -14,7 +14,6 @@ ...@@ -14,7 +14,6 @@
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/observer_list.h"
#include "base/process/process.h" #include "base/process/process.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "ipc/ipc_channel.h" #include "ipc/ipc_channel.h"
...@@ -36,7 +35,6 @@ namespace remoting { ...@@ -36,7 +35,6 @@ namespace remoting {
class AutoThreadTaskRunner; class AutoThreadTaskRunner;
class DesktopSession; class DesktopSession;
class HostEventLogger; class HostEventLogger;
class HostStatusObserver;
class ProcessStatsSender; class ProcessStatsSender;
class ScreenResolution; class ScreenResolution;
...@@ -190,9 +188,6 @@ class DaemonProcess ...@@ -190,9 +188,6 @@ class DaemonProcess
// The highest desktop session ID that has been seen so far. // The highest desktop session ID that has been seen so far.
int next_terminal_id_; int next_terminal_id_;
// Keeps track of observers receiving host status notifications.
base::ObserverList<HostStatusObserver>::Unchecked status_observers_;
// Invoked to ask the owner to delete |this|. // Invoked to ask the owner to delete |this|.
base::OnceClosure stopped_callback_; base::OnceClosure stopped_callback_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment