memory-infra: Add peak-detector skeleton.

base/trace_event/memory_peak_detector_unittest.cc

+// Copyright 2017 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/trace_event/memory_peak_detector.h"
+
+#include <memory>
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/run_loop.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/test/test_timeouts.h"
+#include "base/threading/platform_thread.h"
+#include "base/threading/thread.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "base/trace_event/memory_dump_provider.h"
+#include "base/trace_event/memory_dump_provider_info.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+namespace trace_event {
+
+namespace {
+class MockMemoryDumpProvider : public MemoryDumpProvider {
+ public:
+  MockMemoryDumpProvider(WaitableEvent* evt) : evt_(evt) {}
+  bool OnMemoryDump(const MemoryDumpArgs&, ProcessMemoryDump*) override {
+    NOTREACHED();
+    return true;
+  }
+
+  void PollFastMemoryTotal(uint64_t* memory_total) override { evt_->Signal(); }
+
+  WaitableEvent* const evt_;
+};
+}  // namespace
+
+class MemoryPeakDetectorTest : public testing::Test {
+ public:
+  MemoryPeakDetectorTest() : testing::Test() {}
+
+  void RestartThreadAndReinitializePeakDetector() {
+    if (bg_thread_)
+      bg_thread_->Stop();
+    bg_thread_.reset(new Thread("Peak Detector Test Thread"));
+    bg_thread_->Start();
+    peak_detector_->Initialize(
+        Bind(&MemoryPeakDetectorTest::MockGetDumpProviders, Unretained(this)),
+        bg_thread_->task_runner(),
+        Bind(&MemoryPeakDetectorTest::OnPeakDetectedCallback,
+             Unretained(this)));
+  }
+
+  void SetUp() override {
+    peak_detector_ = MemoryPeakDetector::GetInstance();

[hjd, 2017/04/03 13:25:30]

Maybe it would be better if each test constructed a separate instance of the
MemoryPeakDetector? It seems easy to accidentally add new state and forget to
clean it up in-between tests. This would avoid the need for TearDownForTesting.

[Primiano Tucci (use gerrit), 2017/04/03 15:51:17]

Hmm It's a singleton, so in order to construct it here I'd have to make at least
the dtor public (which would be required for unique_ptr).
Hmm how? The deal with TearDown is that it is always called after each TEST_F
function.
 
But that is what allows to clear up the state between each test. 

[Primiano Tucci (use gerrit), 2017/04/03 20:28:16]

Okay you made very good arguments. Using an actual instance in the test.

+    get_mdp_call_count = 0;
+    if (initialize_peak_detector_during_setup())
+      RestartThreadAndReinitializePeakDetector();
+  }
+
+  void TearDown() override {
+    dump_providers.clear();
+    peak_detector_->TearDownForTesting();
+    bg_thread_->Stop();
+    bg_thread_.reset();
+  }
+
+  // Calls MemoryPeakDetector::state_for_testing() on the bg thread and returns
+  // the result on the current thread.
+  MemoryPeakDetector::State GetPeakDetectorState() {
+    MemoryPeakDetector::State res = MemoryPeakDetector::NOT_INITIALIZED;
+    auto get_fn = [](MemoryPeakDetector* peak_detector, WaitableEvent* evt,
+                     MemoryPeakDetector::State* res) {
+      *res = peak_detector->state_for_testing();
+      evt->Signal();
+    };
+
+    WaitableEvent evt(WaitableEvent::ResetPolicy::MANUAL,
+                      WaitableEvent::InitialState::NOT_SIGNALED);
+    bg_thread_->task_runner()->PostTask(
+        FROM_HERE, Bind(get_fn, Unretained(peak_detector_), Unretained(&evt),
+                        Unretained(&res)));
+    evt.Wait();
+    return res;
+  }
+
+  // Calls MemoryPeakDetector::poll_tasks_count_for_testing() on the bg thread
+  // and returns the result on the current thread.
+  uint32_t GetNumPollingTasksRan() {
+    uint32_t res = 0;
+    auto get_fn = [](MemoryPeakDetector* peak_detector, WaitableEvent* evt,
+                     uint32_t* res) {
+      *res = peak_detector->poll_tasks_count_for_testing();
+      evt->Signal();
+    };
+
+    WaitableEvent evt(WaitableEvent::ResetPolicy::MANUAL,
+                      WaitableEvent::InitialState::NOT_SIGNALED);
+    bg_thread_->task_runner()->PostTask(
+        FROM_HERE, Bind(get_fn, Unretained(peak_detector_), Unretained(&evt),
+                        Unretained(&res)));
+    evt.Wait();
+    return res;
+  }
+
+  void MockGetDumpProviders(MemoryPeakDetector::DumpProvidersList* mdps) {
+    get_mdp_call_count++;
+    *mdps = dump_providers;
+  }
+
+  void OnPeakDetectedCallback() {
+    // TODO(primiano): use in upcoming CLs.
+  }
+
+  scoped_refptr<MemoryDumpProviderInfo> CreateMockDumpProvider(
+      WaitableEvent* evt) {
+    std::unique_ptr<MemoryDumpProvider> mdp(new MockMemoryDumpProvider(evt));
+    MemoryDumpProvider::Options opt;
+    opt.is_fast_polling_supported = true;
+    scoped_refptr<MemoryDumpProviderInfo> mdp_info(
+        new MemoryDumpProviderInfo(mdp.get(), "Mock MDP", nullptr, opt, false));
+
+    // The |mdp| instance will be destroyed together with the |mdp_info|.
+    mdp_info->owned_dump_provider = std::move(mdp);
+    return mdp_info;
+  }
+
+ protected:
+  virtual bool initialize_peak_detector_during_setup() const { return true; }
+
+  MemoryPeakDetector::DumpProvidersList dump_providers;
+  uint32_t get_mdp_call_count;
+  MemoryPeakDetector* peak_detector_;
+  std::unique_ptr<Thread> bg_thread_;
+};
+
+class MemoryPeakDetectorTestNoAutoInitialize : public MemoryPeakDetectorTest {
+  bool initialize_peak_detector_during_setup() const override { return false; }
+};
+
+TEST_F(MemoryPeakDetectorTest, GetDumpProvidersFunctionCalled) {
+  EXPECT_EQ(MemoryPeakDetector::DISABLED, GetPeakDetectorState());
+  peak_detector_->Start();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(1u, get_mdp_call_count);
+  EXPECT_EQ(MemoryPeakDetector::ENABLED, GetPeakDetectorState());
+
+  peak_detector_->Stop();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(MemoryPeakDetector::DISABLED, GetPeakDetectorState());
+  EXPECT_EQ(0u, GetNumPollingTasksRan());
+}
+
+TEST_F(MemoryPeakDetectorTestNoAutoInitialize, NotifyBeforeInitialize) {
+  WaitableEvent evt(WaitableEvent::ResetPolicy::MANUAL,
+                    WaitableEvent::InitialState::NOT_SIGNALED);
+  scoped_refptr<MemoryDumpProviderInfo> mdp = CreateMockDumpProvider(&evt);
+  dump_providers.push_back(mdp);
+  peak_detector_->NotifyMemoryDumpProvidersChanged();

[ssid, 2017/04/03 17:55:28]

Can you add a 
EXPECT_EQ(MemoryPeakDetector::NOT_INITIALIZED, GetPeakDetectorState());

[Primiano Tucci (use gerrit), 2017/04/03 20:28:16]

Good point, done

+  RestartThreadAndReinitializePeakDetector();
+
+  peak_detector_->Start();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(MemoryPeakDetector::RUNNING, GetPeakDetectorState());
+  evt.Wait();  // Wait for a PollFastMemoryTotal() call.
+
+  peak_detector_->Stop();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(MemoryPeakDetector::DISABLED, GetPeakDetectorState());
+  EXPECT_EQ(1u, get_mdp_call_count);
+  EXPECT_GE(GetNumPollingTasksRan(), 1u);
+}
+
+TEST_F(MemoryPeakDetectorTest, DoubleStop) {
+  peak_detector_->Start();
+  bg_thread_->FlushForTesting();
+
+  peak_detector_->Stop();
+  bg_thread_->FlushForTesting();
+
+  peak_detector_->Stop();
+  bg_thread_->FlushForTesting();
+
+  EXPECT_EQ(1u, get_mdp_call_count);
+  EXPECT_EQ(MemoryPeakDetector::DISABLED, GetPeakDetectorState());
+  EXPECT_EQ(0u, GetNumPollingTasksRan());
+}
+
+TEST_F(MemoryPeakDetectorTest, OneDumpProviderRegisteredBeforeStart) {
+  WaitableEvent evt(WaitableEvent::ResetPolicy::MANUAL,
+                    WaitableEvent::InitialState::NOT_SIGNALED);
+  scoped_refptr<MemoryDumpProviderInfo> mdp = CreateMockDumpProvider(&evt);
+  dump_providers.push_back(mdp);
+
+  peak_detector_->Start();
+  bg_thread_->FlushForTesting();
+  evt.Wait();  // Signaled when PollFastMemoryTotal() is called on the MockMDP.
+  EXPECT_EQ(MemoryPeakDetector::RUNNING, GetPeakDetectorState());
+
+  peak_detector_->Stop();
+  bg_thread_->FlushForTesting();
+
+  EXPECT_EQ(1u, get_mdp_call_count);
+  EXPECT_GT(GetNumPollingTasksRan(), 0u);
+}
+
+TEST_F(MemoryPeakDetectorTest, ReInitializeAndRebindToNewThread) {
+  WaitableEvent evt(WaitableEvent::ResetPolicy::MANUAL,
+                    WaitableEvent::InitialState::NOT_SIGNALED);
+  scoped_refptr<MemoryDumpProviderInfo> mdp = CreateMockDumpProvider(&evt);
+  dump_providers.push_back(mdp);
+
+  for (int i = 0; i < 5; ++i) {
+    evt.Reset();
+    peak_detector_->Start();
+    bg_thread_->FlushForTesting();
+    evt.Wait();  // Wait for a PollFastMemoryTotal() call.
+    peak_detector_->Stop();
+
+    // Reinitialize and re-bind to a new task runner.
+    RestartThreadAndReinitializePeakDetector();
+  }
+}
+
+TEST_F(MemoryPeakDetectorTest, OneDumpProviderRegisteredOutOfBand) {
+  peak_detector_->Start();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(MemoryPeakDetector::ENABLED, GetPeakDetectorState());
+  EXPECT_EQ(1u, get_mdp_call_count);
+
+  // Check that no poll tasks are posted before any dump provider is registered.
+  PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+  EXPECT_EQ(0u, GetNumPollingTasksRan());
+
+  // Registed the MDP After Start() has been issued and expect that the
+  // PeakDetector transitions ENABLED -> RUNNING on the next
+  // NotifyMemoryDumpProvidersChanged() call.
+  WaitableEvent evt(WaitableEvent::ResetPolicy::MANUAL,
+                    WaitableEvent::InitialState::NOT_SIGNALED);
+  scoped_refptr<MemoryDumpProviderInfo> mdp = CreateMockDumpProvider(&evt);
+  dump_providers.push_back(mdp);
+  peak_detector_->NotifyMemoryDumpProvidersChanged();
+
+  evt.Wait();  // Signaled when PollFastMemoryTotal() is called on the MockMDP.
+  EXPECT_EQ(MemoryPeakDetector::RUNNING, GetPeakDetectorState());
+  EXPECT_EQ(2u, get_mdp_call_count);
+
+  // Now simulate the unregisration and expect that the PeakDetector transitions
+  // back to ENABLED.
+  dump_providers.clear();
+  peak_detector_->NotifyMemoryDumpProvidersChanged();
+  bg_thread_->FlushForTesting();

[ssid, 2017/04/03 17:55:28]

There is FlushForTesting calls in all the tests. I don't think this simulates
the real world accurately. Look at the comment Hector added about the case where
polling is enabled twice. I haven't reasoned about it fully, but we will miss
such cases if the Flush is called at each check point.
Also, we don't really have to support unregistering all mdp to stop polling.

[Primiano Tucci (use gerrit), 2017/04/03 20:28:16]

I removed all of them but two. Most of them where unneeded because the
evt.wait(), or the GetPeakDetectorState() have the right sequencing.
I need this to simulate time advancing and make sure that the events posted
there happen.
If there is some case missing we have to simulate the right sequence.
Yeah but I don't want to worry about that (think if one day we get gpu fast
polling), feels too fragile.

+  EXPECT_EQ(MemoryPeakDetector::ENABLED, GetPeakDetectorState());
+  EXPECT_EQ(3u, get_mdp_call_count);
+  uint32_t num_poll_tasks = GetNumPollingTasksRan();
+  EXPECT_GT(num_poll_tasks, 0u);
+
+  // At this point, no more polling tasks should be posted.
+  PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+  peak_detector_->Stop();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(num_poll_tasks, GetNumPollingTasksRan());
+}
+
+TEST_F(MemoryPeakDetectorTest, RegisterAndUnregisterTwoDumpProviders) {
+  WaitableEvent evt1(WaitableEvent::ResetPolicy::MANUAL,
+                     WaitableEvent::InitialState::NOT_SIGNALED);
+  WaitableEvent evt2(WaitableEvent::ResetPolicy::MANUAL,
+                     WaitableEvent::InitialState::NOT_SIGNALED);
+  scoped_refptr<MemoryDumpProviderInfo> mdp1 = CreateMockDumpProvider(&evt1);
+  scoped_refptr<MemoryDumpProviderInfo> mdp2 = CreateMockDumpProvider(&evt2);
+
+  // Register only one MDP and start the detector.
+  dump_providers.push_back(mdp1);
+  peak_detector_->Start();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(MemoryPeakDetector::RUNNING, GetPeakDetectorState());
+
+  // Wait for one poll task and then register also the other one.
+  evt1.Wait();
+  dump_providers.push_back(mdp2);
+  peak_detector_->NotifyMemoryDumpProvidersChanged();
+  evt2.Wait();
+  EXPECT_EQ(MemoryPeakDetector::RUNNING, GetPeakDetectorState());
+
+  // Now unregister the first MDP and check that everything is still running.
+  dump_providers.erase(dump_providers.begin());
+  peak_detector_->NotifyMemoryDumpProvidersChanged();
+  EXPECT_EQ(MemoryPeakDetector::RUNNING, GetPeakDetectorState());
+
+  // Now unregister both and check that the detector goes to idle.
+  dump_providers.clear();
+  peak_detector_->NotifyMemoryDumpProvidersChanged();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(MemoryPeakDetector::ENABLED, GetPeakDetectorState());
+
+  // Now re-register both and check that the detector re-activates posting
+  // new polling tasks.
+  uint32_t num_poll_tasks = GetNumPollingTasksRan();
+  evt1.Reset();
+  evt2.Reset();
+  dump_providers.push_back(mdp1);
+  dump_providers.push_back(mdp2);
+  peak_detector_->NotifyMemoryDumpProvidersChanged();
+  bg_thread_->FlushForTesting();
+  evt1.Wait();
+  evt2.Wait();
+  EXPECT_EQ(MemoryPeakDetector::RUNNING, GetPeakDetectorState());
+  EXPECT_GT(GetNumPollingTasksRan(), num_poll_tasks);
+
+  // Stop everything, tear down the MDPs, restart the detector and check that
+  // it detector doesn't accidentally try to re-access them.
+  peak_detector_->Stop();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(MemoryPeakDetector::DISABLED, GetPeakDetectorState());
+  dump_providers.clear();
+  mdp1 = nullptr;
+  mdp2 = nullptr;
+
+  num_poll_tasks = GetNumPollingTasksRan();
+  peak_detector_->Start();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(MemoryPeakDetector::ENABLED, GetPeakDetectorState());
+  PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+  peak_detector_->Stop();
+  bg_thread_->FlushForTesting();
+  EXPECT_EQ(MemoryPeakDetector::DISABLED, GetPeakDetectorState());
+  EXPECT_EQ(num_poll_tasks, GetNumPollingTasksRan());
+
+  EXPECT_EQ(6u, get_mdp_call_count);
+}
+
+}  // namespace trace_event
+}  // namespace base