From 98086b7a870fddabbec86f37284b424cc5551ffb Mon Sep 17 00:00:00 2001
From: Chris H-C <chutten@mozilla.com>
Date: Tue, 12 Jul 2016 11:49:38 -0400
Subject: [PATCH 10/10] bug 1218576 - Ensure remaining batched telemetry is
 flushed on content process shutdown r=gfritzsche

On content process shutdown we send a content process ping to ensure we have
up-to-date data from the content process before it goes away. Now we need to
also flush the batched telemetry accumulations to the parent so that it can be
present in the ping.

No attempt is made to synchronize access to IPCTimerFired. It is safe to
re-enter.

No attempt is made to cancel the timer as its firing is benign.

MozReview-Commit-ID: 1gjNH9IPhKf
---
 toolkit/components/telemetry/Telemetry.cpp          | 7 +++++++
 toolkit/components/telemetry/TelemetryHistogram.cpp | 7 +++++--
 toolkit/components/telemetry/TelemetrySession.jsm   | 2 +-
 toolkit/components/telemetry/nsITelemetry.idl       | 6 ++++++
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp
index f037a29..39b5791 100644
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -2367,6 +2367,13 @@ TelemetryImpl::ClearScalars()
   return NS_OK;
 }
 
+NS_IMETHODIMP
+TelemetryImpl::FlushBatchedChildTelemetry()
+{
+  TelemetryHistogram::IPCTimerFired(nullptr, nullptr);
+  return NS_OK;
+}
+
 size_t
 TelemetryImpl::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf)
 {
diff --git a/toolkit/components/telemetry/TelemetryHistogram.cpp b/toolkit/components/telemetry/TelemetryHistogram.cpp
index deb9a00..0502b0c 100644
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -2530,8 +2530,11 @@ TelemetryHistogram::GetHistogramSizesofIncludingThis(mozilla::MallocSizeOf
 // It releases the lock before calling out to IPC code which can (and does)
 // Accumulate (which would deadlock)
 //
-// To ensure non-reentrancy, the timer is not released until the method
-// completes
+// To ensure we don't loop IPCTimerFired->AccumulateChild->arm timer, we don't
+// unset gIPCTimerArmed until the IPC completes
+//
+// This function may be re-entered. The shared datastructures gAccumulations and
+// gKeyedAccumulations are guarded by the lock.
 void
 TelemetryHistogram::IPCTimerFired(nsITimer* aTimer, void* aClosure)
 {
diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm
index 4b83adf..b4b574e 100644
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -1834,7 +1834,7 @@ var Impl = {
       // content-child-shutdown is only registered for content processes.
       Services.obs.removeObserver(this, "content-child-shutdown");
       this.uninstall();
-
+      Telemetry.flushBatchedChildTelemetry();
       this.sendContentProcessPing(REASON_SAVED_SESSION);
       break;
     case TOPIC_CYCLE_COLLECTOR_BEGIN:
diff --git a/toolkit/components/telemetry/nsITelemetry.idl b/toolkit/components/telemetry/nsITelemetry.idl
index 54a7a8b..702e73c 100644
--- a/toolkit/components/telemetry/nsITelemetry.idl
+++ b/toolkit/components/telemetry/nsITelemetry.idl
@@ -429,4 +429,10 @@ interface nsITelemetry : nsISupports
    * Resets all the stored scalars. This is intended to be only used in tests.
    */
   void clearScalars();
+
+  /**
+   * Immediately sends any Telemetry batched on this process to the parent
+   * process. This is intended only to be used on process shutdown.
+   */
+  void flushBatchedChildTelemetry();
 };
-- 
2.7.4.windows.1