Device Service: Decouple Wake Lock from //content

device/wake_lock/wake_lock_service_context.cc

Index: device/wake_lock/wake_lock_service_context.cc
diff --git a/device/wake_lock/wake_lock_service_context.cc b/device/wake_lock/wake_lock_service_context.cc
index d30f8cdaadd5f8a7224a237932bfb02bc12e1467..b56680f3806510ef241cc5a6ef5cbf12f0ddc495 100644
--- a/device/wake_lock/wake_lock_service_context.cc
+++ b/device/wake_lock/wake_lock_service_context.cc
@@ -11,26 +11,36 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "device/power_save_blocker/power_save_blocker.h"
-#include "mojo/public/cpp/bindings/strong_binding.h"
 
 namespace device {
 
 WakeLockServiceContext::WakeLockServiceContext(
+    mojom::WakeLockContextRequest request,

[dcheng, 2017/03/22 10:09:55]

Sorry, one more lifetime question...

Is it possible that |request| is already disconnected at this point? In that
case, would the connection error handler still be called?

[blundell, 2017/03/22 13:03:40]

Any messages sent from the other end of the pipe bound to |request| are queued
until this end of the pipe is bound. I imagine that the other end of the pipe
disconnecting is treated as simply the last message to be sent from that end. I
got a little lost trying to actually track this down in the code. Ken can verify
I'm sure.

In any case, I think that this must work because otherwise StrongBinding would
be broken:

https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/strong_binding....

[Ken Rockot(use gerrit already), 2017/03/22 18:25:05]

+1 to all of that. A pipe being disconnected is not equivalent to it being
unreadable. We only signal a binding error once it becomes unreadable, i.e.
disconnected AND no remaining unread messages.

[dcheng, 2017/03/22 20:07:32]

OK, so the transition to the disconnected state doesn't happen /until/ we try to
read from the pipe. So even if the pipe is broken while the message is in
flight, this is OK: we'll get the connection error when we try to read from the
pipe.

Is this summary correct?

+    int context_id,
     scoped_refptr<base::SingleThreadTaskRunner> file_task_runner,
-    base::Callback<gfx::NativeView()> native_view_getter)
+    const WakeLockContextCallback& native_view_getter)
     : main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
-      file_task_runner_(file_task_runner),
+      file_task_runner_(std::move(file_task_runner)),
       num_lock_requests_(0),
+#if defined(OS_ANDROID)
+      context_id_(context_id),
       native_view_getter_(native_view_getter),
-      weak_factory_(this) {}
+#endif
+      context_binding_(this, std::move(request)),
+      context_binding_encountered_error_(false) {
+  context_binding_.set_connection_error_handler(base::Bind(
+      &WakeLockServiceContext::OnContextBindingError, base::Unretained(this)));
+  wake_lock_bindings_.set_connection_error_handler(
+      base::Bind(&WakeLockServiceContext::DestroyIfNoLongerNeeded,
+                 base::Unretained(this)));
+}
 
 WakeLockServiceContext::~WakeLockServiceContext() {}
 
-void WakeLockServiceContext::CreateService(
+void WakeLockServiceContext::GetWakeLock(
     mojo::InterfaceRequest<mojom::WakeLockService> request) {
-  mojo::MakeStrongBinding(
-      base::MakeUnique<WakeLockServiceImpl>(weak_factory_.GetWeakPtr()),
-      std::move(request));
+  wake_lock_bindings_.AddBinding(base::MakeUnique<WakeLockServiceImpl>(this),
+                                 std::move(request));
 }
 
 void WakeLockServiceContext::RequestWakeLock() {
@@ -45,8 +55,9 @@ void WakeLockServiceContext::CancelWakeLock() {
   UpdateWakeLock();
 }
 
-bool WakeLockServiceContext::HasWakeLockForTests() const {
-  return !!wake_lock_;
+void WakeLockServiceContext::HasWakeLockForTests(
+    const HasWakeLockForTestsCallback& callback) {
+  callback.Run(!!wake_lock_);
 }
 
 void WakeLockServiceContext::CreateWakeLock() {
@@ -57,7 +68,7 @@ void WakeLockServiceContext::CreateWakeLock() {
       main_task_runner_, file_task_runner_));
 
 #if defined(OS_ANDROID)
-  gfx::NativeView native_view = native_view_getter_.Run();
+  gfx::NativeView native_view = native_view_getter_.Run(context_id_);
   if (native_view) {
     wake_lock_.get()->InitDisplaySleepBlocker(native_view);
   }
@@ -80,4 +91,18 @@ void WakeLockServiceContext::UpdateWakeLock() {
   }
 }
 
+void WakeLockServiceContext::OnContextBindingError() {
+  context_binding_encountered_error_ = true;
+  DestroyIfNoLongerNeeded();
+}
+
+void WakeLockServiceContext::DestroyIfNoLongerNeeded() {
+  if (context_binding_encountered_error_ && wake_lock_bindings_.empty()) {
+    // Delete this instance once there are no more live connections to it.
+    // However, ensure that this instance stays alive throughout the destructor
+    // of a WakeLockServiceImpl instance that might be triggering this callback.
+    base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
+  }
+}
+
 }  // namespace device