Device Service: Decouple Wake Lock from //content

device/wake_lock/wake_lock_service_context.cc

 // Copyright 2015 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 "device/wake_lock/wake_lock_service_context.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #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,
+    int context_id,
     scoped_refptr<base::SingleThreadTaskRunner> file_task_runner,
-    base::Callback<gfx::NativeView()> native_view_getter)
+    WakeLockContextCallback native_view_getter)

[dcheng, 2017/03/17 06:55:26]

Nit: either move or pass by const ref (pass by const ref is the general
convention atm, but there's a vague plan to migrate most things to OnceCallback,
which will imply the former)

[blundell, 2017/03/17 12:28:22]

Done.

     : main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
       file_task_runner_(file_task_runner),

[dcheng, 2017/03/17 06:55:26]

Nit: std::move

[blundell, 2017/03/17 12:28:22]

Done.

       num_lock_requests_(0),
+      binding_(this, std::move(request)),
+#if defined(OS_ANDROID)
+      context_id_(context_id),
       native_view_getter_(native_view_getter),
-      weak_factory_(this) {}
+#endif
+      num_connections_(1) {
+  binding_.set_connection_error_handler(base::Bind(
+      &WakeLockServiceContext::OnConnectionError, 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));
+  num_connections_++;
+  mojo::StrongBindingPtr<mojom::WakeLockService> binding =
+      mojo::MakeStrongBinding(base::MakeUnique<WakeLockServiceImpl>(this),
+                              std::move(request));
+  binding->set_connection_error_handler(base::Bind(
+      &WakeLockServiceContext::OnConnectionError, base::Unretained(this)));
 }
 
 void WakeLockServiceContext::RequestWakeLock() {
   DCHECK(main_task_runner_->RunsTasksOnCurrentThread());
   num_lock_requests_++;
   UpdateWakeLock();
 }
 
 void WakeLockServiceContext::CancelWakeLock() {
   DCHECK(main_task_runner_->RunsTasksOnCurrentThread());
   num_lock_requests_--;
   UpdateWakeLock();
 }
 
-bool WakeLockServiceContext::HasWakeLockForTests() const {
-  return !!wake_lock_;
+void WakeLockServiceContext::HasWakeLockForTests(
+    const HasWakeLockForTestsCallback& callback) {
+  callback.Run(!!wake_lock_);
 }
 
 void WakeLockServiceContext::CreateWakeLock() {
   DCHECK(!wake_lock_);
   wake_lock_.reset(new device::PowerSaveBlocker(
       device::PowerSaveBlocker::kPowerSaveBlockPreventDisplaySleep,
       device::PowerSaveBlocker::kReasonOther, "Wake Lock API",
       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);
   }
 #endif
 }
 
 void WakeLockServiceContext::RemoveWakeLock() {
   DCHECK(wake_lock_);
   wake_lock_.reset();
 }
 
 void WakeLockServiceContext::UpdateWakeLock() {
   DCHECK(num_lock_requests_ >= 0);
   if (num_lock_requests_) {
     if (!wake_lock_)
       CreateWakeLock();
   } else {
     if (wake_lock_)
       RemoveWakeLock();
   }
 }
 
+void WakeLockServiceContext::OnConnectionError() {
+  DCHECK(num_connections_ > 0);

[dcheng, 2017/03/17 06:55:26]

Nit: DCHECK_GT

[blundell, 2017/03/17 12:28:22]

Done.

+  num_connections_--;
+
+  // Delete this instance when 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 by
+  // posting the deletion as a task.
+  if (num_connections_ == 0)
+    base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);

[Ken Rockot(use gerrit already), 2017/03/16 17:48:40]

I haven't understood why this is necessary, i.e. why we must outlive
WakeLockServiceImpl. Can you elaborate? Would be really nice if we could at
least use StrongBindingSet.

[blundell, 2017/03/17 12:28:22]

WakeLockServiceImpl calls back into this class on its destructor, so either this
class needs to outlive WakeLockServiceImpl or the latter needs to have a
WeakPtr. I opted for the former because I like to avoid WeakPtr when possible.

Would be really nice if we could at
Done.

+}
+
 }  // namespace device