Bindings: Separate WorldIdConstants into WorldTypes and WorldId

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 33 matching lines @@
 #include "wtf/StdLibExtras.h"
 #include <memory>
 
 namespace blink {
 
 class DOMObjectHolderBase {
   USING_FAST_MALLOC(DOMObjectHolderBase);
 
  public:
   DOMObjectHolderBase(v8::Isolate* isolate, v8::Local<v8::Value> wrapper)
-      : m_wrapper(isolate, wrapper), m_world(0) {}
+      : m_wrapper(isolate, wrapper), m_world(nullptr) {}
   virtual ~DOMObjectHolderBase() {}
 
   DOMWrapperWorld* world() const { return m_world; }
   void setWorld(DOMWrapperWorld* world) { m_world = world; }
   void setWeak(
       void (*callback)(const v8::WeakCallbackInfo<DOMObjectHolderBase>&)) {
     m_wrapper.setWeak(this, callback);
   }
 
  private:
@@ skipping 12 matching lines @@
  private:
   DOMObjectHolder(v8::Isolate* isolate, T* object, v8::Local<v8::Value> wrapper)
       : DOMObjectHolderBase(isolate, wrapper), m_object(object) {}
 
   Persistent<T> m_object;
 };
 
 unsigned DOMWrapperWorld::isolatedWorldCount = 0;
 
 PassRefPtr<DOMWrapperWorld> DOMWrapperWorld::create(v8::Isolate* isolate,
-                                                    int worldId) {
-  return adoptRef(new DOMWrapperWorld(isolate, worldId));
+                                                    WorldType worldType) {
+  DCHECK_NE(WorldType::Isolated, worldType);
+  return adoptRef(
+      new DOMWrapperWorld(isolate, worldType, getWorldIdForType(worldType)));
 }
 
-DOMWrapperWorld::DOMWrapperWorld(v8::Isolate* isolate, int worldId)
-    : m_worldId(worldId),
+DOMWrapperWorld::DOMWrapperWorld(v8::Isolate* isolate,
+                                 WorldType worldType,
+                                 int worldId)
+    : m_worldType(worldType),
+      m_worldId(worldId),
       m_domDataStore(
           WTF::wrapUnique(new DOMDataStore(isolate, isMainWorld()))) {
-  if (worldId == WorkerWorldId) {
+  if (isWorkerWorld())
     workerWorld() = this;
-  }
 }
 
 DOMWrapperWorld& DOMWrapperWorld::mainWorld() {
   ASSERT(isMainThread());
   DEFINE_STATIC_REF(
       DOMWrapperWorld, cachedMainWorld,
-      (DOMWrapperWorld::create(v8::Isolate::GetCurrent(), MainWorldId)));
+      (DOMWrapperWorld::create(v8::Isolate::GetCurrent(), WorldType::Main)));
   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,
@@ skipping 69 matching lines @@
 
 void DOMWrapperWorld::dispose() {
   m_domObjectHolders.clear();
   m_domDataStore.reset();
   if (isWorkerWorld())
     workerWorld() = nullptr;
 }
 
 #if DCHECK_IS_ON()
 static bool isIsolatedWorldId(int worldId) {
-  return MainWorldId < worldId && worldId < IsolatedWorldIdLimit;
+  return DOMWrapperWorld::MainWorldId < worldId &&
+         worldId < DOMWrapperWorld::IsolatedWorldIdLimit;
 }
 #endif
 
 PassRefPtr<DOMWrapperWorld> DOMWrapperWorld::ensureIsolatedWorld(
     v8::Isolate* isolate,
     int worldId) {
   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);
     return world.release();
   }
 
-  world = DOMWrapperWorld::create(isolate, worldId);
+  world = adoptRef(new DOMWrapperWorld(isolate, WorldType::Isolated, worldId));
   result.storedValue->value = world.get();
   isolatedWorldCount++;
   return world.release();
 }
 
 typedef HashMap<int, RefPtr<SecurityOrigin>> IsolatedWorldSecurityOriginMap;
 static IsolatedWorldSecurityOriginMap& isolatedWorldSecurityOrigins() {
   ASSERT(isMainThread());
   DEFINE_STATIC_LOCAL(IsolatedWorldSecurityOriginMap, map, ());
   return map;
@@ skipping 86 matching lines @@
   ASSERT(m_domObjectHolders.contains(holderBase));
   m_domObjectHolders.erase(holderBase);
 }
 
 void DOMWrapperWorld::weakCallbackForDOMObjectHolder(
     const v8::WeakCallbackInfo<DOMObjectHolderBase>& data) {
   DOMObjectHolderBase* holderBase = data.GetParameter();
   holderBase->world()->unregisterDOMObjectHolder(holderBase);
 }
 
+int DOMWrapperWorld::getWorldIdForType(WorldType worldType) {

[Yuki, 2017/03/08 14:07:51]

IIUC, this function is going to be used to get an identifier for a **new** world
being created, right?  This function should return a different ID for every call
(on the same thread).

If so, "get"WorldIdForType sounds weird to me because "get" implies that it
returns an existing one.  I'd prefer newWorldIdForType.

[nhiroki, 2017/03/09 03:40:28]

I'd prefer to rename this in my second patch that makes this return a different
id every time because in this patch this function always returns an existing id
for a type.

+  switch (worldType) {
+    case WorldType::Unknown:
+      return UnknownWorldId;

[Yuki, 2017/03/08 14:07:51]

I think "Unknown" is not a good name.

It seems that use cases are
  V8GCController::collectGarbage
  V8PerIsolateData::ensureScriptRegexpContext
  DataConsumerHandleTestUtil::Thread::initialize

Then, I'd like to give types for each one.
For example,
  GarbageCollector
  RegExp
  Testing  // Ideally, I'd like to deprecate this one.

Otherwise, I'd like to assign an unique ID for each at least, like
DocumentXMLTreeViewerWorldId.

[nhiroki, 2017/03/09 03:40:28]

Done.

+    case WorldType::Main:
+      return MainWorldId;
+    case WorldType::Isolated:
+      NOTREACHED();

[Yuki, 2017/03/08 14:07:51]

Could you add a comment about the reason why we don't hit this?

I've already read your reply, but I'd like to have it as a comment here.

[nhiroki, 2017/03/09 03:40:28]

Done.

+      return UnknownWorldId;
+    // 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.
+    case WorldType::Worker:
+      return WorkerWorldId;
+  }
+  NOTREACHED();
+  return UnknownWorldId;
+}
+
 }  // namespace blink