[memory-infra] Make MemoryDumpScheduler non-thread safe singleton

base/trace_event/memory_dump_scheduler.h

Index: base/trace_event/memory_dump_scheduler.h
diff --git a/base/trace_event/memory_dump_scheduler.h b/base/trace_event/memory_dump_scheduler.h
index 7d141ee8e861b9894d04e39a8894a7b68210dbd3..db22233491d1364ff78e403cdeb9b2fbb56fe2e0 100644
--- a/base/trace_event/memory_dump_scheduler.h
+++ b/base/trace_event/memory_dump_scheduler.h
@@ -5,9 +5,12 @@
 #ifndef BASE_TRACE_EVENT_MEMORY_DUMP_SCHEDULER_H
 #define BASE_TRACE_EVENT_MEMORY_DUMP_SCHEDULER_H
 
+#include <memory>
+
 #include "base/base_export.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/singleton.h"

[Primiano Tucci (use gerrit), 2017/03/16 19:01:36]

these days you don't need this boiler plate anymore. Due to C++11 thread-safe
statics you can just write, in the .cc file:

MDM* GetInstance()  {
  static MDM* instance = new MDM();
  return instance;
}

and that's all :)

See for instance
https://cs.chromium.org/chromium/src/media/base/media.cc?rcl=c2c59733fe5acfba...

[ssid, 2017/03/22 01:32:58]

Thanks! fixed.

 #include "base/timer/timer.h"
 #include "base/trace_event/memory_dump_request_args.h"
 
@@ -18,37 +21,44 @@ namespace trace_event {
 
 class MemoryDumpManager;
 
-// Schedules global dump requests based on the triggers added.
+// Schedules global dump requests based on the triggers added. The methods of
+// this class are NOT thread safe and the client has to take care of invoking
+// all the methods of the class safely.
 class BASE_EXPORT MemoryDumpScheduler {
  public:
-  MemoryDumpScheduler(
-      MemoryDumpManager* mdm_,
-      scoped_refptr<SingleThreadTaskRunner> polling_task_runner);
-  ~MemoryDumpScheduler();
+  static MemoryDumpScheduler* GetInstance();
+
+  // Initializes the scheduler. NOT thread safe.
+  void Setup(MemoryDumpManager* mdm_,
+             scoped_refptr<SingleThreadTaskRunner> polling_task_runner);
 
   // Adds triggers for scheduling global dumps. Both periodic and peak triggers
   // cannot be added together. At the moment the periodic support is limited to
   // at most one periodic trigger per dump mode and peak triggers are limited to
   // at most one. All intervals should be an integeral multiple of the smallest
-  // interval specified.
+  // interval specified. NOT thread safe.
   void AddTrigger(MemoryDumpType trigger_type,
                   MemoryDumpLevelOfDetail level_of_detail,
                   uint32_t min_time_between_dumps_ms);
 
-  // Starts periodic dumps.
-  void NotifyPeriodicTriggerSupported();
+  // Starts periodic dumps. NOT thread safe and triggers must be added before
+  // enabling.
+  void EnablePeriodicTriggerIfNeeded();
 
-  // Starts polling memory total.
-  void NotifyPollingSupported();
+  // Starts polling memory total. NOT thread safe and triggers must be added
+  // before enabling.
+  void EnablePollingIfNeeded();
 
   // Resets time for triggering dump to account for minimum time between the
-  // dumps.
+  // dumps. NOT thread safe.
   void NotifyDumpTriggered();
 
-  // Disables all triggers.
+  // Disables all triggers. NOT thread safe. This should be called before
+  // polling thread is stopped to stop polling cleanly.
   void DisableAllTriggers();
 
  private:
+  friend struct DefaultSingletonTraits<MemoryDumpScheduler>;
   friend class MemoryDumpManagerTest;
   friend class MemoryDumpSchedulerPollingTest;
   FRIEND_TEST_ALL_PREFIXES(MemoryDumpManagerTest, TestPollingOnDumpThread);
@@ -81,8 +91,7 @@ class BASE_EXPORT MemoryDumpScheduler {
 
     static const uint32_t kMaxNumMemorySamples = 50;
 
-    explicit PollingTriggerState(
-        scoped_refptr<SingleThreadTaskRunner> polling_task_runner);
+    PollingTriggerState();
     ~PollingTriggerState();
 
     // Helper to clear the tracked memory totals and poll count from last dump.
@@ -91,7 +100,6 @@ class BASE_EXPORT MemoryDumpScheduler {
     State current_state;
     MemoryDumpLevelOfDetail level_of_detail;
 
-    scoped_refptr<SingleThreadTaskRunner> polling_task_runner;
     uint32_t polling_interval_ms;
 
     // Minimum numer of polls after the last dump at which next dump can be
@@ -107,8 +115,11 @@ class BASE_EXPORT MemoryDumpScheduler {
     DISALLOW_COPY_AND_ASSIGN(PollingTriggerState);
   };
 
-  // Helper to set polling disabled on the polling thread.
-  void DisablePolling();
+  MemoryDumpScheduler();
+  ~MemoryDumpScheduler();
+
+  // Helper to set polling disabled.
+  void DisablePollingOnPollingThread();
 
   // Periodically called by the timer.
   void RequestPeriodicGlobalDump();
@@ -130,8 +141,19 @@ class BASE_EXPORT MemoryDumpScheduler {
 
   MemoryDumpManager* mdm_;
 
-  PeriodicTriggerState periodic_state_;
-  PollingTriggerState polling_state_;
+  // Accessed on the thread of the client before enabling and only accessed on
+  // the thread that called "EnablePeriodicTriggersIfNeeded()" after enabling.
+  std::unique_ptr<PeriodicTriggerState> periodic_state_;
+
+  // Accessed on the thread of the client before enabling and only accessed on
+  // the polling thread after enabling.
+  std::unique_ptr<PollingTriggerState> polling_state_;
+
+  // Accessed on the thread of the client only.
+  scoped_refptr<SingleThreadTaskRunner> polling_task_runner_;
+
+  // True when the scheduler is setup. Accessed on the thread of client only.
+  bool is_setup_;
 
   DISALLOW_COPY_AND_ASSIGN(MemoryDumpScheduler);
 };