Worklet: Lazily create PaintWorkletGlobalScopes

third_party/WebKit/Source/modules/csspaint/PaintWorklet.cpp

Index: third_party/WebKit/Source/modules/csspaint/PaintWorklet.cpp
diff --git a/third_party/WebKit/Source/modules/csspaint/PaintWorklet.cpp b/third_party/WebKit/Source/modules/csspaint/PaintWorklet.cpp
index 5631ede6909a80d6c746431a4abca10773172f2f..60f9ed2862125bae58e5bb53fa4d7f91c0a8e7b3 100644
--- a/third_party/WebKit/Source/modules/csspaint/PaintWorklet.cpp
+++ b/third_party/WebKit/Source/modules/csspaint/PaintWorklet.cpp
@@ -18,26 +18,57 @@ PaintWorklet* PaintWorklet::Create(LocalFrame* frame) {
 
 PaintWorklet::PaintWorklet(LocalFrame* frame)
     : MainThreadWorklet(frame),
-      global_scope_proxy_(
-          WTF::MakeUnique<PaintWorkletGlobalScopeProxy>(frame)) {}
+      pending_generator_registry_(new PaintWorkletPendingGeneratorRegistry) {}
 
 PaintWorklet::~PaintWorklet() = default;
 
-WorkletGlobalScopeProxy* PaintWorklet::GetWorkletGlobalScopeProxy() const {
-  return global_scope_proxy_.get();
-}
-
 CSSPaintDefinition* PaintWorklet::FindDefinition(const String& name) {
-  return global_scope_proxy_->FindDefinition(name);
+  if (GetNumberOfGlobalScopes() == 0)
+    return nullptr;
+
+  PaintWorkletGlobalScopeProxy* proxy =
+      PaintWorkletGlobalScopeProxy::From(FindAvailableGlobalScope());
+  return proxy->FindDefinition(name);
 }
 
 void PaintWorklet::AddPendingGenerator(const String& name,
                                        CSSPaintImageGeneratorImpl* generator) {
-  return global_scope_proxy_->AddPendingGenerator(name, generator);
+  if (GetNumberOfGlobalScopes() == 0) {
+    pending_generator_registry_->AddPendingGenerator(name, generator);
+    return;
+  }
+
+  PaintWorkletGlobalScopeProxy* proxy =
+      PaintWorkletGlobalScopeProxy::From(FindAvailableGlobalScope());
+  proxy->AddPendingGenerator(name, generator);
 }
 
 DEFINE_TRACE(PaintWorklet) {
+  visitor->Trace(pending_generator_registry_);
   MainThreadWorklet::Trace(visitor);
 }
 
+bool PaintWorklet::NeedsToCreateGlobalScope() {
+  // "The user agent must have, and select from at least two
+  // PaintWorkletGlobalScopes in the worklet's WorkletGlobalScopes list, unless
+  // the user agent is under memory constraints."
+  // https://drafts.css-houdini.org/css-paint-api-1/#drawing-an-image
+  // TODO(nhiroki): In the current impl, we create only one global scope. We
+  // should create at least two global scopes as the spec.
+  return !GetNumberOfGlobalScopes();
+}
+
+std::unique_ptr<WorkletGlobalScopeProxy> PaintWorklet::CreateGlobalScope() {
+  LocalFrame* frame = ToDocument(GetExecutionContext())->GetFrame();
+  auto proxy = WTF::MakeUnique<PaintWorkletGlobalScopeProxy>(frame);
+  if (GetNumberOfGlobalScopes() == 0) {
+    DCHECK(pending_generator_registry_);
+    proxy->global_scope()->SetPendingGeneratorRegistry(
+        pending_generator_registry_);

[haraken, 2017/05/25 01:04:39]

I think this is the point of this CL but would you help me understand why we
need to transfer the ownership to the global scope? Why can't we simply keep the
registry in PaintWorklet?

[nhiroki, 2017/05/25 06:23:23]

PaintWorklet can have multiple PaintWorkletGlobalScopes (it's not implemented
yet though). IIUC, a pending generator is a per-global-scope thing and should be
exclusively owned by one of the global scopes, so transferring the ownership
seems like a natural way here.

[haraken, 2017/05/25 06:28:53]

Does it mean that the pending generators can go to any arbitrary global scope?

Or your intention is to move all the pending generators to the first global
scope?

[nhiroki, 2017/05/25 07:24:08]

Hmmm..., I'm losing the confidence of this comment: "IIUC, a pending generator
is a per-global-scope thing and should be exclusively owned by one of the global
scopes,"

ikilpatrick@, should be pending generators sent to (shared among) all global
scopes? I'm now having a feeling that should be so because switching global
scopes (every 60 frames as you commented) seems to require they are also
identical in terms of the generators. Am I missing something?

haraken@, sorry for confusing you and thank you for pointing it out. I'm also
now learning design/implementation of PaintAPI :p

[nhiroki, 2017/05/25 07:37:44]

If this is correct, we may need to keep pending generators in PaintWorklet as
haraken@ pointed out, and associate them with definitions every time a global
scope is newly created.

[ikilpatrick, 2017/05/25 21:58:52]

Yeah we need to re-work this a little. We'll need to implement the following to
make this correct:

PaintWorklet
  Add document_paint_definitions_ member
(https://drafts.css-houdini.org/css-paint-api/#document-paint-definition)

PaintWorkletGlobalScope
  Add paint_definitions_ member
(https://drafts.css-houdini.org/css-paint-api/#paint-definition)

The PaintWorklet also needs to keep track of the pending_generator_registery_
which is being discussed here.

The way that it works, is when a new PaintWorkletGlobalScope is created it adds
the paint_definition to its own set, and then tries to add it to the
PaintWorklet (document paint definition) one. If they differ (e.g. different
input arguments), it gets "poisoned" and it can't be used anymore.

But yeah, this now belongs on the PaintWorklet.

[nhiroki, 2017/05/29 05:57:31]

Thank you for the clarification. I made PaintWorklet keep the pending generator
registry, and added some TODO/implementation comments about the defintion map.

haraken@, can you take another look?

+    pending_generator_registry_ = nullptr;
+  }
+  DCHECK(!pending_generator_registry_);
+  return std::move(proxy);
+}
+
 }  // namespace blink