Changeset 52937 in webkit

Timestamp:

Jan 7, 2010 12:00:38 PM (14 years ago)

Author:

abarth@webkit.org

Message:

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

No review, rolling out r52847.
​http://trac.webkit.org/changeset/52847
​https://bugs.webkit.org/show_bug.cgi?id=33232

Might have caused a perf regression. I'm rolling this out to see if it
heals the perf bot.

• bindings/v8/ScriptController.cpp: (WebCore::ScriptController::ScriptController): (WebCore::ScriptController::~ScriptController): (WebCore::ScriptController::updateSecurityOrigin): (WebCore::ScriptController::evaluateInIsolatedWorld): (WebCore::ScriptController::haveInterpreter): (WebCore::ScriptController::clearWindowShell): (WebCore::ScriptController::updateDocument):
• bindings/v8/ScriptController.h:
• bindings/v8/V8DOMWindowShell.cpp:
• bindings/v8/V8DOMWindowShell.h:
• 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::clearForClose): (WebCore::V8Proxy::clearForNavigation): (WebCore::V8Proxy::context): (WebCore::V8Proxy::mainWorldContext): (WebCore::V8Proxy::currentContext): (WebCore::V8Proxy::setContextDebugId):
• bindings/v8/V8Proxy.h: (WebCore::V8Proxy::windowShell):
• bindings/v8/WorldContextHandle.cpp: (WebCore::WorldContextHandle::adjustedContext):

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

No review, rolling out r52847.
​http://trac.webkit.org/changeset/52847
​https://bugs.webkit.org/show_bug.cgi?id=33232

Might have caused a perf regression.

• 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-07  Adam Barth  <abarth@webkit.org>
+
+        No review, rolling out r52847.
+        http://trac.webkit.org/changeset/52847
+        https://bugs.webkit.org/show_bug.cgi?id=33232
+
+        Might have caused a perf regression.  I'm rolling this out to see if it
+        heals the perf bot.
+
+        * bindings/v8/ScriptController.cpp:
+        (WebCore::ScriptController::ScriptController):
+        (WebCore::ScriptController::~ScriptController):
+        (WebCore::ScriptController::updateSecurityOrigin):
+        (WebCore::ScriptController::evaluateInIsolatedWorld):
+        (WebCore::ScriptController::haveInterpreter):
+        (WebCore::ScriptController::clearWindowShell):
+        (WebCore::ScriptController::updateDocument):
+        * bindings/v8/ScriptController.h:
+        * bindings/v8/V8DOMWindowShell.cpp:
+        * bindings/v8/V8DOMWindowShell.h:
+        * 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::clearForClose):
+        (WebCore::V8Proxy::clearForNavigation):
+        (WebCore::V8Proxy::context):
+        (WebCore::V8Proxy::mainWorldContext):
+        (WebCore::V8Proxy::currentContext):
+        (WebCore::V8Proxy::setContextDebugId):
+        * bindings/v8/V8Proxy.h:
+        (WebCore::V8Proxy::windowShell):
+        * bindings/v8/WorldContextHandle.cpp:
+        (WebCore::WorldContextHandle::adjustedContext):
+
 2010-01-07  Dan Bernstein  <mitz@apple.com>
 

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)
@@ -118 +117 @@
 {
     m_proxy->disconnectFrame();
-    m_windowShell.clear();
 }
 
@@ -143 +141 @@
 void ScriptController::updateSecurityOrigin()
 {
-    m_windowShell->updateSecurityOrigin();
+    m_proxy->windowShell()->updateSecurityOrigin();
 }
 
@@ -208 +206 @@
 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);
 }
@@ -296 +292 @@
 bool ScriptController::haveInterpreter() const
 {
-    return m_windowShell->isContextInitialized();
+    return m_proxy->windowShell()->isContextInitialized();
 }
 
@@ -440 +436 @@
 }
 
-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. V8DOMWindowShell::clearForNavigation
+    // when a frame is loading a new page. V8Proxy::clearForNavigation
     // creates a new context for the new page.
-    m_windowShell->clearForNavigation();
-}
-
-void ScriptController::clearForClose()
-{
-    m_windowShell->clearForClose();
-}
-
-void ScriptController::destroyWindowShell()
-{
-    m_windowShell->clearForClose();
-    m_windowShell->destroyGlobal();
+    m_proxy->clearForNavigation();
 }
 
@@ -475 +454 @@
 void ScriptController::updateDocument()
 {
-    m_windowShell->updateDocument();
+    m_proxy->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);
@@ -156 +154 @@
     void updateDocument();
 
-    void clearForClose();
-
-    // This is very destructive (e.g., out of memory).
-    void destroyWindowShell();
-
     void updateSecurityOrigin();
     void clearScriptObjects();
@@ -190 +183 @@
     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

@@ -131 +131 @@
     : 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.
@@ -71 +66 @@
     void clearForNavigation();
     void clearForClose();
+
     void destroyGlobal();
 

trunk/WebCore/bindings/v8/V8DOMWrapper.cpp

@@ -836 +836 @@
     if (proxy)
         // FIXME: Fix this to work properly with isolated worlds (see above).
-        instance = proxy->frame()->script()->mainWorldWindowShell()->createWrapperFromCache(descriptorType);
+        instance = proxy->windowShell()->createWrapperFromCache(descriptorType);
     else {
         v8::Local<v8::Function> function = getTemplate(descriptorType)->GetFunction();
@@ -1238 +1238 @@
     // checking if the node already has a wrapper.
     V8Proxy* proxy = V8Proxy::retrieve(document->frame());
-    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();
-    }
+    if (proxy)
+        proxy->windowShell()->initContextIfNeeded();
 
     DOMWrapperMap<Node>& domNodeMap = getDOMNodeMap();
@@ -1335 +1328 @@
 
     if (isDocument) {
-        if (proxy) {
-            // FIXME: Don't we need to do this for every world?
-            proxy->frame()->script()->mainWorldWindowShell()->updateDocumentWrapper(result);
-        }
+        if (proxy)
+            proxy->windowShell()->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(mainWorldWindowShell->createNewContext(v8::Handle<v8::Object>(), extensionGroup));
+    m_context = SharedPersistent<v8::Context>::create(proxy->windowShell()->createNewContext(v8::Handle<v8::Object>(), extensionGroup));
     if (m_context->get().IsEmpty())
         return;
@@ -73 +67 @@
     V8DOMWindowShell::installHiddenObjectPrototype(m_context->get());
     // FIXME: This will go away once we have a windowShell for the isolated world.
-    mainWorldWindowShell->installDOMWindow(m_context->get(), proxy->frame()->domWindow());
+    proxy->windowShell()->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)
@@ -222 +223 @@
 V8Proxy::~V8Proxy()
 {
-    resetIsolatedWorlds();
+    clearForClose();
+    windowShell()->destroyGlobal();
 }
 
@@ -245 +247 @@
     Frame* frame = V8Proxy::retrieveFrame(context);
 
-    frame->script()->destroyWindowShell();
+    V8Proxy* proxy = V8Proxy::retrieve(frame);
+    if (proxy) {
+        // Clean m_context, and event handlers.
+        proxy->clearForClose();
+
+        proxy->windowShell()->destroyGlobal();
+    }
 
     ChromiumBridge::notifyJSOutOfMemory(frame);
@@ -259 +267 @@
 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;
@@ -304 +315 @@
     // Setup context id for JS debugger.
     v8::Context::Scope contextScope(targetContext);
-    v8::Handle<v8::Context> context = m_frame->script()->mainWorldWindowShell()->localHandleForContext();
+    v8::Handle<v8::Context> context = windowShell()->context();
     if (context.IsEmpty())
         return false;
@@ -572 +583 @@
     }
     m_isolatedWorlds.clear();
+}
+
+void V8Proxy::clearForClose()
+{
+    resetIsolatedWorlds();
+    windowShell()->clearForClose();
+}
+
+void V8Proxy::clearForNavigation()
+{
+    resetIsolatedWorlds();
+    windowShell()->clearForNavigation();
 }
 
@@ -633 +656 @@
 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())
@@ -649 +671 @@
 v8::Local<v8::Context> V8Proxy::context()
 {
-    // FIXME: Move this function to ScriptController.
     if (V8IsolatedContext* isolatedContext = V8IsolatedContext::getEntered()) {
         RefPtr<SharedPersistent<v8::Context> > context = isolatedContext->sharedContext();
@@ -656 +677 @@
         return v8::Local<v8::Context>::New(context->get());
     }
-    return m_frame->script()->mainWorldWindowShell()->localHandleForContext();
+    return mainWorldContext();
+}
+
+v8::Local<v8::Context> V8Proxy::mainWorldContext()
+{
+    windowShell()->initContextIfNeeded();
+    return v8::Local<v8::Context>::New(windowShell()->context());
 }
 
 v8::Local<v8::Context> V8Proxy::mainWorldContext(Frame* frame)
 {
-    // FIXME: Move this function to ScriptController.
-    if (!frame->script()->isEnabled())
+    V8Proxy* proxy = retrieve(frame);
+    if (!proxy)
         return v8::Local<v8::Context>();
 
-    return frame->script()->mainWorldWindowShell()->localHandleForContext();
+    return proxy->mainWorldContext();
 }
 
 v8::Local<v8::Context> V8Proxy::currentContext()
 {
-    // FIXME: Why does this function exist?
     return v8::Context::GetCurrent();
 }
@@ -805 +831 @@
 {
     ASSERT(debugId > 0);
+<<<<<<< HEAD
     v8::HandleScope scope;
     v8::Local<v8::Context> context = m_frame->script()->mainWorldWindowShell()->localHandleForContext();
+=======
+    v8::Handle<v8::Context> context = windowShell()->context();
+>>>>>>> 99d3c27... 2010-01-05  Adam Barth  <abarth@webkit.org>
     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; }
@@ -307 +310 @@
 
         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);
@@ -329 +336 @@
         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.
@@ -365 +371 @@
         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"
 
@@ -50 +49 @@
 {
     if (m_worldToUse == UseMainWorld)
-        return proxy->frame()->script()->mainWorldWindowShell()->localHandleForContext();
+        return proxy->mainWorldContext();
     if (!m_context || m_context->get().IsEmpty())
         return proxy->context();

trunk/WebKit/chromium/ChangeLog

@@ +1 @@
+2010-01-07  Adam Barth  <abarth@webkit.org>
+
+        No review, rolling out r52847.
+        http://trac.webkit.org/changeset/52847
+        https://bugs.webkit.org/show_bug.cgi?id=33232
+
+        Might have caused a perf regression.
+
+        * src/FrameLoaderClientImpl.cpp:
+        (WebKit::FrameLoaderClientImpl::detachedFromParent3):
+
 2010-01-06  Mike Belshe  <mike@belshe.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()->clearForClose();
-
+    m_webFrame->frame()->script()->proxy()->clearForClose();
+    
     // Stop communicating with the WebFrameClient at this point since we are no
     // longer associated with the Page.