Skip to content

Commit

Permalink
wake lock: Drop WakeLockType in favor of V8WakeLockType.
Browse files Browse the repository at this point in the history
The (not so new anymore) bindings layer creates a V8WakeLockType class
corresponding to the WakeLockType enum in IDL that already takes care of
validating values received from script as well as providing a C++ enum
representation of the IDL enum.

This means we no longer need to define our own WakeLockType in C++ and
can instead just use V8WakeLockType (and V8WakeLockType::Enum) directly.
We no longer need to perform string comparisons for the enum values, and
the switch statements will automatically cause build failures if we add
new enum values without updating them accordingly.

This change has no user-visible effects.

Change-Id: Icd47f9a01c58bb9cacab59a3afc9f06f2b3f2922
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3532009
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
Auto-Submit: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/main@{#984250}
  • Loading branch information
rakuco authored and Chromium LUCI CQ committed Mar 23, 2022
1 parent 15e7318 commit cd3ae18
Show file tree
Hide file tree
Showing 13 changed files with 132 additions and 163 deletions.
46 changes: 22 additions & 24 deletions third_party/blink/renderer/modules/wake_lock/wake_lock.cc
Expand Up @@ -22,7 +22,6 @@
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
#include "third_party/blink/renderer/platform/wtf/functional.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"

namespace blink {

Expand Down Expand Up @@ -51,12 +50,13 @@ WakeLock::WakeLock(NavigatorBase& navigator)
permission_service_(navigator.GetExecutionContext()),
managers_{
MakeGarbageCollected<WakeLockManager>(navigator.GetExecutionContext(),
WakeLockType::kScreen),
MakeGarbageCollected<WakeLockManager>(navigator.GetExecutionContext(),
WakeLockType::kSystem)} {}
V8WakeLockType::Enum::kScreen),
MakeGarbageCollected<WakeLockManager>(
navigator.GetExecutionContext(),
V8WakeLockType::Enum::kSystem)} {}

ScriptPromise WakeLock::request(ScriptState* script_state,
const String& type,
V8WakeLockType type,
ExceptionState& exception_state) {
// https://w3c.github.io/screen-wake-lock/#the-request-method

Expand All @@ -72,7 +72,8 @@ ScriptPromise WakeLock::request(ScriptState* script_state,
auto* context = ExecutionContext::From(script_state);
DCHECK(context->IsWindow() || context->IsDedicatedWorkerGlobalScope());

if (type == "system" && !RuntimeEnabledFeatures::SystemWakeLockEnabled()) {
if (type == V8WakeLockType::Enum::kSystem &&
!RuntimeEnabledFeatures::SystemWakeLockEnabled()) {
exception_state.ThrowTypeError(
"The provided value 'system' is not a valid enum value of type "
"WakeLockType.");
Expand All @@ -86,7 +87,7 @@ ScriptPromise WakeLock::request(ScriptState* script_state,
// [N.B. Per https://github.com/w3c/webappsec-permissions-policy/issues/207
// there is no official support for workers in the Permissions Policy spec,
// but we can perform FP checks in workers in Blink]
if (type == "screen" &&
if (type == V8WakeLockType::Enum::kScreen &&
!context->IsFeatureEnabled(
mojom::blink::PermissionsPolicyFeature::kScreenWakeLock,
ReportOptions::kReportOnFailure)) {
Expand All @@ -104,7 +105,7 @@ ScriptPromise WakeLock::request(ScriptState* script_state,
// with a "NotAllowedError" DOMException and return promise.
// 3.2. If type is "screen", reject promise with a "NotAllowedError"
// DOMException, and return promise.
if (type == "screen") {
if (type == V8WakeLockType::Enum::kScreen) {
exception_state.ThrowDOMException(
DOMExceptionCode::kNotAllowedError,
"Screen locks cannot be requested from workers");
Expand All @@ -122,7 +123,8 @@ ScriptPromise WakeLock::request(ScriptState* script_state,
}
// 6. If the steps to determine the visibility state return hidden, return a
// promise rejected with "NotAllowedError" DOMException.
if (type == "screen" && !window->GetFrame()->GetPage()->IsPageVisible()) {
if (type == V8WakeLockType::Enum::kScreen &&
!window->GetFrame()->GetPage()->IsPageVisible()) {
exception_state.ThrowDOMException(DOMExceptionCode::kNotAllowedError,
"The requesting page is not visible");
return ScriptPromise();
Expand All @@ -133,37 +135,33 @@ ScriptPromise WakeLock::request(ScriptState* script_state,
auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
ScriptPromise promise = resolver->Promise();

WakeLockType wake_lock_type = ToWakeLockType(type);

switch (wake_lock_type) {
case WakeLockType::kScreen:
switch (type.AsEnum()) {
case V8WakeLockType::Enum::kScreen:
UseCounter::Count(context, WebFeature::kWakeLockAcquireScreenLock);
break;
case WakeLockType::kSystem:
case V8WakeLockType::Enum::kSystem:
UseCounter::Count(context, WebFeature::kWakeLockAcquireSystemLock);
break;
default:
NOTREACHED();
break;
}

// 8. Run the following steps in parallel:
DoRequest(wake_lock_type, resolver);
DoRequest(type.AsEnum(), resolver);

// 9. Return promise.
return promise;
}

void WakeLock::DoRequest(WakeLockType type, ScriptPromiseResolver* resolver) {
void WakeLock::DoRequest(V8WakeLockType::Enum type,
ScriptPromiseResolver* resolver) {
// https://w3c.github.io/screen-wake-lock/#the-request-method
// 8.1. Let state be the result of requesting permission to use
// "screen-wake-lock".
mojom::blink::PermissionName permission_name;
switch (type) {
case WakeLockType::kScreen:
case V8WakeLockType::Enum::kScreen:
permission_name = mojom::blink::PermissionName::SCREEN_WAKE_LOCK;
break;
case WakeLockType::kSystem:
case V8WakeLockType::Enum::kSystem:
permission_name = mojom::blink::PermissionName::SYSTEM_WAKE_LOCK;
break;
}
Expand All @@ -177,7 +175,7 @@ void WakeLock::DoRequest(WakeLockType type, ScriptPromiseResolver* resolver) {
type, WrapPersistent(resolver)));
}

void WakeLock::DidReceivePermissionResponse(WakeLockType type,
void WakeLock::DidReceivePermissionResponse(V8WakeLockType::Enum type,
ScriptPromiseResolver* resolver,
PermissionStatus status) {
// https://w3c.github.io/screen-wake-lock/#the-request-method
Expand Down Expand Up @@ -207,7 +205,7 @@ void WakeLock::DidReceivePermissionResponse(WakeLockType type,
}
// 8.3. Queue a global task on the screen wake lock task source given
// document's relevant global object to run these steps:
if (type == WakeLockType::kScreen &&
if (type == V8WakeLockType::Enum::kScreen &&
!(GetPage() && GetPage()->IsPageVisible())) {
// 8.3.1. If the steps to determine the visibility state return hidden,
// then:
Expand Down Expand Up @@ -246,7 +244,7 @@ void WakeLock::PageVisibilityChanged() {
// 1. For each lock in document.[[ActiveLocks]]["screen"]:
// 1.1. Run release a wake lock with document, lock, and "screen".
WakeLockManager* manager =
managers_[static_cast<size_t>(WakeLockType::kScreen)];
managers_[static_cast<size_t>(V8WakeLockType::Enum::kScreen)];
if (manager)
manager->ClearWakeLocks();
}
Expand Down
13 changes: 4 additions & 9 deletions third_party/blink/renderer/modules/wake_lock/wake_lock.h
Expand Up @@ -9,6 +9,7 @@
#include "base/gtest_prod_util.h"
#include "third_party/blink/public/mojom/permissions/permission.mojom-blink.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_wake_lock_type.h"
#include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
#include "third_party/blink/renderer/core/page/page_visibility_observer.h"
#include "third_party/blink/renderer/modules/modules_export.h"
Expand All @@ -19,12 +20,6 @@
#include "third_party/blink/renderer/platform/mojo/heap_mojo_wrapper_mode.h"
#include "third_party/blink/renderer/platform/supplementable.h"

namespace WTF {

class String;

} // namespace WTF

namespace blink {

class ExceptionState;
Expand All @@ -48,7 +43,7 @@ class MODULES_EXPORT WakeLock final : public ScriptWrappable,
explicit WakeLock(NavigatorBase&);

ScriptPromise request(ScriptState*,
const WTF::String& type,
V8WakeLockType type,
ExceptionState& exception_state);

void Trace(Visitor*) const override;
Expand All @@ -57,9 +52,9 @@ class MODULES_EXPORT WakeLock final : public ScriptWrappable,
// While this could be part of request() itself, having it as a separate
// function makes testing (which uses a custom ScriptPromiseResolver) a lot
// easier.
void DoRequest(WakeLockType, ScriptPromiseResolver*);
void DoRequest(V8WakeLockType::Enum, ScriptPromiseResolver*);

void DidReceivePermissionResponse(WakeLockType,
void DidReceivePermissionResponse(V8WakeLockType::Enum,
ScriptPromiseResolver*,
mojom::blink::PermissionStatus);

Expand Down
Expand Up @@ -16,7 +16,7 @@
namespace blink {

WakeLockManager::WakeLockManager(ExecutionContext* execution_context,
WakeLockType type)
V8WakeLockType::Enum type)
: wake_lock_(execution_context),
wake_lock_type_(type),
execution_context_(execution_context) {
Expand Down
Expand Up @@ -6,7 +6,8 @@
#define THIRD_PARTY_BLINK_RENDERER_MODULES_WAKE_LOCK_WAKE_LOCK_MANAGER_H_

#include "base/gtest_prod_util.h"
#include "services/device/public/mojom/wake_lock.mojom-blink-forward.h"
#include "services/device/public/mojom/wake_lock.mojom-blink.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_wake_lock_type.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/modules/wake_lock/wake_lock_type.h"
#include "third_party/blink/renderer/platform/heap/collection_support/heap_hash_set.h"
Expand All @@ -25,7 +26,7 @@ class WakeLockSentinel;
class MODULES_EXPORT WakeLockManager final
: public GarbageCollected<WakeLockManager> {
public:
WakeLockManager(ExecutionContext*, WakeLockType);
WakeLockManager(ExecutionContext*, V8WakeLockType::Enum);

void AcquireWakeLock(ScriptPromiseResolver*);
void ClearWakeLocks();
Expand All @@ -45,7 +46,7 @@ class MODULES_EXPORT WakeLockManager final
// An actual platform WakeLock. If bound, it means there is an active wake
// lock for a given type.
HeapMojoRemote<device::mojom::blink::WakeLock> wake_lock_;
WakeLockType wake_lock_type_;
V8WakeLockType::Enum wake_lock_type_;

// ExecutionContext from which we will connect to |wake_lock_service_|.
Member<ExecutionContext> execution_context_;
Expand Down
Expand Up @@ -19,7 +19,7 @@ namespace blink {
namespace {

WakeLockManager* MakeManager(WakeLockTestingContext& context,
WakeLockType type) {
V8WakeLockType::Enum type) {
return MakeGarbageCollected<WakeLockManager>(context.DomWindow(), type);
}

Expand All @@ -28,10 +28,10 @@ WakeLockManager* MakeManager(WakeLockTestingContext& context,
TEST(WakeLockManagerTest, AcquireWakeLock) {
MockWakeLockService wake_lock_service;
WakeLockTestingContext context(&wake_lock_service);
auto* manager = MakeManager(context, WakeLockType::kScreen);
auto* manager = MakeManager(context, V8WakeLockType::Enum::kScreen);

MockWakeLock& screen_lock =
wake_lock_service.get_wake_lock(WakeLockType::kScreen);
wake_lock_service.get_wake_lock(V8WakeLockType::Enum::kScreen);
EXPECT_FALSE(screen_lock.is_acquired());
EXPECT_FALSE(manager->wake_lock_.is_bound());

Expand Down Expand Up @@ -64,10 +64,10 @@ TEST(WakeLockManagerTest, AcquireWakeLock) {
TEST(WakeLockManagerTest, ReleaseAllWakeLocks) {
MockWakeLockService wake_lock_service;
WakeLockTestingContext context(&wake_lock_service);
auto* manager = MakeManager(context, WakeLockType::kScreen);
auto* manager = MakeManager(context, V8WakeLockType::Enum::kScreen);

MockWakeLock& screen_lock =
wake_lock_service.get_wake_lock(WakeLockType::kScreen);
wake_lock_service.get_wake_lock(V8WakeLockType::Enum::kScreen);

auto* resolver =
MakeGarbageCollected<ScriptPromiseResolver>(context.GetScriptState());
Expand All @@ -94,10 +94,10 @@ TEST(WakeLockManagerTest, ReleaseAllWakeLocks) {
TEST(WakeLockManagerTest, ReleaseOneWakeLock) {
MockWakeLockService wake_lock_service;
WakeLockTestingContext context(&wake_lock_service);
auto* manager = MakeManager(context, WakeLockType::kScreen);
auto* manager = MakeManager(context, V8WakeLockType::Enum::kScreen);

MockWakeLock& screen_lock =
wake_lock_service.get_wake_lock(WakeLockType::kScreen);
wake_lock_service.get_wake_lock(V8WakeLockType::Enum::kScreen);

auto* resolver1 =
MakeGarbageCollected<ScriptPromiseResolver>(context.GetScriptState());
Expand Down Expand Up @@ -130,10 +130,10 @@ TEST(WakeLockManagerTest, ReleaseOneWakeLock) {
TEST(WakeLockManagerTest, ClearEmptyWakeLockSentinelList) {
MockWakeLockService wake_lock_service;
WakeLockTestingContext context(&wake_lock_service);
auto* manager = MakeManager(context, WakeLockType::kSystem);
auto* manager = MakeManager(context, V8WakeLockType::Enum::kSystem);

MockWakeLock& system_lock =
wake_lock_service.get_wake_lock(WakeLockType::kSystem);
wake_lock_service.get_wake_lock(V8WakeLockType::Enum::kSystem);
EXPECT_FALSE(system_lock.is_acquired());

manager->ClearWakeLocks();
Expand All @@ -145,7 +145,7 @@ TEST(WakeLockManagerTest, ClearEmptyWakeLockSentinelList) {
TEST(WakeLockManagerTest, ClearWakeLocks) {
MockWakeLockService wake_lock_service;
WakeLockTestingContext context(&wake_lock_service);
auto* manager = MakeManager(context, WakeLockType::kSystem);
auto* manager = MakeManager(context, V8WakeLockType::Enum::kSystem);

auto* resolver1 =
MakeGarbageCollected<ScriptPromiseResolver>(context.GetScriptState());
Expand All @@ -155,7 +155,7 @@ TEST(WakeLockManagerTest, ClearWakeLocks) {
ScriptPromise promise2 = resolver2->Promise();

MockWakeLock& system_lock =
wake_lock_service.get_wake_lock(WakeLockType::kSystem);
wake_lock_service.get_wake_lock(V8WakeLockType::Enum::kSystem);

manager->AcquireWakeLock(resolver1);
manager->AcquireWakeLock(resolver2);
Expand All @@ -175,7 +175,7 @@ TEST(WakeLockManagerTest, ClearWakeLocks) {
TEST(WakeLockManagerTest, WakeLockConnectionError) {
MockWakeLockService wake_lock_service;
WakeLockTestingContext context(&wake_lock_service);
auto* manager = MakeManager(context, WakeLockType::kSystem);
auto* manager = MakeManager(context, V8WakeLockType::Enum::kSystem);

auto* resolver1 =
MakeGarbageCollected<ScriptPromiseResolver>(context.GetScriptState());
Expand All @@ -185,7 +185,7 @@ TEST(WakeLockManagerTest, WakeLockConnectionError) {
ScriptPromise promise2 = resolver2->Promise();

MockWakeLock& system_lock =
wake_lock_service.get_wake_lock(WakeLockType::kSystem);
wake_lock_service.get_wake_lock(V8WakeLockType::Enum::kSystem);

manager->AcquireWakeLock(resolver1);
manager->AcquireWakeLock(resolver2);
Expand Down
Expand Up @@ -14,7 +14,7 @@
namespace blink {

WakeLockSentinel::WakeLockSentinel(ScriptState* script_state,
WakeLockType type,
V8WakeLockType::Enum type,
WakeLockManager* manager)
: ExecutionContextLifecycleObserver(ExecutionContext::From(script_state)),
manager_(manager),
Expand All @@ -35,15 +35,10 @@ bool WakeLockSentinel::released() const {
return released_;
}

String WakeLockSentinel::type() const {
V8WakeLockType WakeLockSentinel::type() const {
// https://w3c.github.io/screen-wake-lock/#dom-wakelocksentinel-type
// The type attribute corresponds to the WakeLockSentinel's wake lock type.
switch (type_) {
case WakeLockType::kScreen:
return "screen";
case WakeLockType::kSystem:
return "system";
}
return V8WakeLockType(type_);
}

ExecutionContext* WakeLockSentinel::GetExecutionContext() const {
Expand Down
Expand Up @@ -8,12 +8,13 @@
#include "base/gtest_prod_util.h"
#include "third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_wake_lock_type.h"
#include "third_party/blink/renderer/core/dom/events/event_target.h"
#include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/modules/wake_lock/wake_lock_type.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
#include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"

namespace blink {

Expand All @@ -29,15 +30,15 @@ class MODULES_EXPORT WakeLockSentinel final

public:
WakeLockSentinel(ScriptState* script_state,
WakeLockType type,
V8WakeLockType::Enum type,
WakeLockManager* manager);
~WakeLockSentinel() override;

// Web-exposed interfaces
DEFINE_ATTRIBUTE_EVENT_LISTENER(release, kRelease)
ScriptPromise release(ScriptState*);
bool released() const;
String type() const;
V8WakeLockType type() const;

// EventTarget overrides.
ExecutionContext* GetExecutionContext() const override;
Expand All @@ -63,7 +64,7 @@ class MODULES_EXPORT WakeLockSentinel final

Member<WakeLockManager> manager_;
bool released_ = false;
const WakeLockType type_;
const V8WakeLockType::Enum type_;

FRIEND_TEST_ALL_PREFIXES(WakeLockSentinelTest, MultipleReleaseCalls);
};
Expand Down

0 comments on commit cd3ae18

Please sign in to comment.