Fixing an AttachmentBroker problem which broke the windows host

remoting/host/ipc_util_win.cc

Index: remoting/host/ipc_util_win.cc
diff --git a/remoting/host/ipc_util_win.cc b/remoting/host/ipc_util_win.cc
index 5da5f8264fe1fe7ecf00894b889a766e376a1f59..2998633ecfaffd9a89c609eb1f25bf818f3ffc42 100644
--- a/remoting/host/ipc_util_win.cc
+++ b/remoting/host/ipc_util_win.cc
@@ -59,10 +59,14 @@ bool CreateConnectedIpcChannel(
   // Wrap the pipe into an IPC channel.
   std::unique_ptr<IPC::ChannelProxy> server(
       new IPC::ChannelProxy(listener, io_task_runner));
-  IPC::AttachmentBroker::GetGlobal()->RegisterCommunicationChannel(
-      server.get(), io_task_runner);
+  IPC::AttachmentBroker* broker = IPC::AttachmentBroker::GetGlobal();
+  if (!broker) {
+    LOG(WARNING) << "No AttachmentBroker registered.";

[erikchen, 2016/04/28 23:12:50]

Every process should have a broker. When does this LOG statement get emitted?

[joedow, 2016/04/28 23:20:49]

It occurred after you made your last change and I had to spend more time than I
would have liked tracking it down.  With my change I suppose now the message
will fail (which should be logged) instead of crashing.  I like having the
warning because in our case, we were spinning up a process on a TS session and
there was no debug output as to why the connection was failing.  This is a big
clue which should save them time.

I'd like to leave it as this would have saved me time tracking this issue down.

[erikchen, 2016/04/28 23:24:54]

Would making this a DCHECK(broker) or even a CHECK(broker) satisfy your
constraint for making future problems easy to debug? The presence of a
conditional/log statement implies that this is a valid state for the system
[which it isn't].

[joedow, 2016/04/28 23:39:26]

DCHECK sounds legit.

+  } else if (broker->IsPrivilegedBroker()) {
+    broker->RegisterCommunicationChannel(server.get(), io_task_runner);
+  }
   server->Init(IPC::ChannelHandle(pipe.Get()), IPC::Channel::MODE_SERVER,
-                true);
+               /*create_pipe_now=*/true);
 
   // Convert the channel name to the pipe name.
   std::string pipe_name(kChromePipeNamePrefix);