Add MemoryCoordinatorImpl

content/browser/memory/memory_coordinator_impl.h

Index: content/browser/memory/memory_coordinator_impl.h
diff --git a/content/browser/memory/memory_coordinator_impl.h b/content/browser/memory/memory_coordinator_impl.h
new file mode 100644
index 0000000000000000000000000000000000000000..42aab409057e0833f5493124f36af75cd02546e5
--- /dev/null
+++ b/content/browser/memory/memory_coordinator_impl.h
@@ -0,0 +1,67 @@
+// Copyright 2016 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.
+
+#ifndef CONTENT_BROWSER_MEMORY_MEMORY_COORDINATOR_IMPL_H_
+#define CONTENT_BROWSER_MEMORY_MEMORY_COORDINATOR_IMPL_H_
+
+#include "base/callback.h"
+#include "base/memory/singleton.h"
+#include "base/memory/weak_ptr.h"
+#include "base/single_thread_task_runner.h"
+#include "base/threading/non_thread_safe.h"
+#include "base/time/time.h"
+#include "content/browser/memory/memory_coordinator.h"
+
+namespace content {
+
+class MemoryMonitor;
+struct MemoryCoordinatorSingletonTraits;
+
+class MemoryCoordinatorImpl : public MemoryCoordinator,
+                              public base::NonThreadSafe {
+ public:
+  MemoryCoordinatorImpl(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+                        std::unique_ptr<MemoryMonitor> monitor);
+  ~MemoryCoordinatorImpl() override;
+
+  void Start() override;
+
+  void OnChildAdded(int render_process_id) override;
+
+ private:
+  friend struct MemoryCoordinatorSingletonTraits;
+
+  using MemoryState = base::MemoryState;
+
+  MemoryState CalculateNextState();
+  void UpdateState();
+  void NotifyStateToClients();
+  void NotifyStateToChildren();
+
+  void ScheduleUpdateState(base::TimeDelta delay);
+
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  std::unique_ptr<MemoryMonitor> memory_monitor_;
+  base::Closure update_state_callback_;
+  base::MemoryState current_state_ = MemoryState::NORMAL;
+  base::TimeTicks last_state_change_;
+
+  // These parameters are used to determine the global state.
+  // TODO(bashi): Provide a way to change these parameters.
+  int predicted_renderer_size_;
+  int num_children_until_throttled_;
+  int num_children_until_suspended_;
+  int num_children_back_to_normal_;
+  int num_children_back_to_throttled_;
+  base::TimeDelta minimum_transition_period_;
+  base::TimeDelta monitoring_iterval_;

[chrisha, 2016/10/06 13:35:54]

Unless these are needed for unittesting, or otherwise need to be exposed, I'd
make these all static constants in an anonymous namespace in the .cc file. If
and when they need to be exposed for testing or experimentation, then move them
as need be.

num_children_* is slightly confusing, as this has nothing to do with the current
number of children, rather the expected number of new children/renderers that
could be created. Maybe new_renderers_* ?

I would also prefer to see detailed comments for each of these configuration
items, explaining exactly how they work.

// The median size of a renderer on the current platform. This is used to
convert
// the amount of free memory to an expected number of new renderers that could
be
// started before hitting critical memory pressure.
int predicted_renderer_size_;
// When in a NORMAL state and the potential number of new renderers drop below
this
// level, the coordinator will transition to a THROTTLED state.
int new_renderers_until_throttled_;

Somewhere a detailed description of the current logic would be useful (deadband,
minimum transition time, etc)

[bashi, 2016/10/07 02:10:20]

Added comments. I still put these parameters in this class because I think we
would want to tweak these parameters on Finch or local experiments.

+
+  base::WeakPtrFactory<MemoryCoordinatorImpl> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(MemoryCoordinatorImpl);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_MEMORY_MEMORY_COORDINATOR_IMPL_H_