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

[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.*

Committed: https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018
Cr-Commit-Position: refs/heads/master@{#387786}

[mcasas, 2016-04-12 22:11:43]

Description was changed from

==========
[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

where that functionality is folded into
  WebKit/public/platform/modules/wake_lock/ScreenWakeLock.*

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

The WebKit unittests ScreenWakeLockTest.cpp is moved from web/
to modules/, and is refactored to use a MockServieRegistry and
a mojo MockWakeLockService.


BUG=600895
==========

to

==========
[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

where that functionality is folded into
  WebKit/public/platform/modules/wake_lock/ScreenWakeLock.*

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

The WebKit unittests ScreenWakeLockTest.cpp is moved from web/
to modules/, and is refactored to use a MockServieRegistry and
a mojo MockWakeLockService.


BUG=600895

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

[mcasas, 2016-04-12 22:12:10]

Description was changed from

==========
[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

where that functionality is folded into
  WebKit/public/platform/modules/wake_lock/ScreenWakeLock.*

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

The WebKit unittests ScreenWakeLockTest.cpp is moved from web/
to modules/, and is refactored to use a MockServieRegistry and
a mojo MockWakeLockService.


BUG=600895

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

to

==========
[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

where that functionality is folded into
  WebKit/public/platform/modules/wake_lock/ScreenWakeLock.*

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

The WebKit unittests ScreenWakeLockTest.cpp is moved from web/
to modules/, and is refactored to use a MockServieRegistry and
a mojo MockWakeLockService.

Also made some methods of ScreenWakeLock private.

BUG=600895

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

[mcasas, 2016-04-13 01:09:58]

Patchset #1 (id:1) has been deleted

[mcasas, 2016-04-13 01:10:10]

Patchset #1 (id:20001) has been deleted

[mcasas, 2016-04-14 20:23:33]

Patchset #1 (id:40001) has been deleted

[mcasas, 2016-04-14 20:23:54]

Patchset #1 (id:60001) has been deleted

[mcasas, 2016-04-14 20:53:43]

Description was changed from

==========
[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

where that functionality is folded into
  WebKit/public/platform/modules/wake_lock/ScreenWakeLock.*

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

The WebKit unittests ScreenWakeLockTest.cpp is moved from web/
to modules/, and is refactored to use a MockServieRegistry and
a mojo MockWakeLockService.

Also made some methods of ScreenWakeLock private.

BUG=600895

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

to

==========
[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.*
==========

[mcasas, 2016-04-14 21:50:58]

mcasas@chromium.org changed reviewers:
+ alogvinov@yandex-team.ru, esprehn@chromium.org

[mcasas, 2016-04-14 21:50:59]

esprehn@ PTAL at the general concept.

alogvinov@yandex-team.ru: PTAL at the specific
changes in ScreenWakeLock*, and also note the
OWNERS file I created with you and I.

[haraken, 2016-04-15 00:16:20]

Blink-Mojo interaction LGTM

[esprehn, 2016-04-15 22:38:12]

esprehn@chromium.org changed reviewers:
+ haraken@chromium.org

[esprehn, 2016-04-15 22:38:13]

lgtm w/ some nit, you might want to clean up the client terminology and use
references instead of null checking. I don't think ServiceRegistry can ever be
null.

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/wake_lock/OWNERS (right):

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/wake_lock/OWNERS:1: alogvinov@yandex-team.ru
who is this person? :)

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp (right):

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp:52: registry ?
new ScreenWakeLock(frame, registry) : nullptr);
hmm, why would you ever get a null registry?

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp:79:
ScreenWakeLock::ScreenWakeLock(LocalFrame& frame, ServiceRegistry* registry)
pass a reference

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp:85:
DCHECK(registry);
remove dcheck and just use a reference?

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp:106: void
ScreenWakeLock::notifyClient()
does this need a new name?

[mcasas, 2016-04-15 23:56:16]

Patchset #3 (id:120001) has been deleted

[mcasas, 2016-04-16 00:08:06]

Patchset #3 (id:140001) has been deleted

[mcasas, 2016-04-16 00:34:14]

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/wake_lock/OWNERS (right):

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/wake_lock/OWNERS:1: alogvinov@yandex-team.ru
On 2016/04/15 22:38:13, esprehn wrote:
> who is this person? :)

Removed as per offline discussion.

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp (right):

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp:52: registry ?
new ScreenWakeLock(frame, registry) : nullptr);
On 2016/04/15 22:38:13, esprehn wrote:
> hmm, why would you ever get a null registry?

See below.

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp:79:
ScreenWakeLock::ScreenWakeLock(LocalFrame& frame, ServiceRegistry* registry)
On 2016/04/15 22:38:13, esprehn wrote:
> pass a reference
See below.

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp:85:
DCHECK(registry);
On 2016/04/15 22:38:13, esprehn wrote:
> remove dcheck and just use a reference?

ServiceRegistry should not be null, but the caller
to Create() pull it out of |m_client| and that can be
nullptr [1]. See the patterns like:

    if (RuntimeEnabledFeatures::wakeLockEnabled())
        ScreenWakeLock::provideTo(*m_frame, m_client ?
m_client->serviceRegistry(): nullptr);

in there.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

https://codereview.chromium.org/1883493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp:106: void
ScreenWakeLock::notifyClient()
On 2016/04/15 22:38:13, esprehn wrote:
> does this need a new name?

Done.

[mcasas, 2016-04-16 00:35:47]

mcasas@chromium.org changed reviewers:
- alogvinov@yandex-team.ru

[mcasas, 2016-04-16 00:36:03]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2016-04-16 00:36:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/1883493003/#ps160001
(title: "esprehn@ comments")

[commit-bot: I haz the power, 2016-04-16 00:36:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1883493003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1883493003/160001

[commit-bot: I haz the power, 2016-04-16 01:53:40]

Description was changed from

==========
[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.*
==========

to

==========
[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.*
==========

[commit-bot: I haz the power, 2016-04-16 01:53:41]

Committed patchset #3 (id:160001)

[commit-bot: I haz the power, 2016-04-16 01:54:56]

Description was changed from

==========
[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.*
==========

to

==========
[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.*

Committed: https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018
Cr-Commit-Position: refs/heads/master@{#387786}
==========

[commit-bot: I haz the power, 2016-04-16 01:54:57]

Patchset 3 (id:??) landed as
https://crrev.com/162ec3044c357459bc0c572b210f4afe0a731018
Cr-Commit-Position: refs/heads/master@{#387786}