Worklet: Lazily create PaintWorkletGlobalScopes

third_party/WebKit/Source/core/workers/MainThreadWorklet.h

Index: third_party/WebKit/Source/core/workers/MainThreadWorklet.h
diff --git a/third_party/WebKit/Source/core/workers/MainThreadWorklet.h b/third_party/WebKit/Source/core/workers/MainThreadWorklet.h
index 998ab298423c43e06d00262cea343ab3d212918f..50e9b416b7a184fcc4626b8d3b0af9340cc4042b 100644
--- a/third_party/WebKit/Source/core/workers/MainThreadWorklet.h
+++ b/third_party/WebKit/Source/core/workers/MainThreadWorklet.h
@@ -15,6 +15,7 @@ namespace blink {
 
 class LocalFrame;
 class ScriptPromiseResolver;
+class WorkletGlobalScopeProxy;
 
 // A MainThreadWorklet is a worklet that runs only on the main thread.
 // TODO(nhiroki): This is a temporary class to support module loading for main
@@ -35,11 +36,29 @@ class CORE_EXPORT MainThreadWorklet : public Worklet {
  protected:
   explicit MainThreadWorklet(LocalFrame*);
 
+  WTF::HashSet<std::unique_ptr<WorkletGlobalScopeProxy>>& GetProxies() {

[kouhei (in TOK), 2017/05/12 16:37:10]

I'd want to avoid this as much as possible.

Please create a collection wrapper class which has_a
WTF::HashSet<std::unique_ptr<WorkletGlobalScopeProxy>> and expose that.

[falken, 2017/05/15 05:14:48]

For my edification, what is "this" that you want to avoid?

[nhiroki, 2017/05/22 00:52:33]

I guess he'd like to avoid exposing a reference to an internal collection
(kouhei@, is this correct?)

By the way, I think such a wrapper class still needs to expose a reference or an
iterator to pick up an appropriate global scope in PaintWorklet::FindDefinition
etc. These proxies are accessed only from sub-classes of MainThreadWorklet or
tests, so making the proxies "protected" and removing GetProxies() would be a
simpler solution in this case. What do you think?

[kinuko, 2017/05/22 02:51:04]

Fwiw I think it's still possible to expose this kind of things with a bit
stricter, more explicit interface, e.g. const method for size and mutable
methods to insert new one / to return one proxy (if the required behavior here
is to return one of the two in alternative way intuitively it seems such logic
could be implemented in the wrapper to hide details?)

(Sounds also ok to expose this only to subclasses, but felt that it'd be nice to
clarify what interactions we want to have with that... my 2 cents)

[nhiroki, 2017/05/24 05:12:21]

Fair enough.
This is the list of known interactions for now:

- Get the number of global scopes (proxies)
- Insert a new global scope.
- Request all global scopes to fetch and evaluate module scripts.
- Request all global scopes to terminate themselves.
- Pick up one of global scopes to run a task.

Based on them, I introduced WorkletGlobalScopeManager in the patchset 10. How
does it look to you?

+    return global_scope_proxies_;
+  }
+
  private:
   // Worklet.
   void FetchAndInvokeScript(const KURL& module_url_record,
                             const WorkletOptions&,
                             ScriptPromiseResolver*) override;
+
+  // Returns true if there are no global scopes or additional global scopes are
+  // necessary. CreateGlobalScope() will be called in that case. Each worklet
+  // can define how to pool global scopes here.
+  virtual bool NeedsToCreateGlobalScope() = 0;
+  virtual std::unique_ptr<WorkletGlobalScopeProxy> CreateGlobalScope() = 0;
+
+  // "A Worklet has a list of the worklet's WorkletGlobalScopes. Initially this
+  // list is empty; it is populated when the user agent chooses to create its
+  // WorkletGlobalScope."
+  // https://drafts.css-houdini.org/worklets/#worklet-section
+  // TODO(nhiroki): Make (Paint)WorkletGlobalScopeProxy GC-managed object to
+  // avoid that GC graphs are disjointed (https://crbug.com/719775).
+  WTF::HashSet<std::unique_ptr<WorkletGlobalScopeProxy>> global_scope_proxies_;
 };
 
 }  // namespace blink