PageLoadMetrics renderer and browser implementation (deprecated)

components/page_load_metrics/renderer/page_timing_metrics_sender.cc

Index: components/page_load_metrics/renderer/page_timing_metrics_sender.cc
diff --git a/components/page_load_metrics/renderer/page_timing_metrics_sender.cc b/components/page_load_metrics/renderer/page_timing_metrics_sender.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7cce9e8f8d16311252c5ba632bc97896960895da
--- /dev/null
+++ b/components/page_load_metrics/renderer/page_timing_metrics_sender.cc
@@ -0,0 +1,46 @@
+// Copyright 2015 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.
+
+#include "base/callback.h"
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+#include "components/page_load_metrics/common/page_load_metrics_messages.h"
+#include "components/page_load_metrics/renderer/page_timing_metrics_sender.h"
+#include "ipc/ipc_sender.h"
+
+namespace page_load_metrics {
+
+namespace {
+const int kTimerDelayMillis = 1000;
+}  // namespace
+
+PageTimingMetricsSender::PageTimingMetricsSender(
+    IPC::Sender* ipc_sender, int routing_id, scoped_ptr<base::Timer> timer)

[nasko, 2015/08/26 17:27:24]

Each parameter on a separate line.

+    : ipc_sender_(ipc_sender), routing_id_(routing_id), timer_(timer.Pass()) {}
+
+PageTimingMetricsSender::~PageTimingMetricsSender() {
+  if (timer_->IsRunning()) {
+    timer_->Stop();
+    SendNow();
+  }
+}
+
+void PageTimingMetricsSender::Send(const PageLoadTiming& timing) {
+  if (timing == last_timing_)
+    return;
+
+  last_timing_ = timing;
+
+  if (!timer_->IsRunning())

[Randy Smith (Not in Mondays), 2015/08/26 19:11:48]

It's not a lot of code complexity, so I don't object, but I do question whether
we really need to worry about coalescing IPCs.  This is three IPCs over the
course of a page load, none of which will contribute directly to user jank (i.e.
they aren't on any path the user waits on).  And if we do need coallescing, 1s
seems a random number.  So I'd vote against doing this without evidence that
it's needed.  Having said that, I think I should be outvoted by the people
requesting it, so consider this a quixotic comment :-}.

+    timer_->Start(FROM_HERE,
+        base::TimeDelta::FromMilliseconds(kTimerDelayMillis),
+        base::Bind(&PageTimingMetricsSender::SendNow, base::Unretained(this)));
+}
+
+void PageTimingMetricsSender::SendNow() {
+  ipc_sender_->Send(
+      new PageLoadMetricsMsg_OnTimingUpdated(routing_id_, last_timing_));
+}
+
+}  // namespace page_load_metrics