[Chromoting] Allow ProcessStatsSender to receive nullptr ProcessStatsAgent

remoting/host/process_stats_sender.cc

Index: remoting/host/process_stats_sender.cc
diff --git a/remoting/host/process_stats_sender.cc b/remoting/host/process_stats_sender.cc
index a734877e51887d5c92d33ab8f1686f94ad2db2dd..2bfd4c2186f1045f4536585097d9d7af0b943cec 100644
--- a/remoting/host/process_stats_sender.cc
+++ b/remoting/host/process_stats_sender.cc
@@ -13,16 +13,31 @@
 
 namespace remoting {
 
+namespace {
+
+std::vector<ProcessStatsAgent*> RemoveNullAgent(
+    std::initializer_list<ProcessStatsAgent*> agents) {
+  std::vector<ProcessStatsAgent*> result;
+  for (auto* const agent : agents) {
+    if (agent) {
+      result.push_back(agent);
+    }
+  }
+  return result;
+}
+
+}  // namespace
+
 ProcessStatsSender::ProcessStatsSender(
     protocol::ProcessStatsStub* host_stats_stub,
     base::TimeDelta interval,
     std::initializer_list<ProcessStatsAgent*> agents)
     : host_stats_stub_(host_stats_stub),
-      agents_(agents),
+      agents_(RemoveNullAgent(agents)),

[joedow, 2017/05/03 21:41:01]

I'm not sure about the approach here.  It seems much simpler just to pass the
data over the event channel.  I think that would simplify your code quite a bit
and remove the need for code like this (which is pretty easy to break if a bug
ever occurred).

I can't imagine there will be enough data that sending it over the event channel
would be a problem.

[Hzj_jie, 2017/05/03 22:18:04]

:)

       thread_checker_() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(host_stats_stub_);
-  DCHECK(!interval.is_zero());
+  DCHECK(interval > base::TimeDelta());

[joedow, 2017/05/03 21:41:01]

The !is_zero() check seems more readable, why the change?

[Hzj_jie, 2017/05/03 22:18:04]

Timer handles negative interval as 0:
https://cs.chromium.org/chromium/src/base/timer/timer.cc?rcl=e6baa90f9f21576e...

[joedow, 2017/05/08 16:25:41]

I haven't experienced negative TimeDeltas being a problem but it seems
reasonable (though less readable) to check for it.

Perhaps a cleaner approach would be to add a helper method to TimeDelta which is
'is_positive()'.

[Hzj_jie, 2017/05/08 18:16:19]

Yes, and I also think silently accepting negative delay is not a right thing in
Timer. But unfortunately it looks like a very destructive change to disallow
negative delay in Timer.

   DCHECK(!agents_.empty());
 
   timer_.Start(FROM_HERE, interval, this, &ProcessStatsSender::ReportUsage);