New issue
Advanced search Search tips

Issue 600895 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocked on:
issue 596559

Blocking:
issue 593607



Sign in to add a comment

Move WakeLock content/ parts to third_party/WebKit

Project Member Reported by mcasas@chromium.org, Apr 6 2016

Issue description

Screen Wake Lock implements [1] by having:

1- idls and associated code in Source/modules/wake_lock

2- public/platform/modules/wake_lock/WebWakeLockClient.h
implemented in 
src/content/renderer/wake_lock/wake_lock_dispatcher.*

3 - a service running in browser at
src/content/browser/wake_lock/wake_lock_service_{context,impl}.*
and implementing wake_lock.mojom.

This bug as part of Onion Soup project tracks
removal of 2 by addressing the mojom channel 
directly from Source/modules/wake_lock.


Note that the service is implemented in the RenderFrameHost
(and not in the RenderProcessHost); this needs a bit
of Blink Glue:  http://crbug.com/596559  .

[1] https://www.w3.org/TR/wake-lock/


 
Cc: alogvi...@yandex-team.ru
Most of the code (if not all) was landed in 
 https://codereview.chromium.org/1084923002
 https://codereview.chromium.org/1393203004
so I'm adding the author. For reference, ITI 
is in [1], bug in http://crbug.com/257511

alogvinov@, is this reshuffling OK with you? 
How come this feature is still behind a flag?

https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Yujx917y6-Y
Yes it's OK with me. I think exposing the feature without the flag requires an intent to ship, am I right? The spec itself is still in the Public Working Draft status, so I'm not sure if the feature should be behind the flag or not.
#2: Yes, it'd need at ITS. Thanks for the clarification.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 12 2016

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

commit 689885b8927231762450bbc82c4e50ec9fd716eb
Author: mcasas <mcasas@chromium.org>
Date: Tue Apr 12 18:32:51 2016

[OnionSoup] Moving WakeLock Service to Blink (1/2): moving .mojom

This CL is part 1/2 of moving the WakeLockService renderer parts
from content/ to WebKit/. The complete transfer can be found in
https://crrev.com/1794553002, of which this is the .mojom moving
around parts. See bug for more info.

No new code is added.

BUG= 600895 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1878683002

Cr-Commit-Position: refs/heads/master@{#386747}

[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/content/browser/DEPS
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/content/browser/wake_lock/wake_lock_service_context.cc
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/content/browser/wake_lock/wake_lock_service_context.h
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/content/browser/wake_lock/wake_lock_service_impl.cc
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/content/browser/wake_lock/wake_lock_service_impl.h
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/content/common/BUILD.gn
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/content/content_browser.gypi
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/content/content_common_mojo_bindings.gyp
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/content/renderer/wake_lock/wake_lock_dispatcher.h
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/media/blink/BUILD.gn
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/third_party/WebKit/public/blink.gyp
[add] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/third_party/WebKit/public/platform/modules/wake_lock/OWNERS
[rename] https://crrev.com/689885b8927231762450bbc82c4e50ec9fd716eb/third_party/WebKit/public/platform/modules/wake_lock/wake_lock_service.mojom

Comment 5 Deleted

Comment 6 by mcasas@chromium.org, Apr 15 2016

Removed #5 since it pertained to another bug :P

https://crrev.com/aac4e4dcc28e183d72344c821bd01a9b55998bac
Cr-Commit-Position: refs/heads/master@{#387484}
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 16 2016

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

commit 162ec3044c357459bc0c572b210f4afe0a731018
Author: mcasas <mcasas@chromium.org>
Date: Sat Apr 16 01:53:29 2016

[OnionSoup] Moving WakeLock Service to Blink (2/2): removing WebWakeLockClient and WakeLockDispatcher

This CL is part 2/2 of moving the WakeLockService renderer parts
from content/ to WebKit/. The complete transfer can be found in
https://crrev.com/1794553002, of which this is the part removing
the WebWakeLockClient and its renderer implementation. See bug
for more info.

This CL removes the files
 - content/renderer/wake_lock/wake_lock_dispatcher.*
 - WebKit/Source/platform/wake_lock/WebWakeLockClient.h

since that functionality is folded into
  WebKit/public/platform/modules/wake_lock/ScreenWakeLock.*
of which some methods are made private.

modified to talk directly to the mojo service impl in
browser's render frame host.

The WebKit unittests ScreenWakeLockTest.cpp:
- is refactored to use a MockServiceRegistry and a
mojo MockWakeLockService.
- is NOT moved out of web/ because it needs a couple
of Test infra located there. This unit test should be
made into a (series of) LayoutTests, but that's beyond this CL.

BUG= 600895 

TESTS= in particular:
./out/Debug/webkit_unit_tests  --gtest_filter=*ScreenWakeLockTest.*
./out/Debug/content_browsertests --gtest_filter=WakeLockTest.*

Review URL: https://codereview.chromium.org/1883493003

Cr-Commit-Position: refs/heads/master@{#387786}

[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/content/content_renderer.gypi
[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/content/renderer/render_frame_impl.h
[delete] https://crrev.com/dd04070aa5257204200985a97148f7f8de4d508b/content/renderer/wake_lock/wake_lock_dispatcher.cc
[delete] https://crrev.com/dd04070aa5257204200985a97148f7f8de4d508b/content/renderer/wake_lock/wake_lock_dispatcher.h
[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp
[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.h
[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/third_party/WebKit/Source/web/tests/DEPS
[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/third_party/WebKit/Source/web/tests/ScreenWakeLockTest.cpp
[delete] https://crrev.com/dd04070aa5257204200985a97148f7f8de4d508b/third_party/WebKit/public/platform/modules/wake_lock/WebWakeLockClient.h
[modify] https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018/third_party/WebKit/public/web/WebFrameClient.h

Comment 8 by mcasas@chromium.org, Apr 18 2016

The bug is mostly closed, but had to leave 
ScreenWakeLockTest.cpp in web/tests due to 
dependencies. Now that we have mojo, I'd 
rather move those tests to LayoutTests.

Comment 9 by tkent@chromium.org, Jun 23 2016

Components: -Blink>Architecture Blink>Internals
Renaming Blink>Architecture to Blink>Internals

Status: Fixed (was: Assigned)
Marking this as Fixed and tracking the follow-up work to
nuke WebKit/Source/web/tests/ScreenWakeLockTest.cpp in
https://crbug.com/627511

Sign in to add a comment