Index: third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp |
diff --git a/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp b/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp |
index 4593c3ef2285369cbe2c479171ad9ca9c709cf0f..467134012e77ec7f2812618078bf2e8d7e4a2775 100644 |
--- a/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp |
+++ b/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp |
@@ -40,6 +40,7 @@ |
#include "bindings/core/v8/WindowProxy.h" |
#include "bindings/core/v8/WrapperTypeInfo.h" |
#include "core/dom/ExecutionContext.h" |
+#include "wtf/Atomics.h" |
#include "wtf/HashTraits.h" |
#include "wtf/PtrUtil.h" |
#include "wtf/StdLibExtras.h" |
@@ -83,11 +84,25 @@ class DOMObjectHolder : public DOMObjectHolderBase { |
unsigned DOMWrapperWorld::s_numberOfNonMainWorldsInMainThread = 0; |
+using WorldMap = HashMap<int, DOMWrapperWorld*>; |
+ |
+static WorldMap& isolatedWorldMap() { |
+ DCHECK(isMainThread()); |
+ DEFINE_STATIC_LOCAL(WorldMap, map, ()); |
+ return map; |
+} |
+ |
+static WorldMap& worldMap() { |
+ DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadSpecific<WorldMap>, map, |
+ new ThreadSpecific<WorldMap>); |
+ return *map; |
+} |
+ |
PassRefPtr<DOMWrapperWorld> DOMWrapperWorld::create(v8::Isolate* isolate, |
WorldType worldType) { |
DCHECK_NE(WorldType::Isolated, worldType); |
- return adoptRef( |
- new DOMWrapperWorld(isolate, worldType, getWorldIdForType(worldType))); |
+ return adoptRef(new DOMWrapperWorld(isolate, worldType, |
+ generateWorldIdForType(worldType))); |
} |
DOMWrapperWorld::DOMWrapperWorld(v8::Isolate* isolate, |
@@ -97,8 +112,28 @@ DOMWrapperWorld::DOMWrapperWorld(v8::Isolate* isolate, |
m_worldId(worldId), |
m_domDataStore( |
WTF::wrapUnique(new DOMDataStore(isolate, isMainWorld()))) { |
- if (isWorkerWorld()) |
- workerWorld() = this; |
+ switch (worldType) { |
+ case WorldType::Main: |
+ // MainWorld is managed separately from worldMap() and isolatedWorldMap(). |
+ // See mainWorld(). |
+ break; |
+ case WorldType::Isolated: { |
+ DCHECK(isMainThread()); |
+ WorldMap& map = isolatedWorldMap(); |
+ DCHECK(!map.contains(worldId)); |
+ map.insert(worldId, this); |
+ break; |
+ } |
+ case WorldType::GarbageCollector: |
+ case WorldType::RegExp: |
+ case WorldType::Testing: |
+ case WorldType::Worker: { |
+ WorldMap& map = worldMap(); |
+ DCHECK(!map.contains(worldId)); |
+ map.insert(worldId, this); |
+ break; |
+ } |
peria
2017/03/14 07:10:49
Add
default: NOTREACHED();
to catch unexpected beh
nhiroki
2017/03/14 09:14:38
This unexpected behavior could happen only when st
peria
2017/03/14 09:52:46
Thank you for the comment. Totally agreed.
|
+ } |
if (worldId != WorldId::MainWorldId && isMainThread()) |
s_numberOfNonMainWorldsInMainThread++; |
} |
@@ -111,12 +146,6 @@ DOMWrapperWorld& DOMWrapperWorld::mainWorld() { |
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) |
@@ -124,47 +153,40 @@ PassRefPtr<DOMWrapperWorld> DOMWrapperWorld::fromWorldId(v8::Isolate* isolate, |
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 (DOMWrapperWorld* world : worldMap().values()) |
+ worlds.push_back(world); |
+ for (DOMWrapperWorld* world : isolatedWorldMap().values()) |
+ worlds.push_back(world); |
} |
void DOMWrapperWorld::markWrappersInAllWorlds( |
ScriptWrappable* scriptWrappable, |
const ScriptWrappableVisitor* visitor) { |
nhiroki
2017/03/14 06:25:07
Question: AFAICS, this function doesn't mark wrapp
peria
2017/03/14 07:10:49
scriptWrappable->markWrapper(visitor) does.
IIUC,
nhiroki
2017/03/14 09:14:39
Thank you for the clarification.
|
- // 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); |
peria
2017/03/14 07:10:49
can we put this under the if branch on #181?
Then
nhiroki
2017/03/14 09:14:38
Moved it back (I wrongly thought this is necessary
|
+ |
+ // Marking for worlds other than the main world and the isolated worlds. |
+ DCHECK(ThreadState::current()->isolate()); |
+ for (DOMWrapperWorld* world : worldMap().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); |
- } |
} |
} |
@@ -193,8 +215,7 @@ DOMWrapperWorld::~DOMWrapperWorld() { |
void DOMWrapperWorld::dispose() { |
m_domObjectHolders.clear(); |
m_domDataStore.reset(); |
- if (isWorkerWorld()) |
- workerWorld() = nullptr; |
+ worldMap().remove(m_worldId); |
} |
#if DCHECK_IS_ON() |
@@ -210,16 +231,14 @@ PassRefPtr<DOMWrapperWorld> DOMWrapperWorld::ensureIsolatedWorld( |
ASSERT(isIsolatedWorldId(worldId)); |
WorldMap& map = isolatedWorldMap(); |
- WorldMap::AddResult result = map.insert(worldId, nullptr); |
- RefPtr<DOMWrapperWorld> world = result.storedValue->value; |
- if (world) { |
- ASSERT(world->worldId() == worldId); |
+ auto it = map.find(worldId); |
+ if (it != map.end()) { |
+ RefPtr<DOMWrapperWorld> world = it->value; |
+ DCHECK_EQ(worldId, world->worldId()); |
return world.release(); |
} |
- world = adoptRef(new DOMWrapperWorld(isolate, WorldType::Isolated, worldId)); |
- result.storedValue->value = world.get(); |
- return world.release(); |
+ return adoptRef(new DOMWrapperWorld(isolate, WorldType::Isolated, worldId)); |
} |
typedef HashMap<int, RefPtr<SecurityOrigin>> IsolatedWorldSecurityOriginMap; |
@@ -323,7 +342,12 @@ void DOMWrapperWorld::weakCallbackForDOMObjectHolder( |
holderBase->world()->unregisterDOMObjectHolder(holderBase); |
} |
-int DOMWrapperWorld::getWorldIdForType(WorldType worldType) { |
+// The last used world ID. Must be used with atomic operations because this is |
peria
2017/03/14 07:10:49
code health: remove the first sentence. It explain
nhiroki
2017/03/14 09:14:39
Done.
|
+// accessed from multiple threads. |
+static int s_lastUsedWorldId = |
peria
2017/03/14 07:10:49
First ID to be used is UnspecifiedWorldIdStart, an
Yuki
2017/03/14 08:06:00
nit: Better to put this variable inside generateWo
nhiroki
2017/03/14 09:14:38
Replaced UnspecifiedWorldIdStart with IsolatedWorl
nhiroki
2017/03/14 09:14:39
Done.
|
+ DOMWrapperWorld::WorldId::UnspecifiedWorldIdStart; |
+ |
+int DOMWrapperWorld::generateWorldIdForType(WorldType worldType) { |
switch (worldType) { |
case WorldType::Main: |
return MainWorldId; |
@@ -333,17 +357,10 @@ int DOMWrapperWorld::getWorldIdForType(WorldType worldType) { |
NOTREACHED(); |
return InvalidWorldId; |
case WorldType::GarbageCollector: |
- return GarbageCollectorWorldId; |
case WorldType::RegExp: |
- return RegExpWorldId; |
case WorldType::Testing: |
- return TestingWorldId; |
- // Currently, WorldId for a worker/worklet is a fixed value, but this |
- // doesn't work when multiple worklets are created on a thread. |
- // TODO(nhiroki): Expand the identifier space for workers/worklets. |
- // (https://crbug.com/697622) |
case WorldType::Worker: |
- return WorkerWorldId; |
+ return WTF::atomicIncrement(&s_lastUsedWorldId); |
Yuki
2017/03/14 08:06:00
nit: I understand it's super unlikely, but should
nhiroki
2017/03/14 09:14:39
Done.
|
} |
NOTREACHED(); |
return InvalidWorldId; |