New issue
Advanced search Search tips

Issue 689403 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 689402

Blocking:
issue 612346
issue 689428
issue 701414



Sign in to add a comment

Convert Wake Lock impl to use delegate interface that maps connecting service ID to NativeView for obtaining current view on Android

Project Member Reported by blundell@chromium.org, Feb 7 2017

Issue description

The Wake Lock impl needs to supply the current view to the relevant platform APIs on Android to prevent the display from going to sleep. Currently it does this by calling its associated WebContents via a passed-in callback. This dependency prevents the Device Service from hosting the Wake Lock interface. Instead, the Wake Lock impl should use the client library described in  crbug.com/689402  to obtain the View that is associated with its connecting service, thus eliminating the need for Wake Lock to be hosted by RenderFrameHostImpl (i.e., tied directly to a WebContents).
 
Blocking: 689428
Owner: blundell@chromium.org
Status: Assigned (was: Available)
Summary: Convert Wake Lock impl to use delegate interface that maps connecting service ID to NativeView for obtaining current view on Android (was: Convert Wake Lock impl to use client library for obtaining current view on Android)
Update: As described on this doc: https://docs.google.com/document/d/1DlvoBWGVrSSdPsPKx55g9GaSDTVl3m3G74rQmQN1OxQ/edit, we've decided on a different approach to handling this dependency. We're going to go with per-feature delegate interfaces that are used only on the platforms on which they're required and are passed into the Device Service in its constructor on those platforms. wake_lock_delegate.h will have the following interface:

gfx::NativeView GetViewForServiceInstance(Identity instance_identity)

The embedder will be responsible for implementing this interface.


Blocking: 701414
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e

commit e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e
Author: blundell <blundell@chromium.org>
Date: Mon Mar 27 08:11:17 2017

Device Service: Decouple Wake Lock from //content

Wake Lock presents challenges in decoupling from //content:

(1) It is naturally a per-WebContents interface, as it coalesces
requests made from frames within the same WebContents.
(2) On Android, it needs the NativeView of the WebContents in order to
block the screen from going to sleep.

This CL takes the following approach:
(1) Adds the notion of a context ID, which is set on WakeLockServiceContext
and which the Device Service can use to map contexts to NativeViews (via
a callback that is passed in at Device Service construction time by its
embedder).
(2) Adds a WakeLockContext Mojo interface. This interface can be used to
forward requests to bind wake locks from untrusted clients that are made within
the same (trusted-client specific) context.
(3) Adds a WakeLockContextProvider Mojo interface. This interface allows
for getting a WakeLockContext that is associated with a specific context ID.
The WakeLockContext impl uses that context ID on Android to obtain the
NativeView for its context.

Note that the lifetime model of WakeLockContext is somewhat complex: It must
stay alive as long as either
(1) Its Mojo connection is still valid (as the client might make future
    GetWakeLock() calls) OR
(2) There are still live WakeLock instances that it has instantiated (since
    they call into it when they receive Mojo requests from *their* clients).

Consequently, WakeLockContext monitors the state of the connections described
in (1) and (2), dying only when *all* of those connections go away. As a final
subtlety, WakeLockServiceImpl calls back into WakeLockContext from its
destructor, which executes immediately *after* WakeLockContext receives the
callback that that WakeLockServiceImpl instance has experienced a connection
error. WakeLockContext thus posts a task to delete itself rather than deleting
itself synchronously.

BUG= 689403 
TEST=On Chrome on Android, go to about://flags and turn on "Experimental Web
Platform features." Restart Chrome. Go to
https://colinblundell.github.io/wake_lock.html and verify that the page
says "Wake Lock API found". Verify that if you wait ~40 seconds, the device
goes to sleep. Tap "Block sleep." Verify that if you wait longer than the
above period, the device does not go to sleep. Tap "Unblock sleep" and
verify again that the device goes to sleep after waiting.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2734943003
Cr-Commit-Position: refs/heads/master@{#459719}

[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/BUILD.gn
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/DEPS
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/frame_host/render_frame_host_delegate.cc
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/frame_host/render_frame_host_delegate.h
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/wake_lock/wake_lock_browsertest.cc
[add] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/wake_lock/wake_lock_context_host.cc
[add] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/wake_lock/wake_lock_context_host.h
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/content/test/BUILD.gn
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/BUILD.gn
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/BUILD.gn
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/public/interfaces/BUILD.gn
[add] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/public/interfaces/README.md
[add] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/public/interfaces/wake_lock_context.mojom
[add] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/public/interfaces/wake_lock_context_provider.mojom
[add] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/wake_lock_context_provider.cc
[add] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/wake_lock_context_provider.h
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/wake_lock_service_context.cc
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/wake_lock_service_context.h
[delete] https://crrev.com/e93a5f83b296e8294f0726f1426dbc2dbf131d33/device/wake_lock/wake_lock_service_context_unittest.cc
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/wake_lock_service_impl.cc
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/device/wake_lock/wake_lock_service_impl.h
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/services/device/BUILD.gn
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/services/device/DEPS
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/services/device/device_service.cc
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/services/device/device_service.h
[modify] https://crrev.com/e75a8f91ce3a7a09d0da15f1c1fc83a79a668b8e/services/device/manifest.json

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d8cd72b77ad75432b531dd5d2c5d866544a2240c

commit d8cd72b77ad75432b531dd5d2c5d866544a2240c
Author: blundell <blundell@chromium.org>
Date: Tue Mar 28 07:18:38 2017

Initialize WakeLockContextHost lazily

Most WebContents instances won't need it.

BUG= 689403 
TEST=On Chrome on Android, go to about://flags and turn on "Experimental Web
Platform features." Restart Chrome. Go to
https://colinblundell.github.io/wake_lock.html and verify that the page
says "Wake Lock API found". Verify that if you wait ~40 seconds, the device
goes to sleep. Tap "Block sleep." Verify that if you wait longer than the
above period, the device does not go to sleep. Tap "Unblock sleep" and
verify again that the device goes to sleep after waiting.

Review-Url: https://codereview.chromium.org/2777143002
Cr-Commit-Position: refs/heads/master@{#460035}

[modify] https://crrev.com/d8cd72b77ad75432b531dd5d2c5d866544a2240c/content/browser/web_contents/web_contents_impl.cc

Components: Internals>Services>Device

Sign in to add a comment