<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[165093] trunk/Source</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/165093">165093</a></dd>
<dt>Author</dt> <dd>bburg@apple.com</dd>
<dt>Date</dt> <dd>2014-03-04 20:44:15 -0800 (Tue, 04 Mar 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>Inspector does not restore breakpoints after a page reload
https://bugs.webkit.org/show_bug.cgi?id=129655

Reviewed by Joseph Pecoraro.

Source/JavaScriptCore:

Fix a regression introduced by <a href="http://trac.webkit.org/projects/webkit/changeset/162096">r162096</a> 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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreinspectoragentsInspectorDebuggerAgentcpp">trunk/Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreinspectoragentsInspectorDebuggerAgenth">trunk/Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h</a></li>
<li><a href="#trunkSourceWebInspectorUIChangeLog">trunk/Source/WebInspectorUI/ChangeLog</a></li>
<li><a href="#trunkSourceWebInspectorUIUserInterfaceControllersDebuggerManagerjs">trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (165092 => 165093)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2014-03-05 03:48:33 UTC (rev 165092)
+++ trunk/Source/JavaScriptCore/ChangeLog        2014-03-05 04:44:15 UTC (rev 165093)
</span><span class="lines">@@ -1,3 +1,28 @@
</span><ins>+2014-03-04  Brian Burg  &lt;bburg@apple.com&gt;
+
+        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:
+
</ins><span class="cx"> 2014-03-04  Andreas Kling  &lt;akling@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Streamline JSValue::get().
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreinspectoragentsInspectorDebuggerAgentcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp (165092 => 165093)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp        2014-03-05 03:48:33 UTC (rev 165092)
+++ trunk/Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp        2014-03-05 04:44:15 UTC (rev 165093)
</span><span class="lines">@@ -108,11 +108,11 @@
</span><span class="cx">     if (!m_enabled)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    m_javaScriptBreakpoints.clear();
-
</del><span class="cx">     stopListeningScriptDebugServer(isBeingDestroyed);
</span><del>-    clearResolvedBreakpointState();
</del><ins>+    clearInspectorBreakpointState();
</ins><span class="cx"> 
</span><ins>+    ASSERT(m_javaScriptBreakpoints.isEmpty());
+
</ins><span class="cx">     if (m_listener)
</span><span class="cx">         m_listener-&gt;debuggerWasDisabled();
</span><span class="cx"> 
</span><span class="lines">@@ -686,7 +686,7 @@
</span><span class="cx">     scriptDebugServer().breakProgram();
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void InspectorDebuggerAgent::clearResolvedBreakpointState()
</del><ins>+void InspectorDebuggerAgent::clearInspectorBreakpointState()
</ins><span class="cx"> {
</span><span class="cx">     ErrorString dummyError;
</span><span class="cx">     Vector&lt;String&gt; breakpointIdentifiers;
</span><span class="lines">@@ -694,8 +694,13 @@
</span><span class="cx">     for (const String&amp; identifier : breakpointIdentifiers)
</span><span class="cx">         removeBreakpoint(&amp;dummyError, identifier);
</span><span class="cx"> 
</span><del>-    scriptDebugServer().continueProgram();
</del><ins>+    clearDebuggerBreakpointState();
+}
</ins><span class="cx"> 
</span><ins>+void InspectorDebuggerAgent::clearDebuggerBreakpointState()
+{
+    scriptDebugServer().clearBreakpoints();
+
</ins><span class="cx">     m_pausedScriptState = nullptr;
</span><span class="cx">     m_currentCallStack = Deprecated::ScriptValue();
</span><span class="cx">     m_scripts.clear();
</span><span class="lines">@@ -703,9 +708,20 @@
</span><span class="cx">     m_continueToLocationBreakpointID = JSC::noBreakpointID;
</span><span class="cx">     clearBreakDetails();
</span><span class="cx">     m_javaScriptPauseScheduled = false;
</span><del>-    setOverlayMessage(&amp;dummyError, nullptr);
</del><ins>+
+    scriptDebugServer().continueProgram();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><ins>+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-&gt;globalObjectCleared();
+}
+
</ins><span class="cx"> bool InspectorDebuggerAgent::assertPaused(ErrorString* errorString)
</span><span class="cx"> {
</span><span class="cx">     if (!m_pausedScriptState) {
</span><span class="lines">@@ -722,14 +738,7 @@
</span><span class="cx">     m_breakAuxData = nullptr;
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void InspectorDebuggerAgent::didClearGlobalObject()
-{
-    if (m_frontendDispatcher)
-        m_frontendDispatcher-&gt;globalObjectCleared();
</del><span class="cx"> 
</span><del>-    clearResolvedBreakpointState();
-}
-
</del><span class="cx"> } // namespace Inspector
</span><span class="cx"> 
</span><span class="cx"> #endif // ENABLE(INSPECTOR)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreinspectoragentsInspectorDebuggerAgenth"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h (165092 => 165093)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h        2014-03-05 03:48:33 UTC (rev 165092)
+++ trunk/Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h        2014-03-05 04:44:15 UTC (rev 165093)
</span><span class="lines">@@ -139,7 +139,8 @@
</span><span class="cx"> 
</span><span class="cx">     PassRefPtr&lt;Inspector::TypeBuilder::Debugger::Location&gt; resolveBreakpoint(const String&amp; breakpointIdentifier, JSC::SourceID, const ScriptBreakpoint&amp;);
</span><span class="cx">     bool assertPaused(ErrorString*);
</span><del>-    void clearResolvedBreakpointState();
</del><ins>+    void clearDebuggerBreakpointState();
+    void clearInspectorBreakpointState();
</ins><span class="cx">     void clearBreakDetails();
</span><span class="cx"> 
</span><span class="cx">     bool breakpointActionsFromProtocol(ErrorString*, RefPtr&lt;InspectorArray&gt;&amp; actions, Vector&lt;ScriptBreakpointAction&gt;* result);
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/ChangeLog (165092 => 165093)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/ChangeLog        2014-03-05 03:48:33 UTC (rev 165092)
+++ trunk/Source/WebInspectorUI/ChangeLog        2014-03-05 04:44:15 UTC (rev 165093)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+2014-03-04  Brian Burg  &lt;bburg@apple.com&gt;
+
+        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.
+
</ins><span class="cx"> 2014-03-04  Diego Pino Garcia  &lt;dpino@igalia.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: Remove WebInspector.EventHandler in favor of WebInspector.EventListenerSet
</span></span></pre></div>
<a id="trunkSourceWebInspectorUIUserInterfaceControllersDebuggerManagerjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js (165092 => 165093)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js        2014-03-05 03:48:33 UTC (rev 165092)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js        2014-03-05 04:44:15 UTC (rev 165093)
</span><span class="lines">@@ -305,7 +305,7 @@
</span><span class="cx">         if (!breakpoint)
</span><span class="cx">             return;
</span><span class="cx"> 
</span><del>-        console.assert(breakpoint.id === breakpointIdentifier);
</del><ins>+        console.assert(breakpoint.identifier === breakpointIdentifier);
</ins><span class="cx"> 
</span><span class="cx">         breakpoint.resolved = true;
</span><span class="cx">     },
</span><span class="lines">@@ -411,7 +411,7 @@
</span><span class="cx">     {
</span><span class="cx">         // Don't add the script again if it is already known.
</span><span class="cx">         if (this._scriptIdMap[scriptIdentifier]) {
</span><del>-            console.assert(this._scriptIdMap[scriptIdentifier].url === url);
</del><ins>+            console.assert(this._scriptIdMap[scriptIdentifier].url === (url || null));
</ins><span class="cx">             console.assert(this._scriptIdMap[scriptIdentifier].range.startLine === startLine);
</span><span class="cx">             console.assert(this._scriptIdMap[scriptIdentifier].range.startColumn === startColumn);
</span><span class="cx">             console.assert(this._scriptIdMap[scriptIdentifier].range.endLine === endLine);
</span></span></pre>
</div>
</div>

</body>
</html>