[animation-worklet] Implement registerAnimator in worklet scope

third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScope.cpp

Index: third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScope.cpp
diff --git a/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScope.cpp b/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScope.cpp
index 73ae82a83c28f1c50e97e1047b01d3be0cb28695..b872fc423469a9dd1e24f51b3d23cecbdcb6d1ef 100644
--- a/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScope.cpp
+++ b/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScope.cpp
@@ -6,6 +6,11 @@
 
 #include "platform/weborigin/SecurityOrigin.h"
 
+#include "bindings/core/v8/ExceptionState.h"
+#include "bindings/core/v8/V8BindingMacros.h"
+#include "bindings/core/v8/WorkerOrWorkletScriptController.h"
+#include "core/dom/ExceptionCode.h"
+
 namespace blink {
 
 AnimationWorkletGlobalScope* AnimationWorkletGlobalScope::create(
@@ -32,4 +37,87 @@ AnimationWorkletGlobalScope::AnimationWorkletGlobalScope(
 
 AnimationWorkletGlobalScope::~AnimationWorkletGlobalScope() {}
 
+DEFINE_TRACE(AnimationWorkletGlobalScope) {
+  visitor->trace(m_animatorDefinitions);
+  visitor->trace(m_animators);
+  ThreadedWorkletGlobalScope::trace(visitor);
+}
+
+void AnimationWorkletGlobalScope::dispose() {
+  // Clear animators and definitions to avoid reference cycle.
+  m_animatorDefinitions.clear();
+  m_animators.clear();
+  ThreadedWorkletGlobalScope::dispose();
+}
+
+void AnimationWorkletGlobalScope::registerAnimator(
+    const String& name,
+    const ScriptValue& ctorValue,
+    ExceptionState& exceptionState) {
+  if (m_animatorDefinitions.contains(name)) {
+    exceptionState.throwDOMException(
+        NotSupportedError,
+        "A class with name:'" + name + "' is already registered.");
+    return;
+  }
+
+  if (name.isEmpty()) {
+    exceptionState.throwTypeError("The empty string is not a valid name.");
+    return;
+  }
+
+  v8::Isolate* isolate = scriptController()->getScriptState()->isolate();
+  v8::Local<v8::Context> context = scriptController()->context();
+
+  DCHECK(ctorValue.v8Value()->IsFunction());
+  v8::Local<v8::Function> constructor =
+      v8::Local<v8::Function>::Cast(ctorValue.v8Value());

[flackr, 2017/03/13 19:04:51]

Shouldn't this be another exception state? What happens if you try to
registerAnimator with a non function?

[majidvp, 2017/03/13 22:07:53]

there is a test for this. The IDL defines is as a function which is checked
by binding generator code.

[flackr, 2017/03/14 18:30:29]

Acknowledged. Thanks.

+
+  v8::Local<v8::Value> prototypeValue;
+  if (!v8Call(constructor->Get(context, v8String(isolate, "prototype")),
+              prototypeValue))
+    return;
+
+  if (isUndefinedOrNull(prototypeValue)) {
+    exceptionState.throwTypeError(
+        "The 'prototype' object on the class does not exist.");
+    return;
+  }
+
+  if (!prototypeValue->IsObject()) {
+    exceptionState.throwTypeError(
+        "The 'prototype' property on the class is not an object.");
+    return;
+  }
+
+  v8::Local<v8::Object> prototype = v8::Local<v8::Object>::Cast(prototypeValue);
+
+  v8::Local<v8::Value> animateValue;
+  if (!v8Call(prototype->Get(context, v8String(isolate, "animate")),
+              animateValue))
+    return;
+
+  if (isUndefinedOrNull(animateValue)) {

[flackr, 2017/03/13 19:04:51]

Does it make sense to check the animate at registration time when it could be
changed at any point in time? We probably need to throw these errors when we try
to run the animator later if they don't exist anyways.

[majidvp, 2017/03/13 22:07:53]

Hmmm, you suggestion makes sense. Though this is the same pattern as used in 
Paint Worklet. @ikilpatrick, wdyt? should we check the |animate| function
when we first instantiate it. Also should we even keep a reference to it
rather than looking it up on JS object instance which means the function
can by changed.

[ikilpatrick, 2017/03/13 22:20:34]

We should do this here - firstly consistency with the rest of the platform, i.e.
custom-elements, paint, etc.

It prevents code like:
class Animator {
  get animate() { return Math.random() > 0.5 ? function() { } : 55; }
} 

If we allow this, you have to additionally answer a bunch of specification
questions like, if an animate function was invalid one frame, do we keep on
trying to call them as they may be valid the next frame? etc.

Better to fail early here.

+    exceptionState.throwTypeError(
+        "The 'animate' function on the prototype does not exist.");
+    return;
+  }
+
+  if (!animateValue->IsFunction()) {
+    exceptionState.throwTypeError(
+        "The 'animate' property on the prototype is not a function.");
+    return;
+  }
+
+  v8::Local<v8::Function> animate = v8::Local<v8::Function>::Cast(animateValue);
+
+  AnimatorDefinition* definition = AnimatorDefinition::create(
+      scriptController()->getScriptState(), constructor, animate);
+  m_animatorDefinitions.set(name, definition);
+
+  // Immediately instantiate and animator for the registered definition.

[flackr, 2017/03/13 19:04:51]

s/and animator/an animator

[majidvp, 2017/03/13 22:07:53]

Done.

+  // TODO(majidvp): Remove this once you add alternative way to instantiate
+  m_animators.push_back(definition->createInstance());
+}
+
 }  // namespace blink