Inspector does not restore breakpoints after a page reload
authorbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Mar 2014 04:44:15 +0000 (04:44 +0000)
committerbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Mar 2014 04:44:15 +0000 (04:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=129655

Reviewed by Joseph Pecoraro.

Source/JavaScriptCore:

Fix a regression introduced by r162096 that erroneously removed
the inspector backend's mapping of files to breakpoints whenever the
global object was cleared.

The inspector's breakpoint mappings should only be cleared when the
debugger agent is disabled or destroyed. We should only clear the
debugger's breakpoint state when the global object is cleared.

To make it clearer what state is being cleared, the two cases have
been split into separate methods.

* inspector/agents/InspectorDebuggerAgent.cpp:
(Inspector::InspectorDebuggerAgent::disable):
(Inspector::InspectorDebuggerAgent::clearInspectorBreakpointState):
(Inspector::InspectorDebuggerAgent::clearDebuggerBreakpointState):
(Inspector::InspectorDebuggerAgent::didClearGlobalObject):
* inspector/agents/InspectorDebuggerAgent.h:

Source/WebInspectorUI:

Fix some console asserts that fire when breakpoints resolve.

* UserInterface/Controllers/DebuggerManager.js:
(WebInspector.DebuggerManager.prototype.breakpointResolved):
This had a typo, it should be `breakpoint.identifier`.
(WebInspector.DebuggerManager.prototype.scriptDidParse):
Sometimes the `url` parameter is empty instead of null.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@165093 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp
Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js

index 9718bf7..12ee6dd 100644 (file)
@@ -1,3 +1,28 @@
+2014-03-04  Brian Burg  <bburg@apple.com>
+
+        Inspector does not restore breakpoints after a page reload
+        https://bugs.webkit.org/show_bug.cgi?id=129655
+
+        Reviewed by Joseph Pecoraro.
+
+        Fix a regression introduced by r162096 that erroneously removed
+        the inspector backend's mapping of files to breakpoints whenever the
+        global object was cleared.
+
+        The inspector's breakpoint mappings should only be cleared when the
+        debugger agent is disabled or destroyed. We should only clear the
+        debugger's breakpoint state when the global object is cleared.
+
+        To make it clearer what state is being cleared, the two cases have
+        been split into separate methods.
+
+        * inspector/agents/InspectorDebuggerAgent.cpp:
+        (Inspector::InspectorDebuggerAgent::disable):
+        (Inspector::InspectorDebuggerAgent::clearInspectorBreakpointState):
+        (Inspector::InspectorDebuggerAgent::clearDebuggerBreakpointState):
+        (Inspector::InspectorDebuggerAgent::didClearGlobalObject):
+        * inspector/agents/InspectorDebuggerAgent.h:
+
 2014-03-04  Andreas Kling  <akling@apple.com>
 
         Streamline JSValue::get().
index 4d0bbcd..da54085 100644 (file)
@@ -108,10 +108,10 @@ void InspectorDebuggerAgent::disable(bool isBeingDestroyed)
     if (!m_enabled)
         return;
 
-    m_javaScriptBreakpoints.clear();
-
     stopListeningScriptDebugServer(isBeingDestroyed);
-    clearResolvedBreakpointState();
+    clearInspectorBreakpointState();
+
+    ASSERT(m_javaScriptBreakpoints.isEmpty());
 
     if (m_listener)
         m_listener->debuggerWasDisabled();
@@ -686,7 +686,7 @@ void InspectorDebuggerAgent::breakProgram(InspectorDebuggerFrontendDispatcher::R
     scriptDebugServer().breakProgram();
 }
 
-void InspectorDebuggerAgent::clearResolvedBreakpointState()
+void InspectorDebuggerAgent::clearInspectorBreakpointState()
 {
     ErrorString dummyError;
     Vector<String> breakpointIdentifiers;
@@ -694,7 +694,12 @@ void InspectorDebuggerAgent::clearResolvedBreakpointState()
     for (const String& identifier : breakpointIdentifiers)
         removeBreakpoint(&dummyError, identifier);
 
-    scriptDebugServer().continueProgram();
+    clearDebuggerBreakpointState();
+}
+
+void InspectorDebuggerAgent::clearDebuggerBreakpointState()
+{
+    scriptDebugServer().clearBreakpoints();
 
     m_pausedScriptState = nullptr;
     m_currentCallStack = Deprecated::ScriptValue();
@@ -703,7 +708,18 @@ void InspectorDebuggerAgent::clearResolvedBreakpointState()
     m_continueToLocationBreakpointID = JSC::noBreakpointID;
     clearBreakDetails();
     m_javaScriptPauseScheduled = false;
-    setOverlayMessage(&dummyError, nullptr);
+
+    scriptDebugServer().continueProgram();
+}
+
+void InspectorDebuggerAgent::didClearGlobalObject()
+{
+    // Clear breakpoints from the debugger, but keep the inspector's model of which
+    // pages have what breakpoints, as the mapping is only sent to DebuggerAgent once.
+    clearDebuggerBreakpointState();
+
+    if (m_frontendDispatcher)
+        m_frontendDispatcher->globalObjectCleared();
 }
 
 bool InspectorDebuggerAgent::assertPaused(ErrorString* errorString)
@@ -722,13 +738,6 @@ void InspectorDebuggerAgent::clearBreakDetails()
     m_breakAuxData = nullptr;
 }
 
-void InspectorDebuggerAgent::didClearGlobalObject()
-{
-    if (m_frontendDispatcher)
-        m_frontendDispatcher->globalObjectCleared();
-
-    clearResolvedBreakpointState();
-}
 
 } // namespace Inspector
 
index 24ce166..0c64f68 100644 (file)
@@ -139,7 +139,8 @@ private:
 
     PassRefPtr<Inspector::TypeBuilder::Debugger::Location> resolveBreakpoint(const String& breakpointIdentifier, JSC::SourceID, const ScriptBreakpoint&);
     bool assertPaused(ErrorString*);
-    void clearResolvedBreakpointState();
+    void clearDebuggerBreakpointState();
+    void clearInspectorBreakpointState();
     void clearBreakDetails();
 
     bool breakpointActionsFromProtocol(ErrorString*, RefPtr<InspectorArray>& actions, Vector<ScriptBreakpointAction>* result);
index 6d8c4d0..11ff5b8 100644 (file)
@@ -1,3 +1,18 @@
+2014-03-04  Brian Burg  <bburg@apple.com>
+
+        Inspector does not restore breakpoints after a page reload
+        https://bugs.webkit.org/show_bug.cgi?id=129655
+
+        Reviewed by Joseph Pecoraro.
+
+        Fix some console asserts that fire when breakpoints resolve.
+
+        * UserInterface/Controllers/DebuggerManager.js:
+        (WebInspector.DebuggerManager.prototype.breakpointResolved):
+        This had a typo, it should be `breakpoint.identifier`.
+        (WebInspector.DebuggerManager.prototype.scriptDidParse):
+        Sometimes the `url` parameter is empty instead of null.
+
 2014-03-04  Diego Pino Garcia  <dpino@igalia.com>
 
         Web Inspector: Remove WebInspector.EventHandler in favor of WebInspector.EventListenerSet
index ade7582..e473a51 100644 (file)
@@ -305,7 +305,7 @@ WebInspector.DebuggerManager.prototype = {
         if (!breakpoint)
             return;
 
-        console.assert(breakpoint.id === breakpointIdentifier);
+        console.assert(breakpoint.identifier === breakpointIdentifier);
 
         breakpoint.resolved = true;
     },
@@ -411,7 +411,7 @@ WebInspector.DebuggerManager.prototype = {
     {
         // Don't add the script again if it is already known.
         if (this._scriptIdMap[scriptIdentifier]) {
-            console.assert(this._scriptIdMap[scriptIdentifier].url === url);
+            console.assert(this._scriptIdMap[scriptIdentifier].url === (url || null));
             console.assert(this._scriptIdMap[scriptIdentifier].range.startLine === startLine);
             console.assert(this._scriptIdMap[scriptIdentifier].range.startColumn === startColumn);
             console.assert(this._scriptIdMap[scriptIdentifier].range.endLine === endLine);