<!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>[178232] trunk</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/178232">178232</a></dd>
<dt>Author</dt> <dd>msaboff@apple.com</dd>
<dt>Date</dt> <dd>2015-01-09 18:44:56 -0800 (Fri, 09 Jan 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>Breakpoint doesn't fire in this HTML5 game
https://bugs.webkit.org/show_bug.cgi?id=140269

Reviewed by Mark Lam.

Source/JavaScriptCore:

When parsing a single line cached function, use the lineStartOffset of the
location where we found the cached function instead of the cached lineStartOffset.
The cache location's lineStartOffset has not been adjusted for any possible
containing functions.

This change is not needed for multi-line cached functions.  Consider the
single line source:

function outer(){function inner1(){doStuff();}; (function inner2() {doMoreStuff()})()}

The first parser pass, we parse and cache inner1() and inner2() with a lineStartOffset
of 0.  Later when we parse outer() and find inner1() in the cache, SourceCode start
character is at outer()'s outermost open brace.  That is what we should use for
lineStartOffset for inner1().  When done parsing inner1() we set the parsing token
to the saved location for inner1(), including the lineStartOffset of 0.  We need
to use the value of lineStartOffset before we started parsing inner1().  That is
what the fix does.  When we parse inner2() the lineStartOffset will be correct.

For a multi-line function, the close brace is guaranteed to be on a different line
than the open brace.  Hence, its lineStartOffset will not change with the change of
the SourceCode start character

* parser/Parser.cpp:
(JSC::Parser&lt;LexerType&gt;::parseFunctionInfo):

LayoutTests:

New tests that set breakpoints in functions with various line split
combinations.

* inspector/debugger/breakpoint-columns-expected.txt: Added.
* inspector/debugger/breakpoint-columns.html: Added.
* inspector/debugger/resources/column-breakpoints-1.js: Added.
(columnTest1.x):
(columnTest1):
(columnTest2.x):
(columnTest2.f):
(columnTest3.x):
(columnTest3.f):
(runColumnTest1):
(runColumnTest2):
(runColumnTest3):
* inspector/debugger/resources/column-breakpoints-2.js: Added.
(columnTest4.x):
(columnTest4.f):
(columnTest5.x):
(columnTest5):
(runColumnTest4):
(runColumnTest5):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreparserParsercpp">trunk/Source/JavaScriptCore/parser/Parser.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsinspectordebuggerbreakpointcolumnsexpectedtxt">trunk/LayoutTests/inspector/debugger/breakpoint-columns-expected.txt</a></li>
<li><a href="#trunkLayoutTestsinspectordebuggerbreakpointcolumnshtml">trunk/LayoutTests/inspector/debugger/breakpoint-columns.html</a></li>
<li><a href="#trunkLayoutTestsinspectordebuggerresourcescolumnbreakpoints1js">trunk/LayoutTests/inspector/debugger/resources/column-breakpoints-1.js</a></li>
<li><a href="#trunkLayoutTestsinspectordebuggerresourcescolumnbreakpoints2js">trunk/LayoutTests/inspector/debugger/resources/column-breakpoints-2.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (178231 => 178232)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2015-01-10 02:12:01 UTC (rev 178231)
+++ trunk/LayoutTests/ChangeLog        2015-01-10 02:44:56 UTC (rev 178232)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2015-01-09  Michael Saboff  &lt;msaboff@apple.com&gt;
+
+        Breakpoint doesn't fire in this HTML5 game
+        https://bugs.webkit.org/show_bug.cgi?id=140269
+
+        Reviewed by Mark Lam.
+
+        New tests that set breakpoints in functions with various line split
+        combinations.
+
+        * inspector/debugger/breakpoint-columns-expected.txt: Added.
+        * inspector/debugger/breakpoint-columns.html: Added.
+        * inspector/debugger/resources/column-breakpoints-1.js: Added.
+        (columnTest1.x):
+        (columnTest1):
+        (columnTest2.x):
+        (columnTest2.f):
+        (columnTest3.x):
+        (columnTest3.f):
+        (runColumnTest1):
+        (runColumnTest2):
+        (runColumnTest3):
+        * inspector/debugger/resources/column-breakpoints-2.js: Added.
+        (columnTest4.x):
+        (columnTest4.f):
+        (columnTest5.x):
+        (columnTest5):
+        (runColumnTest4):
+        (runColumnTest5):
+
</ins><span class="cx"> 2015-01-09  Zalan Bujtas  &lt;zalan@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Calling clearSelection on a detached RenderObject leads to segfault.
</span></span></pre></div>
<a id="trunkLayoutTestsinspectordebuggerbreakpointcolumnsexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/debugger/breakpoint-columns-expected.txt (0 => 178232)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/inspector/debugger/breakpoint-columns-expected.txt                                (rev 0)
+++ trunk/LayoutTests/inspector/debugger/breakpoint-columns-expected.txt        2015-01-10 02:44:56 UTC (rev 178232)
</span><span class="lines">@@ -0,0 +1,19 @@
</span><ins>+CONSOLE MESSAGE: line 1: Paused at line: 0, column: 79
+CONSOLE MESSAGE: line 1: column test 1
+CONSOLE MESSAGE: line 1: Paused at line: 6, column: 21
+CONSOLE MESSAGE: line 7: column test 2
+CONSOLE MESSAGE: line 1: Paused at line: 15, column: 8
+CONSOLE MESSAGE: line 16: column test 3
+CONSOLE MESSAGE: line 1: Paused at line: 5, column: 8
+CONSOLE MESSAGE: line 6: column test 4
+CONSOLE MESSAGE: line 1: Paused at line: 11, column: 79
+CONSOLE MESSAGE: line 12: column test 5
+Testing that breakpoints can be set at various line / column combinations.
+
+Hit breakpoint at line: 0, column: 79
+Hit breakpoint at line: 6, column: 21
+Hit breakpoint at line: 15, column: 8
+Hit breakpoint at line: 5, column: 8
+Hit breakpoint at line: 11, column: 79
+Tests done
+
</ins></span></pre></div>
<a id="trunkLayoutTestsinspectordebuggerbreakpointcolumnshtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/debugger/breakpoint-columns.html (0 => 178232)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/inspector/debugger/breakpoint-columns.html                                (rev 0)
+++ trunk/LayoutTests/inspector/debugger/breakpoint-columns.html        2015-01-10 02:44:56 UTC (rev 178232)
</span><span class="lines">@@ -0,0 +1,91 @@
</span><ins>+&lt;!doctype html&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;script type=&quot;text/javascript&quot; src=&quot;../../http/tests/inspector/inspector-test.js&quot;&gt;&lt;/script&gt;
+&lt;script type=&quot;text/javascript&quot; src=&quot;../../http/tests/inspector/debugger/debugger-test.js&quot;&gt;&lt;/script&gt;
+&lt;script type=&quot;text/javascript&quot; src=&quot;./resources/column-breakpoints-1.js&quot;&gt;&lt;/script&gt;
+&lt;script type=&quot;text/javascript&quot; src=&quot;./resources/column-breakpoints-2.js&quot;&gt;&lt;/script&gt;
+&lt;script&gt;
+
+function test()
+{
+    var testInfoList = [
+        { scriptIndex : 0, line : 0, column : 79, startFunc : &quot;runColumnTest1()&quot; },
+        { scriptIndex : 0, line : 6, column : 21, startFunc : &quot;runColumnTest2()&quot; },
+        { scriptIndex : 0, line : 15, column : 8, startFunc : &quot;runColumnTest3()&quot; },
+        { scriptIndex : 1, line : 5, column : 8, startFunc : &quot;runColumnTest4()&quot; },
+        { scriptIndex : 1, line : 11, column : 79, startFunc : &quot;runColumnTest5()&quot; }
+    ]
+
+    var currentTestIndex = 0;
+    var scriptObject1, scriptObject2;
+    var currentScripts = [];
+
+    function runNextTestIfAllScriptsLoaded() {
+        if (scriptObject1 &amp;&amp; scriptObject2) {
+            currentScripts = [scriptObject1, scriptObject2];
+            runNextTest();
+        }
+    }
+
+    function runNextTest() {
+        if (currentTestIndex &gt;= testInfoList.length) {
+            InspectorTest.log(&quot;Tests done&quot;);
+            InspectorTest.completeTest();
+            return;
+        }
+
+        var testInfo = testInfoList[currentTestIndex];
+        
+        var location = currentScripts[testInfo.scriptIndex].createSourceCodeLocation(testInfo.line, testInfo.column);
+        var breakpoint = new WebInspector.Breakpoint(location);
+
+        WebInspector.debuggerManager.addBreakpoint(breakpoint);
+
+        InspectorTest.evaluateInPage(testInfo.startFunc);
+
+        currentTestIndex++;
+    }        
+
+    WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.CallFramesDidChange, function(event) {
+        var activeCallFrame = WebInspector.debuggerManager.activeCallFrame;
+
+        if (!activeCallFrame)
+            return;
+
+        var stopLocation = &quot;line: &quot; + activeCallFrame.sourceCodeLocation.lineNumber + &quot;, column: &quot; + activeCallFrame.sourceCodeLocation.columnNumber;
+
+        InspectorTest.log(&quot;Hit breakpoint at &quot; + stopLocation);
+        InspectorTest.evaluateInPage(&quot;console.log('Paused at &quot; + stopLocation + &quot;')&quot;);
+
+        WebInspector.debuggerManager.resume();
+    });
+
+    WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.Resumed, function(event) {
+       runNextTest();
+    });
+
+    WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.ScriptAdded, function(event) {
+        var scriptObject = event.data.script;
+        
+        if (/column-breakpoints-1\.js$/.test(scriptObject.url)) {
+            scriptObject1 = scriptObject;
+            runNextTestIfAllScriptsLoaded();
+            return;
+        }
+
+        if (/column-breakpoints-2\.js$/.test(scriptObject.url)) {
+            scriptObject2 = scriptObject;
+            runNextTestIfAllScriptsLoaded();
+            return;
+        }
+    });
+
+    InspectorTest.reloadPage();
+}
+&lt;/script&gt;
+&lt;/head&gt;
+&lt;body onload=&quot;runTest()&quot;&gt;
+    &lt;p&gt;Testing that breakpoints can be set at various line / column combinations.&lt;/p&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkLayoutTestsinspectordebuggerresourcescolumnbreakpoints1js"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/debugger/resources/column-breakpoints-1.js (0 => 178232)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/inspector/debugger/resources/column-breakpoints-1.js                                (rev 0)
+++ trunk/LayoutTests/inspector/debugger/resources/column-breakpoints-1.js        2015-01-10 02:44:56 UTC (rev 178232)
</span><span class="lines">@@ -0,0 +1,27 @@
</span><ins>+function columnTest1(){var x=function(){setInterval(function(){})};(function(){console.log(&quot;column test 1&quot;)})()}
+//                                                         breakpoint set here ^ (0, 79)
+
+function columnTest2()
+{
+    var x=function(){setInterval(function(){})};
+    f = function() { console.log(&quot;column test 2&quot;); }();
+//                   ^ breakpoint set here (6, 21)
+}
+
+function columnTest3()
+{
+    var x=function(){setInterval(function(){})};
+    f = function()
+    {
+        console.log(&quot;column test 3&quot;);
+//      ^ breakpoint set here (15, 8)
+    }
+    f();
+}
+
+// Any edits of this file will necessitate a change to the breakpoint (line, column) coordinates
+// used in breakpoint-columns.html.
+
+function runColumnTest1() { setTimeout(columnTest1, 0); }
+function runColumnTest2() { setTimeout(columnTest2, 0); }
+function runColumnTest3() { setTimeout(columnTest3, 0); }
</ins></span></pre></div>
<a id="trunkLayoutTestsinspectordebuggerresourcescolumnbreakpoints2js"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/inspector/debugger/resources/column-breakpoints-2.js (0 => 178232)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/inspector/debugger/resources/column-breakpoints-2.js                                (rev 0)
+++ trunk/LayoutTests/inspector/debugger/resources/column-breakpoints-2.js        2015-01-10 02:44:56 UTC (rev 178232)
</span><span class="lines">@@ -0,0 +1,20 @@
</span><ins>+function columnTest4()
+{
+    var x=function(){setInterval(function(){})};
+    f = function()
+    {
+        console.log(&quot;column test 4&quot;);
+//      ^ breakpoint set here (5, 8)
+    }
+    f();
+}
+
+function columnTest5(){var x=function(){setInterval(function(){})};(function(){console.log(&quot;column test 5&quot;)})()}
+//                                                         breakpoint set here ^ (11, 79)
+
+
+// Any edits of this file will necessitate a change to the breakpoint (line, column) coordinates
+// used in breakpoint-columns.html.
+
+function runColumnTest4() { setTimeout(columnTest4, 0); }
+function runColumnTest5() { setTimeout(columnTest5, 0); }
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (178231 => 178232)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2015-01-10 02:12:01 UTC (rev 178231)
+++ trunk/Source/JavaScriptCore/ChangeLog        2015-01-10 02:44:56 UTC (rev 178232)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2015-01-09  Michael Saboff  &lt;msaboff@apple.com&gt;
+
+        Breakpoint doesn't fire in this HTML5 game
+        https://bugs.webkit.org/show_bug.cgi?id=140269
+
+        Reviewed by Mark Lam.
+
+        When parsing a single line cached function, use the lineStartOffset of the
+        location where we found the cached function instead of the cached lineStartOffset.
+        The cache location's lineStartOffset has not been adjusted for any possible
+        containing functions.
+
+        This change is not needed for multi-line cached functions.  Consider the
+        single line source:
+
+        function outer(){function inner1(){doStuff();}; (function inner2() {doMoreStuff()})()}
+
+        The first parser pass, we parse and cache inner1() and inner2() with a lineStartOffset
+        of 0.  Later when we parse outer() and find inner1() in the cache, SourceCode start
+        character is at outer()'s outermost open brace.  That is what we should use for
+        lineStartOffset for inner1().  When done parsing inner1() we set the parsing token
+        to the saved location for inner1(), including the lineStartOffset of 0.  We need
+        to use the value of lineStartOffset before we started parsing inner1().  That is
+        what the fix does.  When we parse inner2() the lineStartOffset will be correct.
+
+        For a multi-line function, the close brace is guaranteed to be on a different line
+        than the open brace.  Hence, its lineStartOffset will not change with the change of
+        the SourceCode start character
+
+        * parser/Parser.cpp:
+        (JSC::Parser&lt;LexerType&gt;::parseFunctionInfo):
+
</ins><span class="cx"> 2015-01-09  Joseph Pecoraro  &lt;pecoraro@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: Uncaught Exception in ProbeManager deleting breakpoint
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreparserParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/parser/Parser.cpp (178231 => 178232)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/parser/Parser.cpp        2015-01-10 02:12:01 UTC (rev 178231)
+++ trunk/Source/JavaScriptCore/parser/Parser.cpp        2015-01-10 02:44:56 UTC (rev 178232)
</span><span class="lines">@@ -1324,6 +1324,7 @@
</span><span class="cx">         unsigned bodyEndColumn = endColumnIsOnStartLine ?
</span><span class="cx">             endLocation.startOffset - m_token.m_data.lineStartOffset :
</span><span class="cx">             endLocation.startOffset - endLocation.lineStartOffset;
</span><ins>+        unsigned currentLineStartOffset = m_token.m_location.lineStartOffset;
</ins><span class="cx"> 
</span><span class="cx">         body = context.createFunctionBody(startLocation, endLocation, bodyStartColumn, bodyEndColumn, cachedInfo-&gt;strictMode);
</span><span class="cx">         
</span><span class="lines">@@ -1334,6 +1335,8 @@
</span><span class="cx"> 
</span><span class="cx">         context.setFunctionNameStart(body, functionNameStart);
</span><span class="cx">         m_token = cachedInfo-&gt;closeBraceToken();
</span><ins>+        if (endColumnIsOnStartLine)
+            m_token.m_location.lineStartOffset = currentLineStartOffset;
</ins><span class="cx"> 
</span><span class="cx">         m_lexer-&gt;setOffset(m_token.m_location.endOffset, m_token.m_location.lineStartOffset);
</span><span class="cx">         m_lexer-&gt;setLineNumber(m_token.m_location.line);
</span></span></pre>
</div>
</div>

</body>
</html>