Changeset 52847 in webkit

Timestamp:

Jan 5, 2010 7:09:24 PM (14 years ago)

Author:

abarth@webkit.org

Message:

2010-01-05 Adam Barth <​abarth@webkit.org>

Reviewed by Dimitri Glazkov.

[v8] Move V8DOMWindowShell to ScriptController
​https://bugs.webkit.org/show_bug.cgi?id=33232

This patch moves V8DOMWindow shell to ScriptController, where the
window shells live in JSC land. The next step will be to allow for
more than one window shell per script controller.

• bindings/v8/ScriptController.cpp: (WebCore::ScriptController::ScriptController): (WebCore::ScriptController::~ScriptController): (WebCore::ScriptController::updateSecurityOrigin): (WebCore::ScriptController::evaluateInIsolatedWorld): (WebCore::ScriptController::haveInterpreter): (WebCore::ScriptController::mainWorldWindowShell): (WebCore::ScriptController::clearWindowShell): (WebCore::ScriptController::clearForClose): (WebCore::ScriptController::destroyWindowShell): (WebCore::ScriptController::updateDocument):
• bindings/v8/ScriptController.h:
• bindings/v8/V8DOMWindowShell.cpp: (WebCore::V8DOMWindowShell::~V8DOMWindowShell):
• bindings/v8/V8DOMWindowShell.h: (WebCore::V8DOMWindowShell::localHandleForContext):
• bindings/v8/V8DOMWrapper.cpp: (WebCore::V8DOMWrapper::instantiateV8Object): (WebCore::V8DOMWrapper::convertDocumentToV8Object): (WebCore::V8DOMWrapper::convertNewNodeToV8Object):
• bindings/v8/V8IsolatedContext.cpp: (WebCore::V8IsolatedContext::V8IsolatedContext):
• bindings/v8/V8Proxy.cpp: (WebCore::V8Proxy::V8Proxy): (WebCore::V8Proxy::~V8Proxy): (WebCore::V8Proxy::handleOutOfMemory): (WebCore::V8Proxy::evaluateInIsolatedWorld): (WebCore::V8Proxy::setInjectedScriptContextDebugId): (WebCore::V8Proxy::context): (WebCore::V8Proxy::mainWorldContext): (WebCore::V8Proxy::currentContext): (WebCore::V8Proxy::setContextDebugId):
• bindings/v8/V8Proxy.h:
• bindings/v8/WorldContextHandle.cpp: (WebCore::WorldContextHandle::adjustedContext):

2010-01-05 Adam Barth <​abarth@webkit.org>

Reviewed by Dimitri Glazkov.

[v8] Move V8DOMWindowShell to ScriptController
​https://bugs.webkit.org/show_bug.cgi?id=33232

Update FrameLoaderClientImpl.cpp to call clearForClose on
ScriptController instead of on V8Proxy.

• src/FrameLoaderClientImpl.cpp: (WebKit::FrameLoaderClientImpl::detachedFromParent3):

Location:

trunk

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/v8/ScriptController.cpp (modified) (7 diffs)
• WebCore/bindings/v8/ScriptController.h (modified) (3 diffs)
• WebCore/bindings/v8/V8DOMWindowShell.cpp (modified) (1 diff)
• WebCore/bindings/v8/V8DOMWindowShell.h (modified) (2 diffs)
• WebCore/bindings/v8/V8DOMWrapper.cpp (modified) (3 diffs)
• WebCore/bindings/v8/V8IsolatedContext.cpp (modified) (2 diffs)
• WebCore/bindings/v8/V8Proxy.cpp (modified) (10 diffs)
• WebCore/bindings/v8/V8Proxy.h (modified) (4 diffs)
• WebCore/bindings/v8/WorldContextHandle.cpp (modified) (2 diffs)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/src/FrameLoaderClientImpl.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-01-05  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        [v8] Move V8DOMWindowShell to ScriptController
+        https://bugs.webkit.org/show_bug.cgi?id=33232
+
+        This patch moves V8DOMWindow shell to ScriptController, where the
+        window shells live in JSC land.  The next step will be to allow for
+        more than one window shell per script controller.
+
+        * bindings/v8/ScriptController.cpp:
+        (WebCore::ScriptController::ScriptController):
+        (WebCore::ScriptController::~ScriptController):
+        (WebCore::ScriptController::updateSecurityOrigin):
+        (WebCore::ScriptController::evaluateInIsolatedWorld):
+        (WebCore::ScriptController::haveInterpreter):
+        (WebCore::ScriptController::mainWorldWindowShell):
+        (WebCore::ScriptController::clearWindowShell):
+        (WebCore::ScriptController::clearForClose):
+        (WebCore::ScriptController::destroyWindowShell):
+        (WebCore::ScriptController::updateDocument):
+        * bindings/v8/ScriptController.h:
+        * bindings/v8/V8DOMWindowShell.cpp:
+        (WebCore::V8DOMWindowShell::~V8DOMWindowShell):
+        * bindings/v8/V8DOMWindowShell.h:
+        (WebCore::V8DOMWindowShell::localHandleForContext):
+        * bindings/v8/V8DOMWrapper.cpp:
+        (WebCore::V8DOMWrapper::instantiateV8Object):
+        (WebCore::V8DOMWrapper::convertDocumentToV8Object):
+        (WebCore::V8DOMWrapper::convertNewNodeToV8Object):
+        * bindings/v8/V8IsolatedContext.cpp:
+        (WebCore::V8IsolatedContext::V8IsolatedContext):
+        * bindings/v8/V8Proxy.cpp:
+        (WebCore::V8Proxy::V8Proxy):
+        (WebCore::V8Proxy::~V8Proxy):
+        (WebCore::V8Proxy::handleOutOfMemory):
+        (WebCore::V8Proxy::evaluateInIsolatedWorld):
+        (WebCore::V8Proxy::setInjectedScriptContextDebugId):
+        (WebCore::V8Proxy::context):
+        (WebCore::V8Proxy::mainWorldContext):
+        (WebCore::V8Proxy::currentContext):
+        (WebCore::V8Proxy::setContextDebugId):
+        * bindings/v8/V8Proxy.h:
+        * bindings/v8/WorldContextHandle.cpp:
+        (WebCore::WorldContextHandle::adjustedContext):
+
 2010-01-05  Kent Tamura  <tkent@chromium.org>
 

trunk/WebCore/bindings/v8/ScriptController.cpp

@@ -107 +107 @@
     , m_paused(false)
     , m_proxy(new V8Proxy(frame))
+    , m_windowShell(V8DOMWindowShell::create(frame))
 #if ENABLE(NETSCAPE_PLUGIN_API)
     , m_windowScriptNPObject(0)
@@ -117 +118 @@
 {
     m_proxy->disconnectFrame();
+    m_windowShell.clear();
 }
 
@@ -141 +143 @@
 void ScriptController::updateSecurityOrigin()
 {
-    m_proxy->windowShell()->updateSecurityOrigin();
+    m_windowShell->updateSecurityOrigin();
 }
 
@@ -206 +208 @@
 void ScriptController::evaluateInIsolatedWorld(unsigned worldID, const Vector<ScriptSourceCode>& sources)
 {
+    // FIXME: This will need to get reorganized once we have a windowShell for the isolated world.
+    m_windowShell->initContextIfNeeded();
     m_proxy->evaluateInIsolatedWorld(worldID, sources, 0);
 }
@@ -292 +296 @@
 bool ScriptController::haveInterpreter() const
 {
-    return m_proxy->windowShell()->isContextInitialized();
+    return m_windowShell->isContextInitialized();
 }
 
@@ -436 +440 @@
 }
 
+V8DOMWindowShell* ScriptController::mainWorldWindowShell() const
+{
+    m_windowShell->initContextIfNeeded();
+    return m_windowShell.get();
+}
 
 void ScriptController::clearWindowShell()
 {
     m_mainWorldScriptState.clear();
+    m_proxy->resetIsolatedWorlds();
 
     // V8 binding expects ScriptController::clearWindowShell only be called
-    // when a frame is loading a new page. V8Proxy::clearForNavigation
+    // when a frame is loading a new page. V8DOMWindowShell::clearForNavigation
     // creates a new context for the new page.
-    m_proxy->clearForNavigation();
+    m_windowShell->clearForNavigation();
+}
+
+void ScriptController::clearForClose()
+{
+    m_windowShell->clearForClose();
+}
+
+void ScriptController::destroyWindowShell()
+{
+    m_windowShell->clearForClose();
+    m_windowShell->destroyGlobal();
 }
 
@@ -454 +475 @@
 void ScriptController::updateDocument()
 {
-    m_proxy->windowShell()->updateDocument();
+    m_windowShell->updateDocument();
 }
 

trunk/WebCore/bindings/v8/ScriptController.h

@@ -64 +64 @@
     V8Proxy* proxy() { return m_proxy.get(); }
 
+    V8DOMWindowShell* mainWorldWindowShell() const;
+
     ScriptValue executeScript(const ScriptSourceCode&);
     ScriptValue executeScript(const String& script, bool forceUserGesture = false);
@@ -154 +156 @@
     void updateDocument();
 
+    void clearForClose();
+
+    // This is very destructive (e.g., out of memory).
+    void destroyWindowShell();
+
     void updateSecurityOrigin();
     void clearScriptObjects();
@@ -183 +190 @@
     bool m_paused;
 
+    // FIXME: V8Proxy should eventually be removed.
     OwnPtr<V8Proxy> m_proxy;
+
+    // For the moment, we have one of these.  Soon we will have one per DOMWrapperWorld.
+    RefPtr<V8DOMWindowShell> m_windowShell;
+
     typedef HashMap<Widget*, NPObject*> PluginObjectMap;
 

trunk/WebCore/bindings/v8/V8DOMWindowShell.cpp

@@ -99 +99 @@
     : m_frame(frame)
 {
+}
+
+V8DOMWindowShell::~V8DOMWindowShell()
+{
+    clearForClose();
+    destroyGlobal();
 }
 

trunk/WebCore/bindings/v8/V8DOMWindowShell.h

@@ -47 +47 @@
     static PassRefPtr<V8DOMWindowShell> create(Frame*);
 
+    virtual ~V8DOMWindowShell();
+
     v8::Handle<v8::Context> context() const { return m_context; }
+
+    // This method is slower than |context()|, but sometimes we need local handles.
+    v8::Local<v8::Context> localHandleForContext() const { return v8::Local<v8::Context>::New(m_context); }
 
     // Update document object of the frame.
@@ -66 +71 @@
     void clearForNavigation();
     void clearForClose();
-
     void destroyGlobal();
 

trunk/WebCore/bindings/v8/V8DOMWrapper.cpp

@@ -803 +803 @@
     if (proxy)
         // FIXME: Fix this to work properly with isolated worlds (see above).
-        instance = proxy->windowShell()->createWrapperFromCache(descriptorType);
+        instance = proxy->frame()->script()->mainWorldWindowShell()->createWrapperFromCache(descriptorType);
     else {
         v8::Local<v8::Function> function = getTemplate(descriptorType)->GetFunction();
@@ -1205 +1205 @@
     // checking if the node already has a wrapper.
     V8Proxy* proxy = V8Proxy::retrieve(document->frame());
-    if (proxy)
-        proxy->windowShell()->initContextIfNeeded();
+    if (proxy) {
+        // This code looks wacky:
+        // 1) The test for |proxy| above is really a test whether JavaScript
+        //    is enabled in this frame.  I have no idea if that's needed.
+        // 2) Calling initContextIfNeeded here is redundant becase just
+        //    grabbing mainWorldWindowShell() already calls initContextIfNeeded.
+        // FIXME: I bet we can clean this up a bit!
+        proxy->frame()->script()->mainWorldWindowShell()->initContextIfNeeded();
+    }
 
     DOMWrapperMap<Node>& domNodeMap = getDOMNodeMap();
@@ -1295 +1302 @@
 
     if (isDocument) {
-        if (proxy)
-            proxy->windowShell()->updateDocumentWrapper(result);
+        if (proxy) {
+            // FIXME: Don't we need to do this for every world?
+            proxy->frame()->script()->mainWorldWindowShell()->updateDocumentWrapper(result);
+        }
 
         if (type == V8ClassIndex::HTMLDOCUMENT) {

trunk/WebCore/bindings/v8/V8IsolatedContext.cpp

@@ -54 +54 @@
     : m_world(IsolatedWorld::create())
 {
+    // FIXME: None of this code belongs here! Basically, ScriptController
+    // should create a new DOMWrapperWorld and a new windowShell and then
+    // evaluate the script.
+
+    V8DOMWindowShell* mainWorldWindowShell = proxy->frame()->script()->mainWorldWindowShell();
+
     v8::HandleScope scope;
     // 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_context = SharedPersistent<v8::Context>::create(mainWorldWindowShell->createNewContext(v8::Handle<v8::Object>(), extensionGroup));
     if (m_context->get().IsEmpty())
         return;
@@ -67 +73 @@
     V8DOMWindowShell::installHiddenObjectPrototype(m_context->get());
     // FIXME: This will go away once we have a windowShell for the isolated world.
-    proxy->windowShell()->installDOMWindow(m_context->get(), proxy->frame()->domWindow());
+    mainWorldWindowShell->installDOMWindow(m_context->get(), proxy->frame()->domWindow());
 
     // Using the default security token means that the canAccess is always

trunk/WebCore/bindings/v8/V8Proxy.cpp

@@ -214 +214 @@
 V8Proxy::V8Proxy(Frame* frame)
     : m_frame(frame)
-    , m_windowShell(V8DOMWindowShell::create(frame))
     , m_inlineCode(false)
     , m_timerCallback(false)
@@ -223 +222 @@
 V8Proxy::~V8Proxy()
 {
-    clearForClose();
-    windowShell()->destroyGlobal();
+    resetIsolatedWorlds();
 }
 
@@ -247 +245 @@
     Frame* frame = V8Proxy::retrieveFrame(context);
 
-    V8Proxy* proxy = V8Proxy::retrieve(frame);
-    if (proxy) {
-        // Clean m_context, and event handlers.
-        proxy->clearForClose();
-
-        proxy->windowShell()->destroyGlobal();
-    }
+    frame->script()->destroyWindowShell();
 
     ChromiumBridge::notifyJSOutOfMemory(frame);
@@ -267 +259 @@
 void V8Proxy::evaluateInIsolatedWorld(int worldID, const Vector<ScriptSourceCode>& sources, int extensionGroup)
 {
-    // FIXME: This will need to get reorganized once we have a windowShell for the isolated world.
-    windowShell()->initContextIfNeeded();
-
     v8::HandleScope handleScope;
     V8IsolatedContext* isolatedContext = 0;
@@ -315 +304 @@
     // Setup context id for JS debugger.
     v8::Context::Scope contextScope(targetContext);
-    v8::Handle<v8::Context> context = windowShell()->context();
+    v8::Handle<v8::Context> context = m_frame->script()->mainWorldWindowShell()->localHandleForContext();
     if (context.IsEmpty())
         return false;
@@ -583 +572 @@
     }
     m_isolatedWorlds.clear();
-}
-
-void V8Proxy::clearForClose()
-{
-    resetIsolatedWorlds();
-    windowShell()->clearForClose();
-}
-
-void V8Proxy::clearForNavigation()
-{
-    resetIsolatedWorlds();
-    windowShell()->clearForNavigation();
 }
 
@@ -656 +633 @@
 v8::Local<v8::Context> V8Proxy::context(Frame* frame)
 {
+    // FIXME: Move this function to ScriptController.
     v8::Local<v8::Context> context = V8Proxy::mainWorldContext(frame);
     if (context.IsEmpty())
@@ -671 +649 @@
 v8::Local<v8::Context> V8Proxy::context()
 {
+    // FIXME: Move this function to ScriptController.
     if (V8IsolatedContext* isolatedContext = V8IsolatedContext::getEntered()) {
         RefPtr<SharedPersistent<v8::Context> > context = isolatedContext->sharedContext();
@@ -677 +656 @@
         return v8::Local<v8::Context>::New(context->get());
     }
-    return mainWorldContext();
-}
-
-v8::Local<v8::Context> V8Proxy::mainWorldContext()
-{
-    windowShell()->initContextIfNeeded();
-    return v8::Local<v8::Context>::New(windowShell()->context());
+    return m_frame->script()->mainWorldWindowShell()->localHandleForContext();
 }
 
 v8::Local<v8::Context> V8Proxy::mainWorldContext(Frame* frame)
 {
-    V8Proxy* proxy = retrieve(frame);
-    if (!proxy)
+    // FIXME: Move this function to ScriptController.
+    if (!frame->script()->isEnabled())
         return v8::Local<v8::Context>();
 
-    return proxy->mainWorldContext();
+    return frame->script()->mainWorldWindowShell()->localHandleForContext();
 }
 
 v8::Local<v8::Context> V8Proxy::currentContext()
 {
+    // FIXME: Why does this function exist?
     return v8::Context::GetCurrent();
 }
@@ -831 +805 @@
 {
     ASSERT(debugId > 0);
-    v8::Handle<v8::Context> context = windowShell()->context();
+    v8::Local<v8::Context> context = m_frame->script()->mainWorldWindowShell()->localHandleForContext();
     if (context.IsEmpty())
         return false;

trunk/WebCore/bindings/v8/V8Proxy.h

@@ -153 +153 @@
         Frame* frame() { return m_frame; }
 
-        void clearForNavigation();
-        void clearForClose();
-
         // FIXME: Need comment. User Gesture related.
         bool inlineCode() const { return m_inlineCode; }
@@ -310 +307 @@
 
         v8::Local<v8::Context> context();
-        v8::Local<v8::Context> mainWorldContext();
-
-        // FIXME: This should eventually take DOMWrapperWorld argument!
-        V8DOMWindowShell* windowShell() const { return m_windowShell.get(); }
 
         bool setContextDebugId(int id);
@@ -336 +329 @@
         static void reportUnsafeAccessTo(Frame* target, DelayReporting delay);
 
+        // FIXME: This should move to ScriptController.
+        void resetIsolatedWorlds();
+
     private:
         // If m_recursionCount is 0, let LocalStorage know so we can release
         // the storage mutex.
         void releaseStorageMutex();
-
-        void resetIsolatedWorlds();
 
         // Returns false when we're out of memory in V8.
@@ -371 +365 @@
         Frame* m_frame;
 
-        // For the moment, we have one of these.  Soon we will have one per DOMWrapperWorld.
-        RefPtr<V8DOMWindowShell> m_windowShell;
-        
         // Utility context holding JavaScript functions used internally.
         static v8::Persistent<v8::Context> m_utilityContext;

trunk/WebCore/bindings/v8/WorldContextHandle.cpp

@@ -32 +32 @@
 #include "WorldContextHandle.h"
 
+#include "Frame.h"
 #include "V8IsolatedContext.h"
 
@@ -49 +50 @@
 {
     if (m_worldToUse == UseMainWorld)
-        return proxy->mainWorldContext();
+        return proxy->frame()->script()->mainWorldWindowShell()->localHandleForContext();
     if (!m_context || m_context->get().IsEmpty())
         return proxy->context();

trunk/WebKit/chromium/ChangeLog

@@ +1 @@
+2010-01-05  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        [v8] Move V8DOMWindowShell to ScriptController
+        https://bugs.webkit.org/show_bug.cgi?id=33232
+
+        Update FrameLoaderClientImpl.cpp to call clearForClose on
+        ScriptController instead of on V8Proxy.
+
+        * src/FrameLoaderClientImpl.cpp:
+        (WebKit::FrameLoaderClientImpl::detachedFromParent3):
+
 2010-01-05  Rachel Petterson  <rlp@google.com>
 

trunk/WebKit/chromium/src/FrameLoaderClientImpl.cpp

@@ -216 +216 @@
     // go to a page and then navigate to a new page without getting any asserts
     // or crashes.
-    m_webFrame->frame()->script()->proxy()->clearForClose();
-    
+    m_webFrame->frame()->script()->clearForClose();
+
     // Stop communicating with the WebFrameClient at this point since we are no
     // longer associated with the Page.