Move ownership of PowerSaveBlocker from WakeLockServiceContext to WakeLockServiceImpl

device/wake_lock/wake_lock_service_impl.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_impl.h"
 
 #include <utility>
 
-#include "device/wake_lock/wake_lock_service_context.h"
+#include "base/memory/ptr_util.h"
 
 namespace device {
 
-WakeLockServiceImpl::WakeLockServiceImpl(WakeLockServiceContext* context)
-    : context_(context), wake_lock_request_outstanding_(false) {}
+WakeLockServiceImpl::WakeLockServiceImpl(
+    mojom::WakeLockServiceRequest request,
+    device::PowerSaveBlocker::PowerSaveBlockerType type,
+    device::PowerSaveBlocker::Reason reason,
+    const std::string& description,
+    int context_id,
+    WakeLockContextCallback native_view_getter,
+    scoped_refptr<base::SingleThreadTaskRunner> file_task_runner)
+    : type_(type),
+      reason_(reason),
+      description_(base::MakeUnique<std::string>(description)),
+      num_lock_requests_(0),
+#if defined(OS_ANDROID)
+      context_id_(context_id),
+      native_view_getter_(native_view_getter),
+#endif
+      main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      file_task_runner_(std::move(file_task_runner)) {
+  AddClient(std::move(request));
+  binding_set_.set_connection_error_handler(base::Bind(
+      &WakeLockServiceImpl::OnConnectionError, base::Unretained(this)));
+}
 
-WakeLockServiceImpl::~WakeLockServiceImpl() {
-  CancelWakeLock();
+WakeLockServiceImpl::~WakeLockServiceImpl() {}
+
+void WakeLockServiceImpl::AddClient(mojom::WakeLockServiceRequest request) {
+  // Multiple frames that associate to the same WebContents share the same one
+  // WakeLockServiceImpl instance. Two consecutive |RequestWakeLock| requests
+  // from the same frame should be coalesced as one request. Everytime a new

[blundell, 2017/05/05 12:49:38]

s/frame/client

[ke.he, 2017/05/05 14:56:36]

Done.

+  // client is being added into the bindingSet, we create an unique_ptr<bool>
+  // as its context, this context records the clients' outstanding status.

[blundell, 2017/05/05 12:49:37]

, which records this client's request status.

[ke.he, 2017/05/05 14:56:36]

Done.

+  binding_set_.AddBinding(this, std::move(request),
+                          base::MakeUnique<bool>(false));
 }
 
 void WakeLockServiceImpl::RequestWakeLock() {
-  if (wake_lock_request_outstanding_)
+  DCHECK(main_task_runner_->RunsTasksOnCurrentThread());
+  DCHECK(binding_set_.dispatch_context());
+
+  // Uses the Context to get the outstanding status of current binding.
+  // Two consecutive requests from the same client should be coalesced
+  // as one request.
+  if (*binding_set_.dispatch_context()) {
     return;
-
-  wake_lock_request_outstanding_ = true;
-  context_->RequestWakeLock();
+  }
+  *binding_set_.dispatch_context() = true;
+  num_lock_requests_++;
+  UpdateWakeLock();
 }
 
 void WakeLockServiceImpl::CancelWakeLock() {
-  if (!wake_lock_request_outstanding_)
+  DCHECK(main_task_runner_->RunsTasksOnCurrentThread());
+  DCHECK(binding_set_.dispatch_context());
+
+  if (!(*binding_set_.dispatch_context())) {
     return;
+  }
+  *binding_set_.dispatch_context() = false;
+  if (num_lock_requests_ > 0) {
+    num_lock_requests_--;
+    UpdateWakeLock();
+  }
+}
 
-  wake_lock_request_outstanding_ = false;
-  context_->CancelWakeLock();
+void WakeLockServiceImpl::HasWakeLockForTests(
+    const HasWakeLockForTestsCallback& callback) {
+  callback.Run(!!wake_lock_);
+}
+void WakeLockServiceImpl::UpdateWakeLock() {
+  DCHECK(num_lock_requests_ >= 0);
+
+  if (num_lock_requests_) {
+    if (!wake_lock_)
+      CreateWakeLock();
+  } else {
+    if (wake_lock_)
+      RemoveWakeLock();
+  }
+}
+
+void WakeLockServiceImpl::CreateWakeLock() {
+  DCHECK(!wake_lock_);
+  wake_lock_ = base::MakeUnique<device::PowerSaveBlocker>(
+      type_, reason_, *description_, main_task_runner_, file_task_runner_);
+
+#if defined(OS_ANDROID)

[blundell, 2017/05/05 12:49:37]

What I'

+  gfx::NativeView native_view = native_view_getter_.Run(context_id_);

[blundell, 2017/05/05 12:49:37]

What I'm saying here is that this class should be internally consistent: Either
it has a generalized interface and properly implements that generalized
interface, or it hardcodes information internally and acts based on that
hardcoded information. Right now it's in the middle. So I'd either add a
condition here or move the hardcoding from WakeLockServiceContext to
WakeLockServiceImpl for this CL. Probably the former, since the latter would
just need to be undone in the next CL in any case.

[ke.he, 2017/05/05 14:56:36]

Done.

+  if (native_view) {
+    wake_lock_.get()->InitDisplaySleepBlocker(native_view);
+  }
+#endif
+}
+
+void WakeLockServiceImpl::RemoveWakeLock() {
+  DCHECK(wake_lock_);
+  wake_lock_.reset();
+}
+
+void WakeLockServiceImpl::OnConnectionError() {
+  if (binding_set_.dispatch_context()) {

[blundell, 2017/05/05 12:49:37]

Hmm, why the need for this if?

[ke.he, 2017/05/05 14:56:36]

it should be a DCHECK() instead of if().
DCHECK(binding_set_.dispatch_context());

+    // If the error-happening client's wakelock is in outstanding status,
+    // decrease the num_lock_requests and call UpdateWakeLock().
+    if (*binding_set_.dispatch_context() && num_lock_requests_ > 0) {
+      num_lock_requests_--;
+      UpdateWakeLock();
+    }
+  }
+
+  // If |binding_set_| is empty, WakeLockServiceImpl should delele itself.
+  if (binding_set_.empty()) {
+    base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
+  }
 }
 
 }  // namespace device