Bindings: Manage multiple DOMWrapperWorlds for worklets

third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 63 matching lines @@
     return WTF::wrapUnique(new DOMObjectHolder(isolate, object, wrapper));
   }
 
  private:
   DOMObjectHolder(v8::Isolate* isolate, T* object, v8::Local<v8::Value> wrapper)
       : DOMObjectHolderBase(isolate, wrapper), m_object(object) {}
 
   Persistent<T> m_object;
 };
 
+using WorldMap = HashMap<int, DOMWrapperWorld*>;
+
+static WorldMap& workerWorldMap() {
+  DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadSpecific<WorldMap>, map,
+                                  new ThreadSpecific<WorldMap>);
+  return *map;
+}
+
 unsigned DOMWrapperWorld::isolatedWorldCount = 0;
 
 PassRefPtr<DOMWrapperWorld> DOMWrapperWorld::create(v8::Isolate* isolate,
                                                     int worldId) {
   return adoptRef(new DOMWrapperWorld(isolate, worldId));
 }
 
 DOMWrapperWorld::DOMWrapperWorld(v8::Isolate* isolate, int worldId)
     : m_worldId(worldId),
       m_domDataStore(
           WTF::wrapUnique(new DOMDataStore(isolate, isMainWorld()))) {
   if (worldId == WorkerWorldId) {
-    workerWorld() = this;
+    WorldMap& map = workerWorldMap();
+    DCHECK(!map.contains(worldId));
+    map.insert(worldId, this);

[Yuki, 2017/03/07 10:17:16]

Does this support multiple worlds in a thread?

IIUC, worldId == m_worldId == WorkerWorldId == 1<<29 + 3, so we're always using
the same world id.

[nhiroki, 2017/03/07 10:45:14]

You're right. This doesn't make sense. Is it ok to reserve the identifier space
for worklets like IsolatedWorldIdLimit?

[nhiroki, 2017/03/07 10:51:40]

Or, how about separating WorldIdConstants into tags of world types and world
ids?

[Yuki, 2017/03/07 11:51:26]

I'm personally okay to introduce IsolatedWorldIdLimit, but we need to be super
careful not to break things.  IMHO, it has been a kind of fundamental assumption
that we only have a single world for worker.  It wouldn't be easy to change (but
I'm supportive).

haraken@, what do you think?

   }
 }
 
 DOMWrapperWorld& DOMWrapperWorld::mainWorld() {
   ASSERT(isMainThread());
   DEFINE_STATIC_REF(
       DOMWrapperWorld, cachedMainWorld,
       (DOMWrapperWorld::create(v8::Isolate::GetCurrent(), MainWorldId)));
   return *cachedMainWorld;
 }
 
-DOMWrapperWorld*& DOMWrapperWorld::workerWorld() {
-  DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadSpecific<DOMWrapperWorld*>, workerWorld,
-                                  new ThreadSpecific<DOMWrapperWorld*>);
-  return *workerWorld;
-}
-
 PassRefPtr<DOMWrapperWorld> DOMWrapperWorld::fromWorldId(v8::Isolate* isolate,
                                                          int worldId) {
   if (worldId == MainWorldId)
     return &mainWorld();
   return ensureIsolatedWorld(isolate, worldId);
 }
 
-typedef HashMap<int, DOMWrapperWorld*> WorldMap;
 static WorldMap& isolatedWorldMap() {
   ASSERT(isMainThread());
   DEFINE_STATIC_LOCAL(WorldMap, map, ());
   return map;
 }
 
 void DOMWrapperWorld::allWorldsInMainThread(
     Vector<RefPtr<DOMWrapperWorld>>& worlds) {
   ASSERT(isMainThread());
   worlds.push_back(&mainWorld());
-  WorldMap& isolatedWorlds = isolatedWorldMap();
-  for (WorldMap::iterator it = isolatedWorlds.begin();
-       it != isolatedWorlds.end(); ++it)
-    worlds.push_back(it->value);
+  for (auto worldById : workerWorldMap())
+    worlds.push_back(worldById.value);
+  for (auto worldById : isolatedWorldMap())
+    worlds.push_back(worldById.value);
 }
 
 void DOMWrapperWorld::markWrappersInAllWorlds(
     ScriptWrappable* scriptWrappable,
     const ScriptWrappableVisitor* visitor) {
-  // Handle marking in per-worker wrapper worlds.
-  if (!isMainThread()) {
-    DCHECK(ThreadState::current()->isolate());
-    DOMWrapperWorld* worker = workerWorld();
-    if (worker) {
-      DOMDataStore& dataStore = worker->domDataStore();
-      if (dataStore.containsWrapper(scriptWrappable)) {
-        dataStore.markWrapper(scriptWrappable);
-      }
-    }
-    return;
+  if (isMainThread())
+    scriptWrappable->markWrapper(visitor);
+
+  // Marking for the per-worker-or-worklet wrapper worlds. Worklets can be
+  // created on the main thread, so we iterate the worker worlds even if we're
+  // on the main thread.
+  DCHECK(ThreadState::current()->isolate());
+  WorldMap& workerWorlds = workerWorldMap();
+  for (auto& world : workerWorlds.values()) {
+    DOMDataStore& dataStore = world->domDataStore();
+    if (dataStore.containsWrapper(scriptWrappable))
+      dataStore.markWrapper(scriptWrappable);
   }
 
-  scriptWrappable->markWrapper(visitor);
+  // The isolated worlds should exist only on the main thread.
+  if (!isMainThread())
+    return;
+
+  // Marking for the isolated worlds.
   WorldMap& isolatedWorlds = isolatedWorldMap();
   for (auto& world : isolatedWorlds.values()) {
     DOMDataStore& dataStore = world->domDataStore();
-    if (dataStore.containsWrapper(scriptWrappable)) {
-      // Marking for the isolated worlds
+    if (dataStore.containsWrapper(scriptWrappable))
       dataStore.markWrapper(scriptWrappable);
-    }
   }
 }
 
 DOMWrapperWorld::~DOMWrapperWorld() {
   ASSERT(!isMainWorld());
 
   dispose();
 
   if (!isIsolatedWorld())
     return;
 
   WorldMap& map = isolatedWorldMap();
   WorldMap::iterator it = map.find(m_worldId);
   if (it == map.end()) {
     ASSERT_NOT_REACHED();
     return;
   }
   ASSERT(it->value == this);
 
   map.remove(it);
   isolatedWorldCount--;
 }
 
 void DOMWrapperWorld::dispose() {
   m_domObjectHolders.clear();
   m_domDataStore.reset();
-  if (isWorkerWorld())
-    workerWorld() = nullptr;
+  if (isWorkerWorld()) {
+    WorldMap& map = workerWorldMap();
+    DCHECK(map.contains(m_worldId));
+    map.remove(m_worldId);
+  }
 }
 
 #if DCHECK_IS_ON()
 static bool isIsolatedWorldId(int worldId) {
   return MainWorldId < worldId && worldId < IsolatedWorldIdLimit;
 }
 #endif
 
 PassRefPtr<DOMWrapperWorld> DOMWrapperWorld::ensureIsolatedWorld(
     v8::Isolate* isolate,
@@ skipping 109 matching lines @@
   m_domObjectHolders.erase(holderBase);
 }
 
 void DOMWrapperWorld::weakCallbackForDOMObjectHolder(
     const v8::WeakCallbackInfo<DOMObjectHolderBase>& data) {
   DOMObjectHolderBase* holderBase = data.GetParameter();
   holderBase->world()->unregisterDOMObjectHolder(holderBase);
 }
 
 }  // namespace blink