Changeset 37656 in webkit

Timestamp:

Oct 17, 2008 9:51:03 AM (16 years ago)

Author:

timothy@apple.com

Message:

Fixes a regression where starting a debugging session in
another Web Inspector window would not show any scripts.
Also only attach the debugger to only the required Pages.

​https://bugs.webkit.org/show_bug.cgi?id=21708

Reviewed by Darin Adler.

• inspector/JavaScriptDebugServer.cpp: (WebCore::JavaScriptDebugServer::addListener): Add an assert and remove the call to willAddFirstListener(). Add a call to didAddListener(). (WebCore::JavaScriptDebugServer::removeListener): Add an assert and call to didRemoveListener(). (WebCore::JavaScriptDebugServer::pageCreated): Attach the debugger if there is a listener interested in the new Page. (WebCore::JavaScriptDebugServer::hasListenersInterestedInPage): Use hasGlobalListeners() instead of isEmpty(). (WebCore::JavaScriptDebugServer::sourceParsed): Ditto. (WebCore::JavaScriptDebugServer::didAddListener): Added. Attach the debugger to the page if non-null, or all pages if page is null. (WebCore::JavaScriptDebugServer::didRemoveListener): Added. Detach the debugger only if there are no interested listeners. (WebCore::JavaScriptDebugServer::didRemoveLastListener): Remove the call to Page::setDebuggerForAllPages().
• inspector/JavaScriptDebugServer.h: (WebCore::JavaScriptDebugServer::hasGlobalListeners): Added.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• inspector/JavaScriptDebugServer.cpp (modified) (12 diffs)
• inspector/JavaScriptDebugServer.h (modified) (2 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-10-17  Timothy Hatcher  <timothy@apple.com>
+
+        Fixes a regression where starting a debugging session in
+        another Web Inspector window would not show any scripts.
+        Also only attach the debugger to only the required Pages.
+
+        https://bugs.webkit.org/show_bug.cgi?id=21708
+
+        Reviewed by Darin Adler.
+
+        * inspector/JavaScriptDebugServer.cpp:
+        (WebCore::JavaScriptDebugServer::addListener): Add an assert and
+        remove the call to willAddFirstListener(). Add a call to
+        didAddListener().
+        (WebCore::JavaScriptDebugServer::removeListener): Add an assert
+        and call to didRemoveListener().
+        (WebCore::JavaScriptDebugServer::pageCreated): Attach the debugger
+        if there is a listener interested in the new Page.
+        (WebCore::JavaScriptDebugServer::hasListenersInterestedInPage):
+        Use hasGlobalListeners() instead of isEmpty().
+        (WebCore::JavaScriptDebugServer::sourceParsed): Ditto.
+        (WebCore::JavaScriptDebugServer::didAddListener): Added. Attach the
+        debugger to the page if non-null, or all pages if page is null.
+        (WebCore::JavaScriptDebugServer::didRemoveListener): Added. Detach
+        the debugger only if there are no interested listeners.
+        (WebCore::JavaScriptDebugServer::didRemoveLastListener): Remove the
+        call to Page::setDebuggerForAllPages().
+        * inspector/JavaScriptDebugServer.h:
+        (WebCore::JavaScriptDebugServer::hasGlobalListeners): Added.
+
 2008-10-15  Brett Wilson  <brettw@chromium.org>
 

trunk/WebCore/inspector/JavaScriptDebugServer.cpp

@@ -84 +84 @@
 void JavaScriptDebugServer::addListener(JavaScriptDebugListener* listener)
 {
-    if (!hasListeners())
-        willAddFirstListener();
+    ASSERT_ARG(listener, listener);
 
     m_listeners.add(listener);
+
+    didAddListener(0);
 }
 
 void JavaScriptDebugServer::removeListener(JavaScriptDebugListener* listener)
 {
+    ASSERT_ARG(listener, listener);
+
     m_listeners.remove(listener);
+
+    didRemoveListener(0);
     if (!hasListeners())
         didRemoveLastListener();
@@ -99 +104 @@
 void JavaScriptDebugServer::addListener(JavaScriptDebugListener* listener, Page* page)
 {
+    ASSERT_ARG(listener, listener);
     ASSERT_ARG(page, page);
-
-    if (!hasListeners())
-        willAddFirstListener();
 
     pair<PageListenersMap::iterator, bool> result = m_pageListenersMap.add(page, 0);
     if (result.second)
         result.first->second = new ListenerSet;
+
     ListenerSet* listeners = result.first->second;
-
     listeners->add(listener);
+
+    didAddListener(page);
 }
 
 void JavaScriptDebugServer::removeListener(JavaScriptDebugListener* listener, Page* page)
 {
+    ASSERT_ARG(listener, listener);
     ASSERT_ARG(page, page);
 
@@ -127 +133 @@
     }
 
+    didRemoveListener(page);
     if (!hasListeners())
         didRemoveLastListener();
@@ -133 +140 @@
 void JavaScriptDebugServer::pageCreated(Page* page)
 {
-    if (!hasListeners())
-        return;
-
+    ASSERT_ARG(page, page);
+
+    if (!hasListenersInterestedInPage(page))
+        return;
     page->setDebugger(this);
 }
@@ -143 +151 @@
     ASSERT_ARG(page, page);
 
-    if (!m_listeners.isEmpty())
+    if (hasGlobalListeners())
         return true;
 
@@ -282 +290 @@
     bool isError = errorLine != -1;
 
-    if (!m_listeners.isEmpty()) {
+    if (hasGlobalListeners()) {
         if (isError)
             dispatchFailedToParseSource(m_listeners, exec, source, errorLine, errorMessage);
@@ -318 +326 @@
 
     WebCore::dispatchFunctionToListeners(m_listeners, callback);
+
     if (ListenerSet* pageListeners = m_pageListenersMap.get(page)) {
         ASSERT(!pageListeners->isEmpty());
@@ -510 +519 @@
     if (m_paused)
         return;
-    
+
     ASSERT(m_currentCallFrame);
     if (!m_currentCallFrame)
@@ -524 +533 @@
     JSLock lock(false);
     JSGlobalData* globalData = JSDOMWindow::commonJSGlobalData();
-    
+
     // If JavaScript is running, it's not safe to recompile, since we'll end
     // up throwing away code that is live on the stack.
@@ -543 +552 @@
     FunctionBodyMap functionBodies;
     SourceProviderSet sourceProviders;
-    
+
     size_t size = functions.size();
     for (size_t i = 0; i < size; ++i) {
         JSFunction* function = functions[i];
-        
+
         FunctionBodyNode* oldBody = function->m_body.get();
         pair<FunctionBodyMap::iterator, bool> result = functionBodies.add(oldBody, 0);
@@ -561 +570 @@
         ASSERT(newBody);
         newBody->finishParsing(oldBody->copyParameters(), oldBody->parameterCount());
-        
+
         result.first->second = newBody;
         function->m_body = newBody.release();
@@ -573 +582 @@
 }
 
-void JavaScriptDebugServer::willAddFirstListener()
-{
-    Page::setDebuggerForAllPages(this);
+void JavaScriptDebugServer::didAddListener(Page* page)
+{
     m_recompileTimer.startOneShot(0);
+
+    if (page)
+        page->setDebugger(this);
+    else
+        Page::setDebuggerForAllPages(this);
+}
+
+void JavaScriptDebugServer::didRemoveListener(Page* page)
+{
+    if (hasGlobalListeners() || (page && hasListenersInterestedInPage(page)))
+        return;
+
+    if (page)
+        page->setDebugger(0);
+    else
+        Page::setDebuggerForAllPages(0);
 }
 
 void JavaScriptDebugServer::didRemoveLastListener()
 {
-    Page::setDebuggerForAllPages(0);
     m_doneProcessingDebuggerEvents = true;
     m_recompileTimer.startOneShot(0);

trunk/WebCore/inspector/JavaScriptDebugServer.h

@@ -88 +88 @@
 
         bool hasListeners() const { return !m_listeners.isEmpty() || !m_pageListenersMap.isEmpty(); }
+        bool hasGlobalListeners() const { return !m_listeners.isEmpty(); }
         bool hasListenersInterestedInPage(Page*);
 
@@ -107 +108 @@
         virtual void didReachBreakpoint(const JSC::DebuggerCallFrame&, intptr_t sourceID, int lineno);
         
-        void willAddFirstListener();
+        void didAddListener(Page*);
+        void didRemoveListener(Page*);
         void didRemoveLastListener();