Changeset 125884 in webkit

Timestamp:

Aug 17, 2012 5:22:37 AM (12 years ago)

Author:

commit-queue@webkit.org

Message:

Refactor away IsolatedWorld
​https://bugs.webkit.org/show_bug.cgi?id=93971

Patch by Dan Carney <​dcarney@google.com> on 2012-08-17
Reviewed by Kentaro Hara.

Remove IsolatedWorld class as it was 1:1 with DOMWrapperWorld.
This update fixes a bad assertion which caused the previous
version to be rolled back.

No tests. No change in functionality.

• UseV8.cmake:
• WebCore.gypi:
• bindings/v8/DOMWrapperWorld.cpp:

(WebCore):
(WebCore::mainThreadNormalWorld):

• bindings/v8/DOMWrapperWorld.h:

(WebCore):
(DOMWrapperWorld):
(WebCore::DOMWrapperWorld::create):
(WebCore::DOMWrapperWorld::~DOMWrapperWorld):
(WebCore::DOMWrapperWorld::count):
(WebCore::DOMWrapperWorld::worldId):
(WebCore::DOMWrapperWorld::domDataStore):
(WebCore::DOMWrapperWorld::DOMWrapperWorld):

• bindings/v8/IsolatedWorld.cpp: Removed.
• bindings/v8/IsolatedWorld.h: Removed.
• bindings/v8/V8DOMWrapper.h:

(WebCore::V8DOMWrapper::getCachedWrapper):

• bindings/v8/V8IsolatedContext.cpp:

(WebCore::V8IsolatedContext::V8IsolatedContext):
(WebCore::V8IsolatedContext::destroy):

• bindings/v8/V8IsolatedContext.h:

(WebCore::V8IsolatedContext::getEntered):
(WebCore::V8IsolatedContext::world):
(V8IsolatedContext):

• bindings/v8/V8PerIsolateData.h:

(WebCore::V8PerIsolateData::registerDOMDataStore):
(WebCore::V8PerIsolateData::unregisterDOMDataStore):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• UseV8.cmake (modified) (1 diff)
• WebCore.gypi (modified) (1 diff)
• bindings/v8/DOMWrapperWorld.cpp (modified) (1 diff)
• bindings/v8/DOMWrapperWorld.h (modified) (2 diffs)
• bindings/v8/IsolatedWorld.cpp (deleted)
• bindings/v8/IsolatedWorld.h (deleted)
• bindings/v8/V8DOMWrapper.h (modified) (2 diffs)
• bindings/v8/V8IsolatedContext.cpp (modified) (4 diffs)
• bindings/v8/V8IsolatedContext.h (modified) (4 diffs)
• bindings/v8/V8PerIsolateData.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-08-17  Dan Carney  <dcarney@google.com>
+
+        Refactor away IsolatedWorld
+        https://bugs.webkit.org/show_bug.cgi?id=93971
+
+        Reviewed by Kentaro Hara.
+
+        Remove IsolatedWorld class as it was 1:1 with DOMWrapperWorld.
+        This update fixes a bad assertion which caused the previous
+        version to be rolled back.
+
+        No tests.  No change in functionality.
+
+        * UseV8.cmake:
+        * WebCore.gypi:
+        * bindings/v8/DOMWrapperWorld.cpp:
+        (WebCore):
+        (WebCore::mainThreadNormalWorld):
+        * bindings/v8/DOMWrapperWorld.h:
+        (WebCore):
+        (DOMWrapperWorld):
+        (WebCore::DOMWrapperWorld::create):
+        (WebCore::DOMWrapperWorld::~DOMWrapperWorld):
+        (WebCore::DOMWrapperWorld::count):
+        (WebCore::DOMWrapperWorld::worldId):
+        (WebCore::DOMWrapperWorld::domDataStore):
+        (WebCore::DOMWrapperWorld::DOMWrapperWorld):
+        * bindings/v8/IsolatedWorld.cpp: Removed.
+        * bindings/v8/IsolatedWorld.h: Removed.
+        * bindings/v8/V8DOMWrapper.h:
+        (WebCore::V8DOMWrapper::getCachedWrapper):
+        * bindings/v8/V8IsolatedContext.cpp:
+        (WebCore::V8IsolatedContext::V8IsolatedContext):
+        (WebCore::V8IsolatedContext::destroy):
+        * bindings/v8/V8IsolatedContext.h:
+        (WebCore::V8IsolatedContext::getEntered):
+        (WebCore::V8IsolatedContext::world):
+        (V8IsolatedContext):
+        * bindings/v8/V8PerIsolateData.h:
+        (WebCore::V8PerIsolateData::registerDOMDataStore):
+        (WebCore::V8PerIsolateData::unregisterDOMDataStore):
+
 2012-08-17  Kangil Han  <kangil.han@samsung.com>
 

trunk/Source/WebCore/UseV8.cmake

@@ -26 +26 @@
     bindings/v8/IDBBindingUtilities.cpp
     bindings/v8/IDBCustomBindings.cpp
-    bindings/v8/IsolatedWorld.cpp
     bindings/v8/Dictionary.cpp
     bindings/v8/PageScriptDebugServer.cpp

trunk/Source/WebCore/WebCore.gypi

@@ -2183 +2183 @@
             'bindings/v8/IDBCustomBindings.cpp',
             'bindings/v8/IntrusiveDOMWrapperMap.h',
-            'bindings/v8/IsolatedWorld.cpp',
-            'bindings/v8/IsolatedWorld.h',
             'bindings/v8/JavaScriptCallFrame.cpp',
             'bindings/v8/JavaScriptCallFrame.h',

trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.cpp

@@ -37 +37 @@
 namespace WebCore {
 
-DOMWrapperWorld::DOMWrapperWorld()
-{
-    // This class is pretty boring, huh?
-}
+int DOMWrapperWorld::isolatedWorldCount = 0;
 
 DOMWrapperWorld* mainThreadNormalWorld()
 {
     ASSERT(isMainThread());
-    DEFINE_STATIC_LOCAL(RefPtr<DOMWrapperWorld>, cachedNormalWorld, (DOMWrapperWorld::create()));
+    DEFINE_STATIC_LOCAL(RefPtr<DOMWrapperWorld>, cachedNormalWorld, (DOMWrapperWorld::create(DOMWrapperWorld::mainWorldId)));
     return cachedNormalWorld.get();
 }

trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h

@@ -32 +32 @@
 #define DOMWrapperWorld_h
 
+#include "DOMDataStore.h"
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
@@ -39 +40 @@
 
 // This class represent a collection of DOM wrappers for a specific world.
-// The base class is pretty boring because the wrappers are actually stored
-// statically in V8DOMMap and garbage collected by V8 itself.
 class DOMWrapperWorld : public RefCounted<DOMWrapperWorld> {
 public:
-    static PassRefPtr<DOMWrapperWorld> create() { return adoptRef(new DOMWrapperWorld()); }
-    virtual ~DOMWrapperWorld() {}
+    static const int mainWorldId = -1;
+    static PassRefPtr<DOMWrapperWorld> create(int worldId = mainWorldId) { return adoptRef(new DOMWrapperWorld(worldId)); }
+    ~DOMWrapperWorld()
+    {
+        if (m_worldId != mainWorldId)
+            isolatedWorldCount--;
+    }
+    static int count() { return isolatedWorldCount; }
 
-protected:
-    DOMWrapperWorld();
+    int worldId() const { return m_worldId; }
+    DOMDataStore* domDataStore() const { return m_domDataStore.getStore(); }
+
+private:
+    DOMWrapperWorld(int worldId)
+        : m_worldId(worldId)
+    {
+        if (m_worldId != mainWorldId)
+            isolatedWorldCount++;
+    }
+
+    // The backing store for the isolated world's DOM wrappers. This class
+    // doesn't have visibility into the wrappers. This handle simply helps
+    // manage their lifetime.
+    DOMDataStoreHandle m_domDataStore;
+
+    const int m_worldId;
+    static int isolatedWorldCount;
 };
 

trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h

@@ -34 +34 @@
 #include "DOMDataStore.h"
 #include "Event.h"
-#include "IsolatedWorld.h"
 #include "Node.h"
 #include "NodeFilter.h"
@@ -131 +130 @@
         {
             ASSERT(isMainThread());
-            if (LIKELY(!IsolatedWorld::count())) {
+            if (LIKELY(!DOMWrapperWorld::count())) {
                 v8::Persistent<v8::Object>* wrapper = node->wrapper();
                 if (LIKELY(!!wrapper))

trunk/Source/WebCore/bindings/v8/V8IsolatedContext.cpp

@@ -67 +67 @@
 
 V8IsolatedContext::V8IsolatedContext(V8Proxy* proxy, int extensionGroup, int worldId)
-    : m_world(IsolatedWorld::create(worldId)),
+    : m_world(DOMWrapperWorld::create(worldId)),
       m_frame(proxy->frame())
 {
@@ -76 +76 @@
 
     // FIXME: We should be creating a new V8DOMWindowShell here instead of riping out the context.
-    m_context = SharedPersistent<v8::Context>::create(proxy->windowShell()->createNewContext(v8::Handle<v8::Object>(), extensionGroup, m_world->id()));
+    m_context = SharedPersistent<v8::Context>::create(proxy->windowShell()->createNewContext(v8::Handle<v8::Object>(), extensionGroup, m_world->worldId()));
     if (m_context->get().IsEmpty())
         return;
@@ -102 +102 @@
     m_context->get()->UseDefaultSecurityToken();
 
-    m_frame->loader()->client()->didCreateScriptContext(context(), extensionGroup, m_world->id());
+    m_frame->loader()->client()->didCreateScriptContext(context(), extensionGroup, m_world->worldId());
 }
 
@@ -108 +108 @@
 {
     m_perContextData.clear();
-    m_frame->loader()->client()->willReleaseScriptContext(context(), m_world->id());
+    m_frame->loader()->client()->willReleaseScriptContext(context(), m_world->worldId());
     m_context->get().MakeWeak(this, &contextWeakReferenceCallback);
     m_frame = 0;

trunk/Source/WebCore/bindings/v8/V8IsolatedContext.h

@@ -32 +32 @@
 #define V8IsolatedContext_h
 
-#include "IsolatedWorld.h"
+#include "DOMWrapperWorld.h"
 #include "ScriptSourceCode.h" // for WebCore::ScriptSourceCode
 #include "SharedPersistent.h"
@@ -83 +83 @@
         // Until then, we optimize the common case of not having any isolated
         // worlds at all.
-        if (!IsolatedWorld::count())
+        if (!DOMWrapperWorld::count())
             return 0;
         if (!v8::Context::InContext())
@@ -93 +93 @@
     PassRefPtr<SharedPersistent<v8::Context> > sharedContext() { return m_context; }
 
-    IsolatedWorld* world() const { return m_world.get(); }
+    DOMWrapperWorld* world() const { return m_world.get(); }
 
     SecurityOrigin* securityOrigin() const { return m_securityOrigin.get(); }
@@ -116 +116 @@
     RefPtr<SharedPersistent<v8::Context> > m_context;
 
-    RefPtr<IsolatedWorld> m_world;
+    RefPtr<DOMWrapperWorld> m_world;
 
     RefPtr<SecurityOrigin> m_securityOrigin;

trunk/Source/WebCore/bindings/v8/V8PerIsolateData.h

@@ -91 +91 @@
     void registerDOMDataStore(DOMDataStore* domDataStore) 
     {
+        ASSERT(m_domDataList.find(domDataStore) == notFound);
         m_domDataList.append(domDataStore);
     }
@@ -96 +97 @@
     void unregisterDOMDataStore(DOMDataStore* domDataStore)
     {
-        ASSERT(m_domDataList.find(domDataStore));
+        ASSERT(m_domDataList.find(domDataStore) != notFound);
         m_domDataList.remove(m_domDataList.find(domDataStore));
     }