Changeset 56404 in webkit

Timestamp:

Mar 23, 2010 11:49:46 AM (14 years ago)

Author:

eric@webkit.org

Message:

2010-03-23 Andrey Kosyakov <​caseq@chromium.org>

Reviewed by Dmitry Titov.

Change notification sent to inspector frontend on workers creation / destruction
to be posted asynchronously to avoid JS reenterability problems. Also, renamed
willDestroyWorker to didDestroyWorker to reflect that it gets called later and got
rid of explicit IDs for workers (use addresses instead).
​https://bugs.webkit.org/show_bug.cgi?id=36213

• inspector/InjectedScriptHost.cpp: (WebCore::InjectedScriptHost::didDestroyWorker):
• inspector/InjectedScriptHost.h:
• inspector/InjectedScriptHost.idl:
• inspector/InspectorController.cpp: (WebCore::PostWorkerNotificationToFrontendTask::create): (WebCore::PostWorkerNotificationToFrontendTask::PostWorkerNotificationToFrontendTask): (WebCore::PostWorkerNotificationToFrontendTask::performTask): (WebCore::InspectorController::postWorkerNotificationToFrontend): (WebCore::InspectorController::didCreateWorker): (WebCore::InspectorController::didDestroyWorker):
• inspector/InspectorController.h: (WebCore::InspectorController::):
• inspector/InspectorFrontend.cpp: (WebCore::InspectorFrontend::didDestroyWorker):
• inspector/InspectorFrontend.h:
• inspector/InspectorWorkerResource.h: (WebCore::InspectorWorkerResource::create): (WebCore::InspectorWorkerResource::id): (WebCore::InspectorWorkerResource::InspectorWorkerResource):
• inspector/front-end/InjectedFakeWorker.js: (InjectedFakeWorker.FakeWorker.prototype.terminate):
• inspector/front-end/WorkersSidebarPane.js: (WebInspector.didDestroyWorker):
• workers/AbstractWorker.cpp: (WebCore::AbstractWorker::AbstractWorker): (WebCore::AbstractWorker::onDestroyWorker):
• workers/AbstractWorker.h: (WebCore::AbstractWorker::asID):
• workers/SharedWorker.cpp: (WebCore::SharedWorker::SharedWorker):
• workers/Worker.cpp: (WebCore::Worker::Worker):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• inspector/InjectedScriptHost.cpp (modified) (1 diff)
• inspector/InjectedScriptHost.h (modified) (1 diff)
• inspector/InjectedScriptHost.idl (modified) (1 diff)
• inspector/InspectorController.cpp (modified) (3 diffs)
• inspector/InspectorController.h (modified) (2 diffs)
• inspector/InspectorFrontend.cpp (modified) (1 diff)
• inspector/InspectorFrontend.h (modified) (1 diff)
• inspector/InspectorWorkerResource.h (modified) (2 diffs)
• inspector/front-end/InjectedFakeWorker.js (modified) (1 diff)
• inspector/front-end/WorkersSidebarPane.js (modified) (1 diff)
• workers/AbstractWorker.cpp (modified) (2 diffs)
• workers/AbstractWorker.h (modified) (3 diffs)
• workers/SharedWorker.cpp (modified) (1 diff)
• workers/Worker.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-03-23  Andrey Kosyakov  <caseq@chromium.org>
+
+        Reviewed by Dmitry Titov.
+
+        Change notification sent to inspector frontend on workers creation / destruction
+        to be posted asynchronously to avoid JS reenterability problems. Also, renamed
+        willDestroyWorker to didDestroyWorker to reflect that it gets called later and got
+        rid of explicit IDs for workers (use addresses instead).
+        https://bugs.webkit.org/show_bug.cgi?id=36213
+
+        * inspector/InjectedScriptHost.cpp:
+        (WebCore::InjectedScriptHost::didDestroyWorker):
+        * inspector/InjectedScriptHost.h:
+        * inspector/InjectedScriptHost.idl:
+        * inspector/InspectorController.cpp:
+        (WebCore::PostWorkerNotificationToFrontendTask::create):
+        (WebCore::PostWorkerNotificationToFrontendTask::PostWorkerNotificationToFrontendTask):
+        (WebCore::PostWorkerNotificationToFrontendTask::performTask):
+        (WebCore::InspectorController::postWorkerNotificationToFrontend):
+        (WebCore::InspectorController::didCreateWorker):
+        (WebCore::InspectorController::didDestroyWorker):
+        * inspector/InspectorController.h:
+        (WebCore::InspectorController::):
+        * inspector/InspectorFrontend.cpp:
+        (WebCore::InspectorFrontend::didDestroyWorker):
+        * inspector/InspectorFrontend.h:
+        * inspector/InspectorWorkerResource.h:
+        (WebCore::InspectorWorkerResource::create):
+        (WebCore::InspectorWorkerResource::id):
+        (WebCore::InspectorWorkerResource::InspectorWorkerResource):
+        * inspector/front-end/InjectedFakeWorker.js:
+        (InjectedFakeWorker.FakeWorker.prototype.terminate):
+        * inspector/front-end/WorkersSidebarPane.js:
+        (WebInspector.didDestroyWorker):
+        * workers/AbstractWorker.cpp:
+        (WebCore::AbstractWorker::AbstractWorker):
+        (WebCore::AbstractWorker::onDestroyWorker):
+        * workers/AbstractWorker.h:
+        (WebCore::AbstractWorker::asID):
+        * workers/SharedWorker.cpp:
+        (WebCore::SharedWorker::SharedWorker):
+        * workers/Worker.cpp:
+        (WebCore::Worker::Worker):
+
 2010-03-22  David Hyatt  <hyatt@apple.com>
 

trunk/WebCore/inspector/InjectedScriptHost.cpp

@@ -214 +214 @@
 }
 
-void InjectedScriptHost::willDestroyWorker(long id)
-{
-    if (m_inspectorController)
-        m_inspectorController->willDestroyWorker(id);
+void InjectedScriptHost::didDestroyWorker(long id)
+{
+    if (m_inspectorController)
+        m_inspectorController->didDestroyWorker(id);
 }
 #endif // ENABLE(WORKERS)

trunk/WebCore/inspector/InjectedScriptHost.h

@@ -84 +84 @@
     long nextWorkerId();
     void didCreateWorker(long id, const String& url, bool isSharedWorker);
-    void willDestroyWorker(long id);
+    void didDestroyWorker(long id);
 #endif
     void reportDidDispatchOnInjectedScript(long callId, SerializedScriptValue* result, bool isException);

trunk/WebCore/inspector/InjectedScriptHost.idl

@@ -58 +58 @@
 #if defined(ENABLE_WORKERS) && ENABLE_WORKERS
         void didCreateWorker(in long id, in DOMString url, in boolean isFakeWorker);
-        void willDestroyWorker(in long id);
+        void didDestroyWorker(in long id);
         long nextWorkerId();
 #endif

trunk/WebCore/inspector/InspectorController.cpp

@@ -1058 +1058 @@
 
 #if ENABLE(WORKERS)
-void InspectorController::didCreateWorker(long id, const String& url, bool isSharedWorker)
+class PostWorkerNotificationToFrontendTask : public ScriptExecutionContext::Task {
+public:
+    static PassOwnPtr<PostWorkerNotificationToFrontendTask> create(PassRefPtr<InspectorWorkerResource> worker, InspectorController::WorkerAction action)
+    {
+        return new PostWorkerNotificationToFrontendTask(worker, action);
+    }
+
+private:
+    PostWorkerNotificationToFrontendTask(PassRefPtr<InspectorWorkerResource> worker, InspectorController::WorkerAction action)
+        : m_worker(worker)
+        , m_action(action)
+    {
+    }
+
+    virtual void performTask(ScriptExecutionContext* scriptContext)
+    {
+        if (InspectorController* inspector = scriptContext->inspectorController())
+            inspector->postWorkerNotificationToFrontend(*m_worker, m_action);
+    }
+
+private:
+    RefPtr<InspectorWorkerResource> m_worker;
+    InspectorController::WorkerAction m_action;
+};
+
+void InspectorController::postWorkerNotificationToFrontend(const InspectorWorkerResource& worker, InspectorController::WorkerAction action)
+{
+    if (!m_frontend)
+        return;
+    switch (action) {
+    case InspectorController::WorkerCreated:
+        m_frontend->didCreateWorker(worker);
+        break;
+    case InspectorController::WorkerDestroyed:
+        m_frontend->didDestroyWorker(worker);
+        break;
+    }
+}
+
+void InspectorController::didCreateWorker(intptr_t id, const String& url, bool isSharedWorker)
 {
     if (!enabled())
@@ -1065 +1104 @@
     RefPtr<InspectorWorkerResource> workerResource(InspectorWorkerResource::create(id, url, isSharedWorker));
     m_workers.set(id, workerResource);
-    if (m_frontend)
-        m_frontend->didCreateWorker(*workerResource);
-}
-
-void InspectorController::willDestroyWorker(long id)
+    if (m_inspectedPage && m_frontend)
+        m_inspectedPage->mainFrame()->document()->postTask(PostWorkerNotificationToFrontendTask::create(workerResource, InspectorController::WorkerCreated));
+}
+
+void InspectorController::didDestroyWorker(intptr_t id)
 {
     if (!enabled())
@@ -1077 +1116 @@
     if (workerResource == m_workers.end())
         return;
-    if (m_frontend)
-        m_frontend->willDestroyWorker(*workerResource->second);
+    if (m_inspectedPage && m_frontend)
+        m_inspectedPage->mainFrame()->document()->postTask(PostWorkerNotificationToFrontendTask::create(workerResource->second, InspectorController::WorkerDestroyed));
     m_workers.remove(workerResource);
 }

trunk/WebCore/inspector/InspectorController.h

@@ -184 +184 @@
     void didModifyDOMAttr(Element*);
 #if ENABLE(WORKERS)
-    void didCreateWorker(long id, const String& url, bool isSharedWorker);
-    void willDestroyWorker(long id);
+    enum WorkerAction { WorkerCreated, WorkerDestroyed };
+
+    void postWorkerNotificationToFrontend(const InspectorWorkerResource&, WorkerAction);
+    void didCreateWorker(intptr_t, const String& url, bool isSharedWorker);
+    void didDestroyWorker(intptr_t);
 #endif
     void getCookies(long callId);
@@ -362 +365 @@
 #endif
 #if ENABLE(WORKERS)
-    typedef HashMap<long, RefPtr<InspectorWorkerResource> > WorkersMap;
+    typedef HashMap<intptr_t, RefPtr<InspectorWorkerResource> > WorkersMap;
 
     WorkersMap m_workers;

trunk/WebCore/inspector/InspectorFrontend.cpp

@@ -603 +603 @@
 }
 
-void InspectorFrontend::willDestroyWorker(const InspectorWorkerResource& worker)
-{
-    ScriptFunctionCall function(m_webInspector, "dispatch"); 
-    function.appendArgument("willDestroyWorker");
+void InspectorFrontend::didDestroyWorker(const InspectorWorkerResource& worker)
+{
+    ScriptFunctionCall function(m_webInspector, "dispatch"); 
+    function.appendArgument("didDestroyWorker");
     function.appendArgument(worker.id());
     function.call();

trunk/WebCore/inspector/InspectorFrontend.h

@@ -151 +151 @@
 #if ENABLE(WORKERS)
         void didCreateWorker(const InspectorWorkerResource&);
-        void willDestroyWorker(const InspectorWorkerResource&);
+        void didDestroyWorker(const InspectorWorkerResource&);
 #endif // ENABLE(WORKER)
 

trunk/WebCore/inspector/InspectorWorkerResource.h

@@ -43 +43 @@
 class InspectorWorkerResource : public RefCounted<InspectorWorkerResource> {
 public:
-    static PassRefPtr<InspectorWorkerResource> create(long id, const String& url, bool isSharedWorker)
+    static PassRefPtr<InspectorWorkerResource> create(intptr_t id, const String& url, bool isSharedWorker)
     {
         return adoptRef(new InspectorWorkerResource(id, url, isSharedWorker));
     }
 
-    long id() const { return m_id; }
+    intptr_t id() const { return m_id; }
     const String& url() const { return m_url; }
     bool isSharedWorker() const { return m_isSharedWorker; }
 private:
-    InspectorWorkerResource(long id, const String& url, bool isSharedWorker)
+    InspectorWorkerResource(intptr_t id, const String& url, bool isSharedWorker)
         : m_id(id)
         , m_url(url)
@@ -59 +59 @@
     }
 
-    int m_id;
+    intptr_t m_id;
     String m_url;
     bool m_isSharedWorker;

trunk/WebCore/inspector/front-end/InjectedFakeWorker.js

@@ -68 +68 @@
     terminate: function()
     {
-        InjectedScriptHost.willDestroyWorker(this._id);
+        InjectedScriptHost.didDestroyWorker(this._id);
 
         if (this._frame != null) {

trunk/WebCore/inspector/front-end/WorkersSidebarPane.js

@@ -108 +108 @@
 }
 
-WebInspector.willDestroyWorker = function()
+WebInspector.didDestroyWorker = function()
 {
     var workersPane = WebInspector.panels.scripts.sidebarPanes.workers;

trunk/WebCore/workers/AbstractWorker.cpp

@@ -45 +45 @@
 namespace WebCore {
 
-long AbstractWorker::s_lastId;
-
 AbstractWorker::AbstractWorker(ScriptExecutionContext* context)
     : ActiveDOMObject(context, this)
-    , m_id(++s_lastId)
 {
 }
@@ -62 +59 @@
 #if ENABLE(INSPECTOR)
     if (InspectorController* inspector = scriptExecutionContext() ? scriptExecutionContext()->inspectorController() : 0)
-        inspector->willDestroyWorker(id());
+        inspector->didDestroyWorker(asID());
 #endif
 }

trunk/WebCore/workers/AbstractWorker.h

@@ -58 +58 @@
         using RefCounted<AbstractWorker>::deref;
 
-        long id() const { return m_id; }
         virtual void contextDestroyed();
         AbstractWorker(ScriptExecutionContext*);
@@ -66 +65 @@
         // Helper function that converts a URL to an absolute URL and checks the result for validity.
         KURL resolveURL(const String& url, ExceptionCode& ec);
+        intptr_t asID() const { return reinterpret_cast<intptr_t>(this); }
 
     private:
@@ -75 +75 @@
         
         EventTargetData m_eventTargetData;
-        long m_id;
-
-        static long s_lastId;
     };
 

trunk/WebCore/workers/SharedWorker.cpp

@@ -57 +57 @@
 #if ENABLE(INSPECTOR)
     if (InspectorController* inspector = scriptExecutionContext()->inspectorController())
-        inspector->didCreateWorker(id(), scriptUrl.string(), true);
+        inspector->didCreateWorker(asID(), scriptUrl.string(), true);
 #endif
 }

trunk/WebCore/workers/Worker.cpp

@@ -64 +64 @@
 #if ENABLE(INSPECTOR)
     if (InspectorController* inspector = scriptExecutionContext()->inspectorController())
-        inspector->didCreateWorker(id(), scriptURL.string(), false);
+        inspector->didCreateWorker(asID(), scriptURL.string(), false);
 #endif
 }