Convert renderer JS memory usage reporting to use Mojo.

chrome/browser/process_resource_usage.h

Index: chrome/browser/process_resource_usage.h
diff --git a/chrome/browser/process_resource_usage.h b/chrome/browser/process_resource_usage.h
index ad295a6bf5528c37f95045821f9c0870a50706fe..46061e9fa1f03316e46f281fd8595a02969f8fd9 100644
--- a/chrome/browser/process_resource_usage.h
+++ b/chrome/browser/process_resource_usage.h
@@ -5,9 +5,13 @@
 #ifndef CHROME_BROWSER_PROCESS_RESOURCE_USAGE_H_
 #define CHROME_BROWSER_PROCESS_RESOURCE_USAGE_H_
 
+#include <deque>
+
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/threading/thread_checker.h"
 #include "chrome/common/resource_usage_reporter.mojom.h"
+#include "third_party/mojo/src/mojo/public/cpp/bindings/error_handler.h"
 
 // Provides resource usage information about a child process.
 //
@@ -28,14 +32,15 @@
 //
 // Note: ProcessResourceUsage is thread-hostile and must live on a single
 // thread.
-class ProcessResourceUsage {
+class ProcessResourceUsage : public mojo::ErrorHandler {
  public:
   // Must be called from the same thread that created |service|.
   explicit ProcessResourceUsage(ResourceUsageReporterPtr service);
-  ~ProcessResourceUsage();
+  ~ProcessResourceUsage() override;
 
-  // Refresh the resource usage information.
-  void Refresh();
+  // Refresh the resource usage information. |callback| is invoked when the
+  // usage data is updated, or when the IPC connection is lost.
+  void Refresh(const base::Closure& callback);
 
   // Get V8 memory usage information.
   bool ReportsV8MemoryStats() const;
@@ -46,8 +51,14 @@ class ProcessResourceUsage {
   // Mojo IPC callback.
   void OnRefreshDone(ResourceUsageDataPtr data);
 
+  // mojo::ErrorHandler implementation:
+  void OnConnectionError() override;

[ncarter (slow), 2015/05/14 06:28:12]

Making this method private doesn't really hide it from clients of
ProcessResourceUsage -- because it's public in the base class. So I'd say these
ought to be public as well.

Since the paradigm here is "ProcessResourceUsage is implemented in terms of
mojo::ErrorHandler" instead of "ProcessResourceUsage is a kind of
mojo::ErrorHandler", you could hide these methods from clients of
ProcessResourceUsage by using composition instead of inheritance.

Generally, when I'm tempted to change the access specifier in the derived class
to be different from that in a base interface class, an alternative is to move
the interface implementation into a helper class of which the parent class has a
private member, and the helper class keeps a pointer back to the parent class if
needed. If you need to privately implement more than one interface, this can
become PIMPL, where all the data members from the parent type move into the
helper type.

[Anand Mistry (off Chromium), 2015/05/14 07:30:41]

Moved to a private helper class. I'd rather not go PIMPL just yet, but it would
be useful if I want to remove the exposed dependency on Mojo, which would be
nice. However, that's more work than I think it's worth at this point.

+
+  void RunPendingRefreshCallbacks();
+
   ResourceUsageReporterPtr service_;
   bool update_in_progress_;
+  std::deque<base::Closure> refresh_callbacks_;
 
   ResourceUsageDataPtr stats_;