Hi, Colin, I met a problem here. See line 125 in file power_api.cc
(https://cs.chromium.org/chromium/src/extensions/browser/api/power/power_api.cc?rcl=8944638a34f6c6bdb393c0ee5dd05293e6421776&l=125)

It has a |power_save_blocker_| which is a unique_ptr,  then it creates a |new_blocker| which is also a unique_ptr. Afterwards, it swaps the two unique_ptrs as:
   power_save_blocker_.swap(new_blocker);
The comment says "do a swap to ensure that there isn't a brief period where power management is unblocked."

Now we use mojo, we have a |wakelock_| and a |new_wakelock|, but we know they have 2 separate mojo pipes, so even calling |new_wakelock->Request()| before |wakelock_->Cancel()|, it still cannot perfectly guarantee that "there isn't a brief period".

One solution is to add another mojo interface like |GetWakeLockAssociatedWithoutContext()|, then we have to add another "AssociatedBindingSet" in "wake_lock.cc", it will mess up the wake_lock.cc if there are both BindingSet and AssociatedBindingSet in it.

Another solution is totally convert the non-associated mojo interfaces into associated. Then we have to convert many clients.

I also feel that the "unexpected brief period" nearly never happen even there are two mojo pipes. Maybe we can just ignore this problem?

Could you please help to give some comments? Thanks very much!

Great find and analysis! Another option would be to add a ChangeType() method to WakeLock and have WakeLock do the same swap internally. What do you think of that option?

Ohhh! Great idea!!

WakeLock can serve multiple clients by AddClient(). I think we can add check in ChangeType() to guarantee that client is allowed to change the type only if there is no other client.

Yup, that makes sense to me.

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

commit 849ea0481b84658c0029c5b044d005b817ce9273
Author: Ke He <ke.he@intel.com>
Date: Sat Jun 24 03:27:51 2017

Convert //extensions/power_api to be client of wake lock interface.

Wake Lock is a Mojo interface that wraps PowerSaveBlocker. As part of the
creation of standalone Device Service, all browser-side clients of
PowerSaveBlocker should be converted to be clients of the Wake Lock Mojo
interface instead.

Adds the "ChangeType()" in WakeLock mojo interfaces to meet the scenario
of PowerAPI usage. Unittest of wake lock is modified accordingly.

BUG= 689421 

Change-Id: Ic73852d87bd32543cf959ca9957223da7bae4966
Reviewed-on: https://chromium-review.googlesource.com/539655
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Ke He <ke.he@intel.com>
Cr-Commit-Position: refs/heads/master@{#482135}
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/chrome/browser/chromeos/policy/power_policy_browsertest.cc
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/chromeos/dbus/fake_power_manager_client.cc
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/chromeos/dbus/fake_power_manager_client.h
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/content/browser/webrtc/webrtc_internals_unittest.cc
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/device/wake_lock/public/interfaces/wake_lock.mojom
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/device/wake_lock/public/interfaces/wake_lock_context.mojom
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/device/wake_lock/wake_lock.cc
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/device/wake_lock/wake_lock.h
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/device/wake_lock/wake_lock_for_testing.cc
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/device/wake_lock/wake_lock_for_testing.h
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/extensions/browser/BUILD.gn
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/extensions/browser/api/BUILD.gn
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/extensions/browser/api/DEPS
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/extensions/browser/api/power/BUILD.gn
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/extensions/browser/api/power/power_api.cc
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/extensions/browser/api/power/power_api.h
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/extensions/browser/api/power/power_api_unittest.cc
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/mojo/public/cpp/bindings/binding_set.h
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/services/device/wake_lock/wake_lock_unittest.cc
[modify] https://crrev.com/849ea0481b84658c0029c5b044d005b817ce9273/third_party/WebKit/Source/web/tests/ScreenWakeLockTest.cpp