Stop using callbacks in MemoryCoordinatorProxy

base/memory/memory_coordinator_proxy.h

Index: base/memory/memory_coordinator_proxy.h
diff --git a/base/memory/memory_coordinator_proxy.h b/base/memory/memory_coordinator_proxy.h
index 4148da5dceec0c59e39462a6f740569263432ba9..2adf5657de490b5f1bdbd05b6c4fbf5e5babe3ab 100644
--- a/base/memory/memory_coordinator_proxy.h
+++ b/base/memory/memory_coordinator_proxy.h
@@ -6,33 +6,38 @@
 #define BASE_MEMORY_MEMORY_COORDINATOR_PROXY_H_
 
 #include "base/base_export.h"
-#include "base/callback.h"
 #include "base/memory/memory_coordinator_client.h"
 #include "base/memory/singleton.h"
+#include "base/memory/weak_ptr.h"
 
 namespace base {
 
+// An interface to MemoryCoordinator. See comments in MemoryCoordinatorProxy for
+// method descriptions.
+class BASE_EXPORT MemoryCoordinatorInterface {
+ public:
+  virtual ~MemoryCoordinatorInterface() {}
+
+  virtual MemoryState GetLocalMemoryState() = 0;
+  virtual void SetMemoryStateForTesting(MemoryState state) = 0;
+};
+
 // The proxy of MemoryCoordinator to be accessed from components that are not
 // in content/browser e.g. net.
 class BASE_EXPORT MemoryCoordinatorProxy {
  public:
-  using GetCurrentMemoryStateCallback = base::Callback<MemoryState()>;
-  using SetCurrentMemoryStateCallback = base::Callback<void(MemoryState)>;
-
   static MemoryCoordinatorProxy* GetInstance();
 
-  // Returns the current memory state.
-  MemoryState GetCurrentMemoryState() const;
-
-  // Sets the current memory state. This function is for testing only.
-  void SetCurrentMemoryStateForTesting(MemoryState memory_state);
+  // Sets an instance of MemoryCoordinator
+  void Set(WeakPtr<MemoryCoordinatorInterface> instance);
 
-  // Sets state-getter callback.
-  void SetGetCurrentMemoryStateCallback(GetCurrentMemoryStateCallback callback);
+  // Returns the process's current memory state. Note that the local state
+  // could be different from the global memory state as some processes cannot
+  // be suspended.
+  MemoryState GetLocalMemoryState() const;
 
-  // Sets state-setter callback.
-  void SetSetCurrentMemoryStateForTestingCallback(
-      SetCurrentMemoryStateCallback callback);
+  // Sets the memory state of this process. This function is for testing only.
+  void SetMemoryStateForTesting(MemoryState memory_state);

[haraken, 2016/12/12 06:51:57]

This sets the global memory state, not the memory state of this process, right?

SetGlobalMemoryStateForTesting ?

[bashi, 2016/12/12 07:17:55]

This is a tricky part in some reasons. First, from caller's perspective, this
may look a bit strange:

SetGlobalMemoryState(NORMAL)
...
EXPECT_EQ(NORMAL, GetLocalMemoryState())

Second, thinking about renderer side, there wouldn't be a way to set the global
state, so above code doesn't look correct (note that proxy exists to provide
common interface to both browser/renderers).

FWIW I was thinking about moving GetGlobalMemoryState() into this class, but I
didn't do that because of the same reason.

That said, there are the same problem for a method which I'm going to add in
follow-up CLs. In follow-up CLs, I'll add something like below:

MemoryState GetStateForProcess(ProcessHandle);

This wouldn't work on renderer side. I was thinking better ways to avoid it, but
I couldn't come up with better ideas (Moving MemoryCoordinator to content/public
is another idea, but it adds another abstraction layer).

 
  private:
   friend struct base::DefaultSingletonTraits<MemoryCoordinatorProxy>;
@@ -40,8 +45,7 @@ class BASE_EXPORT MemoryCoordinatorProxy {
   MemoryCoordinatorProxy();
   virtual ~MemoryCoordinatorProxy();
 
-  GetCurrentMemoryStateCallback getter_callback_;
-  SetCurrentMemoryStateCallback setter_callback_;
+  WeakPtr<MemoryCoordinatorInterface> instance_;
 
   DISALLOW_COPY_AND_ASSIGN(MemoryCoordinatorProxy);
 };