Title: [135309] trunk/Source/WebCore
Revision
135309
Author
[email protected]
Date
2012-11-20 13:47:58 -0800 (Tue, 20 Nov 2012)

Log Message

Unreviewed, rolling out r135293.
http://trac.webkit.org/changeset/135293
https://bugs.webkit.org/show_bug.cgi?id=102832

This patch causes crash to some layout tests on chromium
(Requested by jianli on #webkit).

Patch by Sheriff Bot <[email protected]> on 2012-11-20

* bindings/v8/DOMDataStore.cpp:
(WebCore::DOMDataStore::current):
* bindings/v8/DOMWrapperWorld.cpp:
* bindings/v8/DOMWrapperWorld.h:
* bindings/v8/ScriptController.cpp:
(WebCore::ScriptController::existingWindowShell):
(WebCore::ScriptController::windowShell):
(WebCore::ScriptController::evaluateInIsolatedWorld):
(WebCore::ScriptController::currentWorldContext):
(WebCore::ScriptController::collectIsolatedContexts):
* bindings/v8/ScriptController.h:
(ScriptController):
* bindings/v8/V8Binding.h:
(WebCore::worldForEnteredContextIfIsolated):
* bindings/v8/V8DOMWindowShell.cpp:
(WebCore::V8DOMWindowShell::destroyIsolatedShell):
(WebCore):
(WebCore::isolatedContextWeakCallback):
(WebCore::V8DOMWindowShell::disposeContext):
(WebCore::V8DOMWindowShell::initializeIfNeeded):
* bindings/v8/V8DOMWindowShell.h:
(WebCore::V8DOMWindowShell::isolated):
(V8DOMWindowShell):
* bindings/v8/V8PerContextData.h:
* bindings/v8/WorldContextHandle.cpp:
(WebCore::WorldContextHandle::WorldContextHandle):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (135308 => 135309)


--- trunk/Source/WebCore/ChangeLog	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/ChangeLog	2012-11-20 21:47:58 UTC (rev 135309)
@@ -1,3 +1,39 @@
+2012-11-20  Sheriff Bot  <[email protected]>
+
+        Unreviewed, rolling out r135293.
+        http://trac.webkit.org/changeset/135293
+        https://bugs.webkit.org/show_bug.cgi?id=102832
+
+        This patch causes crash to some layout tests on chromium
+        (Requested by jianli on #webkit).
+
+        * bindings/v8/DOMDataStore.cpp:
+        (WebCore::DOMDataStore::current):
+        * bindings/v8/DOMWrapperWorld.cpp:
+        * bindings/v8/DOMWrapperWorld.h:
+        * bindings/v8/ScriptController.cpp:
+        (WebCore::ScriptController::existingWindowShell):
+        (WebCore::ScriptController::windowShell):
+        (WebCore::ScriptController::evaluateInIsolatedWorld):
+        (WebCore::ScriptController::currentWorldContext):
+        (WebCore::ScriptController::collectIsolatedContexts):
+        * bindings/v8/ScriptController.h:
+        (ScriptController):
+        * bindings/v8/V8Binding.h:
+        (WebCore::worldForEnteredContextIfIsolated):
+        * bindings/v8/V8DOMWindowShell.cpp:
+        (WebCore::V8DOMWindowShell::destroyIsolatedShell):
+        (WebCore):
+        (WebCore::isolatedContextWeakCallback):
+        (WebCore::V8DOMWindowShell::disposeContext):
+        (WebCore::V8DOMWindowShell::initializeIfNeeded):
+        * bindings/v8/V8DOMWindowShell.h:
+        (WebCore::V8DOMWindowShell::isolated):
+        (V8DOMWindowShell):
+        * bindings/v8/V8PerContextData.h:
+        * bindings/v8/WorldContextHandle.cpp:
+        (WebCore::WorldContextHandle::WorldContextHandle):
+
 2012-11-20  David Grogan  <[email protected]>
 
         IndexedDB: Remove legacy enum-based constants from IDL

Modified: trunk/Source/WebCore/bindings/v8/DOMDataStore.cpp (135308 => 135309)


--- trunk/Source/WebCore/bindings/v8/DOMDataStore.cpp	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/bindings/v8/DOMDataStore.cpp	2012-11-20 21:47:58 UTC (rev 135309)
@@ -57,9 +57,9 @@
     V8PerIsolateData* data = "" ? V8PerIsolateData::from(isolate) : V8PerIsolateData::current();
     if (UNLIKELY(!!data->domDataStore()))
         return data->domDataStore();
-    DOMWrapperWorld* isolatedWorld = DOMWrapperWorld::isolated(v8::Context::GetEntered());
-    if (UNLIKELY(!!isolatedWorld))
-        return isolatedWorld->isolatedWorldDOMDataStore();
+    V8DOMWindowShell* shell = V8DOMWindowShell::isolated(v8::Context::GetEntered());
+    if (UNLIKELY(!!shell))
+        return shell->world()->isolatedWorldDOMDataStore();
     return &mainWorldDOMDataStore;
 }
 

Modified: trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.cpp (135308 => 135309)


--- trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.cpp	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.cpp	2012-11-20 21:47:58 UTC (rev 135309)
@@ -64,27 +64,6 @@
     return cachedNormalWorld.get();
 }
 
-static void isolatedWorldWeakCallback(v8::Persistent<v8::Value> object, void* parameter)
-{
-    object.Dispose();
-    object.Clear();
-    static_cast<DOMWrapperWorld*>(parameter)->deref();
-}
-
-void DOMWrapperWorld::makeContextWeak(v8::Handle<v8::Context> context)
-{
-    ASSERT(isIsolatedWorld());
-    ASSERT(isolated(context) == this);
-    v8::Persistent<v8::Context>::New(context).MakeWeak(this, isolatedWorldWeakCallback);
-    // Matching deref is in weak callback.
-    this->ref();
-}
-
-void DOMWrapperWorld::setIsolatedWorldField(v8::Handle<v8::Context> context)
-{
-    context->SetAlignedPointerInEmbedderData(v8ContextIsolatedWorld, isMainWorld() ? 0 : this);
-}
-
 typedef HashMap<int, DOMWrapperWorld*> WorldMap;
 static WorldMap& isolatedWorldMap()
 {

Modified: trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h (135308 => 135309)


--- trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h	2012-11-20 21:47:58 UTC (rev 135309)
@@ -32,8 +32,6 @@
 #define DOMWrapperWorld_h
 
 #include "SecurityOrigin.h"
-#include "V8PerContextData.h"
-#include <v8.h>
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
@@ -55,14 +53,6 @@
     static bool isolatedWorldsExist() { return isolatedWorldCount; }
     static bool isIsolatedWorldId(int worldId) { return worldId != mainWorldId && worldId != uninitializedWorldId; }
     static void getAllWorlds(Vector<RefPtr<DOMWrapperWorld> >& worlds);
-
-    void makeContextWeak(v8::Handle<v8::Context>);
-    void setIsolatedWorldField(v8::Handle<v8::Context>);
-    static DOMWrapperWorld* isolated(v8::Handle<v8::Context> context)
-    {
-        return static_cast<DOMWrapperWorld*>(context->GetAlignedPointerFromEmbedderData(v8ContextIsolatedWorld));
-    }
-
     // Associates an isolated world (see above for description) with a security
     // origin. XMLHttpRequest instances used in that world will be considered
     // to come from that origin, not the frame's.

Modified: trunk/Source/WebCore/bindings/v8/ScriptController.cpp (135308 => 135309)


--- trunk/Source/WebCore/bindings/v8/ScriptController.cpp	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/bindings/v8/ScriptController.cpp	2012-11-20 21:47:58 UTC (rev 135309)
@@ -347,7 +347,7 @@
     IsolatedWorldMap::iterator iter = m_isolatedWorlds.find(world->worldId());
     if (iter == m_isolatedWorlds.end())
         return 0;
-    return iter->value->isContextInitialized() ? iter->value.get() : 0;
+    return iter->value->isContextInitialized() ? iter->value : 0;
 }
 
 V8DOMWindowShell* ScriptController::windowShell(DOMWrapperWorld* world)
@@ -360,11 +360,11 @@
     else {
         IsolatedWorldMap::iterator iter = m_isolatedWorlds.find(world->worldId());
         if (iter != m_isolatedWorlds.end())
-            shell = iter->value.get();
+            shell = iter->value;
         else {
             OwnPtr<V8DOMWindowShell> isolatedWorldShell = V8DOMWindowShell::create(m_frame, world);
             shell = isolatedWorldShell.get();
-            m_isolatedWorlds.set(world->worldId(), isolatedWorldShell.release());
+            m_isolatedWorlds.set(world->worldId(), isolatedWorldShell.leakPtr());
         }
     }
     if (!shell->isContextInitialized() && shell->initializeIfNeeded()) {
@@ -407,8 +407,9 @@
 
         // Mark temporary shell for weak destruction.
         if (worldID == DOMWrapperWorld::uninitializedWorldId) {
+            int actualWorldId = isolatedWorldShell->world()->worldId();
+            m_isolatedWorlds.remove(actualWorldId);
             isolatedWorldShell->destroyIsolatedShell();
-            m_isolatedWorlds.remove(world->worldId());
         }
 
         v8Results = evaluateHandleScope.Close(resultArray);
@@ -443,7 +444,7 @@
 {
     if (v8::Context::InContext()) {
         v8::Handle<v8::Context> context = v8::Context::GetEntered();
-        if (DOMWrapperWorld::isolated(context)) {
+        if (V8DOMWindowShell::isolated(context)) {
             if (m_frame == toFrameIfNotDetached(context))
                 return v8::Local<v8::Context>::New(context);
             return v8::Local<v8::Context>();
@@ -672,7 +673,7 @@
 {
     v8::HandleScope handleScope;
     for (IsolatedWorldMap::iterator it = m_isolatedWorlds.begin(); it != m_isolatedWorlds.end(); ++it) {
-        V8DOMWindowShell* isolatedWorldShell = it->value.get();
+        V8DOMWindowShell* isolatedWorldShell = it->value;
         SecurityOrigin* origin = isolatedWorldShell->world()->isolatedWorldSecurityOrigin();
         if (!origin)
             continue;

Modified: trunk/Source/WebCore/bindings/v8/ScriptController.h (135308 => 135309)


--- trunk/Source/WebCore/bindings/v8/ScriptController.h	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/bindings/v8/ScriptController.h	2012-11-20 21:47:58 UTC (rev 135309)
@@ -193,7 +193,10 @@
     static int contextDebugId(v8::Handle<v8::Context>);
 
 private:
-    typedef HashMap<int, OwnPtr<V8DOMWindowShell> > IsolatedWorldMap;
+    // Note: although the pointer is raw, the instance is kept alive by a strong
+    // reference to the v8 context it contains, which is not made weak until we
+    // call world->destroyIsolatedShell().
+    typedef HashMap<int, V8DOMWindowShell*> IsolatedWorldMap;
 
     void reset();
 
@@ -201,6 +204,10 @@
     const String* m_sourceURL;
 
     OwnPtr<V8DOMWindowShell> m_windowShell;
+
+    // The isolated worlds we are tracking for this frame. We hold them alive
+    // here so that they can be used again by future calls to
+    // evaluateInIsolatedWorld().
     IsolatedWorldMap m_isolatedWorlds;
 
     bool m_paused;

Modified: trunk/Source/WebCore/bindings/v8/V8Binding.h (135308 => 135309)


--- trunk/Source/WebCore/bindings/v8/V8Binding.h	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/bindings/v8/V8Binding.h	2012-11-20 21:47:58 UTC (rev 135309)
@@ -32,10 +32,10 @@
 #define V8Binding_h
 
 #include "BindingSecurity.h"
-#include "DOMWrapperWorld.h"
 #include "Document.h"
 #include "V8BindingMacros.h"
 #include "V8DOMConfiguration.h"
+#include "V8DOMWindowShell.h"
 #include "V8DOMWrapper.h"
 #include "V8HiddenPropertyName.h"
 #include "V8ObjectConstructor.h"
@@ -383,7 +383,10 @@
     {
         if (!v8::Context::InContext())
             return 0;
-        return DOMWrapperWorld::isolated(v8::Context::GetEntered());
+        V8DOMWindowShell* shell = V8DOMWindowShell::isolated(v8::Context::GetEntered());
+        if (!shell)
+            return 0;
+        return shell->world();
     }
 
     // If the current context causes out of memory, _javascript_ setting

Modified: trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp (135308 => 135309)


--- trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp	2012-11-20 21:47:58 UTC (rev 135309)
@@ -192,11 +192,18 @@
 
 void V8DOMWindowShell::destroyIsolatedShell()
 {
-    disposeContext();
+    disposeContext(true);
 }
 
-void V8DOMWindowShell::disposeContext()
+static void isolatedContextWeakCallback(v8::Persistent<v8::Value> object, void* parameter)
 {
+    // Handle will be disposed in delete.
+    delete static_cast<V8DOMWindowShell*>(parameter);
+}
+
+void V8DOMWindowShell::disposeContext(bool weak)
+{
+    ASSERT(!m_context.get().IsWeak());
     m_perContextData.clear();
 
     if (m_context.isEmpty())
@@ -204,18 +211,22 @@
 
     m_frame->loader()->client()->willReleaseScriptContext(m_context.get(), m_world->worldId());
 
-    if (m_world->isIsolatedWorld()) {
+    if (!weak)
+        m_context.clear();
+    else {
+        ASSERT(!m_world->isMainWorld());
         destroyGlobal();
-        m_world->makeContextWeak(m_context.get());
+        m_frame = 0;
+        m_context.get().MakeWeak(this, isolatedContextWeakCallback);
     }
 
-    m_context.clear();
-
     // It's likely that disposing the context has created a lot of
     // garbage. Notify V8 about this so it'll have a chance of cleaning
     // it up when idle.
-    bool isMainFrame = m_frame->page() && (m_frame->page()->mainFrame() == m_frame);
-    V8GCForContextDispose::instance().notifyContextDisposed(isMainFrame);
+    if (m_world->isMainWorld()) {
+        bool isMainFrame = m_frame->page() && (m_frame->page()->mainFrame() == m_frame);
+        V8GCForContextDispose::instance().notifyContextDisposed(isMainFrame);
+    }
 }
 
 void V8DOMWindowShell::destroyGlobal()
@@ -309,8 +320,6 @@
     v8::Local<v8::Context> context = v8::Local<v8::Context>::New(m_context.get());
     v8::Context::Scope contextScope(context);
 
-    m_world->setIsolatedWorldField(m_context.get());
-
     if (m_global.isEmpty()) {
         m_global.set(context->Global());
         if (m_global.isEmpty()) {
@@ -319,10 +328,13 @@
         }
     }
 
-    if (!isMainWorld) {
+    if (isMainWorld)
+        context->SetAlignedPointerInEmbedderData(v8ContextIsolatedWindowShell, 0);
+    else {
         V8DOMWindowShell* mainWindow = m_frame->script()->existingWindowShell(mainThreadNormalWorld());
         if (mainWindow && !mainWindow->context().IsEmpty())
             setInjectedScriptContextDebugId(m_context.get(), m_frame->script()->contextDebugId(mainWindow->context()));
+        context->SetAlignedPointerInEmbedderData(v8ContextIsolatedWindowShell, this);
     }
 
     m_perContextData = V8PerContextData::create(m_context.get());

Modified: trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.h (135308 => 135309)


--- trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.h	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.h	2012-11-20 21:47:58 UTC (rev 135309)
@@ -80,6 +80,11 @@
 
     void destroyGlobal();
 
+    static V8DOMWindowShell* isolated(v8::Handle<v8::Context> context)
+    {
+        return static_cast<V8DOMWindowShell*>(context->GetAlignedPointerFromEmbedderData(v8ContextIsolatedWindowShell));
+    }
+
     V8PerContextData* perContextData() { return m_perContextData.get(); }
     DOMWrapperWorld* world() { return m_world.get(); }
 
@@ -88,7 +93,7 @@
 private:
     V8DOMWindowShell(Frame*, PassRefPtr<DOMWrapperWorld>);
 
-    void disposeContext();
+    void disposeContext(bool weak = false);
 
     void setSecurityToken();
 

Modified: trunk/Source/WebCore/bindings/v8/V8PerContextData.h (135308 => 135309)


--- trunk/Source/WebCore/bindings/v8/V8PerContextData.h	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/bindings/v8/V8PerContextData.h	2012-11-20 21:47:58 UTC (rev 135309)
@@ -47,7 +47,7 @@
 enum V8ContextEmbedderDataField {
     v8ContextDebugIdIndex,
     v8ContextPerContextDataIndex,
-    v8ContextIsolatedWorld,
+    v8ContextIsolatedWindowShell,
     // Rather than adding more embedder data fields to v8::Context,
     // consider adding the data to V8PerContextData instead.
 };

Modified: trunk/Source/WebCore/bindings/v8/WorldContextHandle.cpp (135308 => 135309)


--- trunk/Source/WebCore/bindings/v8/WorldContextHandle.cpp	2012-11-20 21:10:23 UTC (rev 135308)
+++ trunk/Source/WebCore/bindings/v8/WorldContextHandle.cpp	2012-11-20 21:47:58 UTC (rev 135309)
@@ -34,6 +34,7 @@
 #include "ScriptController.h"
 #include "V8Binding.h"
 #include "V8DOMWindow.h"
+#include "V8DOMWindowShell.h"
 
 namespace WebCore {
 
@@ -51,7 +52,7 @@
             return;
         }
 #endif
-        if (DOMWrapperWorld::isolated(context)) {
+        if (V8DOMWindowShell::isolated(context)) {
             m_context = SharedPersistent<v8::Context>::create(context);
             return;
         }
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to