Add renderer-side client for LeakDetectorRemoteController

chrome/renderer/leak_detector_remote_client.cc

Index: chrome/renderer/leak_detector_remote_client.cc
diff --git a/chrome/renderer/leak_detector_remote_client.cc b/chrome/renderer/leak_detector_remote_client.cc
new file mode 100644
index 0000000000000000000000000000000000000000..369bcceb43d2fba655819cbba6c96afdff8f5d69
--- /dev/null
+++ b/chrome/renderer/leak_detector_remote_client.cc
@@ -0,0 +1,39 @@
+// 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.
+
+#include "chrome/renderer/leak_detector_remote_client.h"
+
+#include "base/bind.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "components/metrics/proto/memory_leak_report.pb.h"
+#include "content/public/common/service_registry.h"
+#include "content/public/renderer/render_thread.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
+
+LeakDetectorRemoteClient::LeakDetectorRemoteClient() {
+  // Connect to Mojo service.
+  content::RenderThread::Get()->GetServiceRegistry()->ConnectToRemoteService(
+      mojo::GetProxy(&remote_service_));
+
+  remote_service_->GetParams(
+      base::Bind(&LeakDetectorRemoteClient::OnParamsReceived,
+                 base::Unretained(this)));

[Ilya Sherman, 2016/06/10 21:27:35]

Why is base::Unretained safe?  Is it because |this| owns the remote_service_,
and therefore the service will be destroyed before |this| is?  If so, and this
is a common pattern, then it's probably fine not to document.  Otherwise, it
would be nice to document.

[Simon Que, 2016/06/16 07:13:27]

I'm actually not sure if this is the correct pattern. I found an example here
that binds a non-member function:
https://www.chromium.org/developers/design-documents/mojo#TOC-Responding-to-R...

However, I didn't find any clear examples of binding a class member function by
poking around the code base for a bit. I'll ask chromium-dev or mojo-dev about
this.

[Ilya Sherman, 2016/06/16 18:14:20]

Okay, sounds like the conclusion on that thread is that it's safe -- yay!  Since
both of us were not quite certain about it, though, it's probably useful to
document the reasoning here.  Maybe someday it'll become a common pattern that
doesn't need documentation, but for now it'll probably help.

+}
+
+LeakDetectorRemoteClient::~LeakDetectorRemoteClient() {}
+
+void LeakDetectorRemoteClient::OnParamsReceived(mojo::String result) {
+  metrics::MemoryLeakReportProto::Params params;
+  if (!params.ParseFromString(result.get()))
+    return;

[Ilya Sherman, 2016/06/10 21:27:35]

Should this be some sort of DCHECK that either the result is empty, or
parseable?

[Simon Que, 2016/06/16 07:13:27]

Done. However, the interface might change to sending the protobuf fields
individually rather than as a serialized string.

+
+  metrics::LeakDetector* detector = metrics::LeakDetector::GetInstance();
+  detector->AddObserver(this);

[Ilya Sherman, 2016/06/10 21:27:35]

Should you unregister as an observer somewhere?

[Simon Que, 2016/06/16 07:13:27]

Done.

+  detector->Init(params, base::ThreadTaskRunnerHandle::Get());
+}
+
+void LeakDetectorRemoteClient::OnLeaksFound(
+    const std::vector<metrics::MemoryLeakReportProto>& reports) {
+  // TODO(sque): Handle reports.
+}