Convert devtools to be client of WakeLock mojo service.

content/browser/devtools/render_frame_devtools_agent_host.cc

 // Copyright (c) 2012 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 "content/browser/devtools/render_frame_devtools_agent_host.h"
 
 #include <tuple>
 #include <utility>
 
 #include "base/guid.h"
@@ skipping 32 matching lines @@
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 
 #if defined(OS_ANDROID)
 #include "content/public/browser/render_widget_host_view.h"
-#include "device/power_save_blocker/power_save_blocker.h"
+#include "device/wake_lock/public/interfaces/wake_lock_context.mojom.h"
 #endif
 
 namespace content {
 
 typedef std::vector<RenderFrameDevToolsAgentHost*> Instances;
 
 namespace {
 base::LazyInstance<Instances>::Leaky g_instances = LAZY_INSTANCE_INITIALIZER;
 
 static RenderFrameDevToolsAgentHost* FindAgentHost(RenderFrameHost* host) {
@@ skipping 532 matching lines @@
     current_->InspectElement(session->session_id(), x, y);
   if (pending_)
     pending_->InspectElement(session->session_id(), x, y);
 }
 
 void RenderFrameDevToolsAgentHost::OnClientAttached() {
   if (!web_contents())
     return;
 
   frame_trace_recorder_.reset(new DevToolsFrameTraceRecorder());
-  CreatePowerSaveBlocker();
+#if defined(OS_ANDROID)
+  GetWakeLockService()->RequestWakeLock();
+#endif
 }
 
 void RenderFrameDevToolsAgentHost::OnClientDetached() {
 #if defined(OS_ANDROID)
-  power_save_blocker_.reset();
+  GetWakeLockService()->CancelWakeLock();
 #endif
   frame_trace_recorder_.reset();
   in_navigation_protocol_message_buffer_.clear();
 }
 
 RenderFrameDevToolsAgentHost::~RenderFrameDevToolsAgentHost() {
   Instances::iterator it = std::find(g_instances.Get().begin(),
                                      g_instances.Get().end(),
                                      this);
   if (it != g_instances.Get().end())
@@ skipping 194 matching lines @@
     return false;
   if (IsBrowserSideNavigationEnabled())
     return true;
   if (!frame_tree_node_)
     return !handlers_frame_host_;
   RenderFrameHostManager* manager = frame_tree_node_->render_manager();
   return handlers_frame_host_ == manager->current_frame_host() ||
       handlers_frame_host_ == manager->pending_frame_host();
 }
 
-void RenderFrameDevToolsAgentHost::CreatePowerSaveBlocker() {
 #if defined(OS_ANDROID)
-  power_save_blocker_.reset(new device::PowerSaveBlocker(
-      device::PowerSaveBlocker::kPowerSaveBlockPreventDisplaySleep,
-      device::PowerSaveBlocker::kReasonOther, "DevTools",
-      BrowserThread::GetTaskRunnerForThread(BrowserThread::UI),
-      BrowserThread::GetTaskRunnerForThread(BrowserThread::FILE)));
-  if (web_contents()->GetNativeView()) {
-    power_save_blocker_->InitDisplaySleepBlocker(
-        web_contents()->GetNativeView());
+device::mojom::WakeLockService*
+RenderFrameDevToolsAgentHost::GetWakeLockService() {

[dgozman, 2017/05/18 21:58:45]

I'm not very good at mojo yet, so let me ask a naive question: shouldn't there
be a more centralized place for the service, like WebContents? What's the
difference between WakeLockService and WakeLockContext?

[ke.he, 2017/05/18 23:58:47]

The design of WakeLock servicification is necessarily very complex. see design
doc:
https://docs.google.com/document/d/1AEmiwtEj1nfTCmqSneht0xad2k3huxZQsIRHvVz0S8c

WakeLockService is the instance that manages the PowerSaveBLocker. The
WakeLockContext is designed to identify WakeLockService to one instance of
WebContents, because different WebContents has different native_view, which is
needed by PowerSaveBlocker (only on Android). So in Android case, user needs to
get a WakeLockContext first, then use the WakeLockContext to create a
WakeLockService, in this way the WakeLockService knows which WebContents's
native_view it is operating on.

MediaWebContentsObsever is another client that is very similar to this See
https://codereview.chromium.org/2846813002/

+  // Here is a lazy binding, and will not reconnect after connection error.
+  if (!wake_lock_) {
+    device::mojom::WakeLockServiceRequest request =
+        mojo::MakeRequest(&wake_lock_);

[dgozman, 2017/05/18 21:58:45]

Inline this into line 841?

[ke.he, 2017/05/18 23:58:47]

We don't inline this on purpose, so the |wake_lock_| can always be bound by
"mojo::MakeRequest", then it is safe to make mojo call even if the connection is
not created.

[dgozman, 2017/05/19 19:15:45]

Do you mean that wake_lock_ will be functional (although noop) even when there
is no wake_lock_context? Interesting...

[ke.he, 2017/05/20 01:00:34]

Yes, mojo::MakeRequest() make it be functional, but noop without
wake_lock_context_ connect it to remote side.

+    device::mojom::WakeLockContext* wake_lock_context =
+        web_contents()->GetWakeLockContext();
+    if (wake_lock_context) {
+      wake_lock_context->GetWakeLock(
+          device::mojom::WakeLockType::PreventAppSuspension,

[ke.he, 2017/05/26 03:11:12]

Oh! The original PowerSaveBlocker type is "kPowerSaveBlockPreventDisplaySleep",
I mistakenly set it as "PreventAppSuspension" here!
I guess this is the reason mentioned in Issue 726290

+          device::mojom::WakeLockReason::ReasonOther, "DevTools",
+          std::move(request));
+    }
   }
+  return wake_lock_.get();
+}
 #endif
-}
 
 void RenderFrameDevToolsAgentHost::RenderProcessGone(
     base::TerminationStatus status) {
   switch(status) {
     case base::TERMINATION_STATUS_ABNORMAL_TERMINATION:
     case base::TERMINATION_STATUS_PROCESS_WAS_KILLED:
 #if defined(OS_CHROMEOS)
     case base::TERMINATION_STATUS_PROCESS_WAS_KILLED_BY_OOM:
 #endif
     case base::TERMINATION_STATUS_PROCESS_CRASHED:
@@ skipping 49 matching lines @@
   pending_handle_ = nullptr;
   DCHECK(CheckConsistency());
 }
 
 void RenderFrameDevToolsAgentHost::DidDetachInterstitialPage() {
   for (auto* page : protocol::PageHandler::ForAgentHost(this))
     page->DidDetachInterstitialPage();
 }
 
 void RenderFrameDevToolsAgentHost::WasShown() {
-  CreatePowerSaveBlocker();
+#if defined(OS_ANDROID)
+  GetWakeLockService()->RequestWakeLock();
+#endif
 }
 
 void RenderFrameDevToolsAgentHost::WasHidden() {
 #if defined(OS_ANDROID)
-  power_save_blocker_.reset();
+  GetWakeLockService()->CancelWakeLock();

[dgozman, 2017/05/18 21:58:45]

I think we should destroy the service at this point as well.

[ke.he, 2017/05/18 23:58:47]

we don't need to destroy the service here or in destructor. when the
|wake_lock_| gets destructed, the remote Service will trigger
WakeLockServiceImpl::OnConnectionError() and destroy itself.

[dgozman, 2017/05/19 19:15:44]

Shouldn't we do it from the performance standpoint? How expensive holding the
service is?

[ke.he, 2017/05/20 01:00:34]

In GetWakeLockService(), the |wake_lock_| is initialized(bind and connect) only
for the first time. 

In the remote service side, the WakeLockServiceImpl will be created when client
makes the first mojo call(RequestWakeLock() or CancelWakeLock()) and will be
destroyed when |wake_lock_| is destructed.

So if we destroy the |wake_lock_| here, we need to create and init it again
before next use. I think we can just keep the remote WakeLockServiceImpl alive
as long as this RenderFrameDevToolsAgentHost alive. So we can avoid creating and
deleting remote service instance again and again.

 #endif
 }
 
 void RenderFrameDevToolsAgentHost::DidReceiveCompositorFrame() {
   const cc::CompositorFrameMetadata& metadata =
       RenderWidgetHostImpl::From(
           web_contents()->GetRenderViewHost()->GetWidget())
           ->last_frame_metadata();
   for (auto* page : protocol::PageHandler::ForAgentHost(this))
     page->OnSwapCompositorFrame(metadata.Clone());
@@ skipping 245 matching lines @@
     RenderFrameHost* host) {
   return (current_ && current_->host() == host) ||
       (pending_ && pending_->host() == host);
 }
 
 bool RenderFrameDevToolsAgentHost::IsChildFrame() {
   return current_ && current_->host()->GetParent();
 }
 
 }  // namespace content