Skip to content

Commit 2f208b1

Browse files
JosephPecoraroJosephPecoraro
committed
Web Inspector: Wrong function name next to scope
https://bugs.webkit.org/show_bug.cgi?id=158210 <rdar://problem/26543093> Reviewed by Brian Burg. Source/JavaScriptCore: * CMakeLists.txt: * JavaScriptCore.xcodeproj/project.pbxproj: Add DebuggerLocation. A helper for describing a unique location. * bytecode/CodeBlock.cpp: (JSC::CodeBlock::setConstantRegisters): When compiled with debug info, add a SymbolTable rare data pointer back to the CodeBlock. This will be used later to get JSScope debug info if Web Inspector pauses. * runtime/SymbolTable.h: * runtime/SymbolTable.cpp: (JSC::SymbolTable::cloneScopePart): (JSC::SymbolTable::prepareForTypeProfiling): (JSC::SymbolTable::uniqueIDForVariable): (JSC::SymbolTable::uniqueIDForOffset): (JSC::SymbolTable::globalTypeSetForOffset): (JSC::SymbolTable::globalTypeSetForVariable): Rename rareData and include a CodeBlock pointer. (JSC::SymbolTable::rareDataCodeBlock): (JSC::SymbolTable::setRareDataCodeBlock): Setter and getter for the rare data. It should only be set once. (JSC::SymbolTable::visitChildren): Visit the rare data code block if we have one. * debugger/DebuggerLocation.cpp: Added. (JSC::DebuggerLocation::DebuggerLocation): * debugger/DebuggerLocation.h: Added. (JSC::DebuggerLocation::DebuggerLocation): Construction from a ScriptExecutable. * runtime/JSScope.cpp: (JSC::JSScope::symbolTable): * runtime/JSScope.h: * debugger/DebuggerScope.h: * debugger/DebuggerScope.cpp: (JSC::DebuggerScope::name): (JSC::DebuggerScope::location): Name and location for a scope. This uses: JSScope -> SymbolTable -> CodeBlock -> Executable * inspector/protocol/Debugger.json: * inspector/InjectedScriptSource.js: (InjectedScript.CallFrameProxy.prototype._wrapScopeChain): (InjectedScript.CallFrameProxy._createScopeJson): * inspector/JSJavaScriptCallFrame.cpp: (Inspector::valueForScopeType): (Inspector::valueForScopeLocation): (Inspector::JSJavaScriptCallFrame::scopeDescriptions): (Inspector::JSJavaScriptCallFrame::scopeType): Deleted. * inspector/JSJavaScriptCallFrame.h: * inspector/JSJavaScriptCallFramePrototype.cpp: (Inspector::JSJavaScriptCallFramePrototype::finishCreation): (Inspector::jsJavaScriptCallFramePrototypeFunctionScopeDescriptions): (Inspector::jsJavaScriptCallFramePrototypeFunctionScopeType): Deleted. Simplify this code to build the objects we will send across the protocol to descript a Scope. Source/WebInspectorUI: * UserInterface/Controllers/DebuggerManager.js: (WebInspector.DebuggerManager.prototype._scopeChainNodeFromPayload): Include new payload data in the construction call. All the new data is optional, so we gracefully handle legacy backends. * UserInterface/Models/ScopeChainNode.js: (WebInspector.ScopeChainNode): (WebInspector.ScopeChainNode.prototype.get type): (WebInspector.ScopeChainNode.prototype.get objects): (WebInspector.ScopeChainNode.prototype.get name): (WebInspector.ScopeChainNode.prototype.get location): (WebInspector.ScopeChainNode.prototype.get hash): Hash is a rough (name:sourceId:line:column) string for quick comparisons. (WebInspector.ScopeChainNode.prototype.makeLocalScope): Make this an action you take on a scope, to avoid having to do it at construction time, or making it a generic setting. * UserInterface/Views/ScopeChainDetailsSidebarPanel.js: (WebInspector.ScopeChainDetailsSidebarPanel.prototype._generateCallFramesSection): This was wrong before. Move the work to CallFrame and change it to be correct. * UserInterface/CallFrame.js: (WebInspector.CallFrame.prototype.mergedScopeChain): This transforms the scope chain for a call frame from: scope1 scope2 scope3 scope4 scope5 scope6 scope7 foo foo foo bar bar - - Block Closure Closure Closure Closure GLE GBL To: scope1 scope2&3 scope4&5 scope6 scope7 foo foo* bar* - - Block Local Closure GLE GBL Doing a few things: - Merge the first two Closure scopes sharing a location. These are the "var" and "let" Closure scopes in a function, and it is better to present these together in the UI. - Mark the first Closure scope within a function (*). When this is displayed in the UI, we can provide the name of the function: "Closure Scope (name)", and we even have location information that we can use to display a goto arrow if needed. - Make the first Closure scope the Local scope if it matches the Call Frame's function name. This lets us display the section as "Local Variables". LayoutTests: * inspector/debugger/paused-scopes-expected.txt: Added. * inspector/debugger/paused-scopes.html: Added. * inspector/debugger/resources/paused-scopes.js: Added. Test dumps the call frames and scope chains for each call frame when pausing at different locations in a program. Outputting the hashes we can see even identically named functions have different hashes because their location is different. Canonical link: https://commits.webkit.org/177401@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@202659 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 1c5a98a commit 2f208b1

26 files changed

Lines changed: 972 additions & 260 deletions

‎LayoutTests/ChangeLog‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,19 @@
1+
2016-06-29 Joseph Pecoraro <pecoraro@apple.com>
2+
3+
Web Inspector: Wrong function name next to scope
4+
https://bugs.webkit.org/show_bug.cgi?id=158210
5+
<rdar://problem/26543093>
6+
7+
Reviewed by Brian Burg.
8+
9+
* inspector/debugger/paused-scopes-expected.txt: Added.
10+
* inspector/debugger/paused-scopes.html: Added.
11+
* inspector/debugger/resources/paused-scopes.js: Added.
12+
Test dumps the call frames and scope chains for each call frame
13+
when pausing at different locations in a program. Outputting
14+
the hashes we can see even identically named functions have
15+
different hashes because their location is different.
16+
117
2016-06-29 Ryan Haddad <ryanhaddad@apple.com>
218

319
Marking perf/rel-list-remove.html as flaky on ios-simulator

‎LayoutTests/inspector/debugger/paused-scopes-expected.txt‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
Check scope chains for different call frames at different pauses.
2+
3+
4+
== Running test suite: PausedCallFrameScope
5+
-- Running test case: TriggerFirstPause
6+
CALLFRAME: firstPause
7+
8+
---- Scope Chain ----
9+
SCOPE: Name (firstPause) - Type (Block) - Hash (firstPause:<sourceID>:21:29)
10+
- barLexicalVariable2
11+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:21:29)
12+
- barLexicalVariable2
13+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:21:29)
14+
- barVarVariable1
15+
SCOPE: Name (firstPause) - Type (FunctionName) - Hash (firstPause:<sourceID>:21:29)
16+
- firstPause
17+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:19:24)
18+
- fakeFirstPauseLexicalVariable
19+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:19:24)
20+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
21+
- fooLexicalVariable2
22+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
23+
- firstPause
24+
- fooVarVariable1
25+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
26+
- globalLet2
27+
SCOPE: Name () - Type (Global) - Hash ()
28+
29+
---- Merged Scope Chain ----
30+
SCOPE: Name (firstPause) - Type (Block) - Hash (firstPause:<sourceID>:21:29)
31+
- barLexicalVariable2
32+
SCOPE: Name (firstPause) - Type (Local) - Hash (firstPause:<sourceID>:21:29)
33+
- barVarVariable1
34+
- barLexicalVariable2
35+
SCOPE: Name (firstPause) - Type (FunctionName) - Hash (firstPause:<sourceID>:21:29)
36+
- firstPause
37+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:19:24)
38+
- fakeFirstPauseLexicalVariable
39+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
40+
- firstPause
41+
- fooVarVariable1
42+
- fooLexicalVariable2
43+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
44+
- globalLet2
45+
SCOPE: Name () - Type (Global) - Hash ()
46+
47+
CALLFRAME: firstPause
48+
49+
---- Scope Chain ----
50+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:19:24)
51+
- fakeFirstPauseLexicalVariable
52+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:19:24)
53+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
54+
- fooLexicalVariable2
55+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
56+
- firstPause
57+
- fooVarVariable1
58+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
59+
- globalLet2
60+
SCOPE: Name () - Type (Global) - Hash ()
61+
62+
---- Merged Scope Chain ----
63+
SCOPE: Name (firstPause) - Type (Local) - Hash (firstPause:<sourceID>:19:24)
64+
- fakeFirstPauseLexicalVariable
65+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
66+
- firstPause
67+
- fooVarVariable1
68+
- fooLexicalVariable2
69+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
70+
- globalLet2
71+
SCOPE: Name () - Type (Global) - Hash ()
72+
73+
CALLFRAME: entry
74+
75+
---- Scope Chain ----
76+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
77+
- fooLexicalVariable2
78+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
79+
- firstPause
80+
- fooVarVariable1
81+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
82+
- globalLet2
83+
SCOPE: Name () - Type (Global) - Hash ()
84+
85+
---- Merged Scope Chain ----
86+
SCOPE: Name (entry) - Type (Local) - Hash (entry:<sourceID>:14:15)
87+
- firstPause
88+
- fooVarVariable1
89+
- fooLexicalVariable2
90+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
91+
- globalLet2
92+
SCOPE: Name () - Type (Global) - Hash ()
93+
94+
95+
-- Running test case: TriggerSecondPause
96+
CALLFRAME: secondPause
97+
98+
---- Scope Chain ----
99+
SCOPE: Name (secondPause) - Type (Block) - Hash (secondPause:<sourceID>:6:21)
100+
- blockLexicalVariable
101+
SCOPE: Name (secondPause) - Type (Closure) - Hash (secondPause:<sourceID>:6:21)
102+
- shoeLexicalVariable1
103+
SCOPE: Name (secondPause) - Type (Closure) - Hash (secondPause:<sourceID>:6:21)
104+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
105+
- globalLet2
106+
SCOPE: Name () - Type (Global) - Hash ()
107+
108+
---- Merged Scope Chain ----
109+
SCOPE: Name (secondPause) - Type (Block) - Hash (secondPause:<sourceID>:6:21)
110+
- blockLexicalVariable
111+
SCOPE: Name (secondPause) - Type (Local) - Hash (secondPause:<sourceID>:6:21)
112+
- shoeLexicalVariable1
113+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
114+
- globalLet2
115+
SCOPE: Name () - Type (Global) - Hash ()
116+
117+
CALLFRAME: firstPause
118+
119+
---- Scope Chain ----
120+
SCOPE: Name (firstPause) - Type (Block) - Hash (firstPause:<sourceID>:21:29)
121+
- barLexicalVariable2
122+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:21:29)
123+
- barLexicalVariable2
124+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:21:29)
125+
- barVarVariable1
126+
SCOPE: Name (firstPause) - Type (FunctionName) - Hash (firstPause:<sourceID>:21:29)
127+
- firstPause
128+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:19:24)
129+
- fakeFirstPauseLexicalVariable
130+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:19:24)
131+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
132+
- fooLexicalVariable2
133+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
134+
- firstPause
135+
- fooVarVariable1
136+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
137+
- globalLet2
138+
SCOPE: Name () - Type (Global) - Hash ()
139+
140+
---- Merged Scope Chain ----
141+
SCOPE: Name (firstPause) - Type (Block) - Hash (firstPause:<sourceID>:21:29)
142+
- barLexicalVariable2
143+
SCOPE: Name (firstPause) - Type (Local) - Hash (firstPause:<sourceID>:21:29)
144+
- barVarVariable1
145+
- barLexicalVariable2
146+
SCOPE: Name (firstPause) - Type (FunctionName) - Hash (firstPause:<sourceID>:21:29)
147+
- firstPause
148+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:19:24)
149+
- fakeFirstPauseLexicalVariable
150+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
151+
- firstPause
152+
- fooVarVariable1
153+
- fooLexicalVariable2
154+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
155+
- globalLet2
156+
SCOPE: Name () - Type (Global) - Hash ()
157+
158+
CALLFRAME: firstPause
159+
160+
---- Scope Chain ----
161+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:19:24)
162+
- fakeFirstPauseLexicalVariable
163+
SCOPE: Name (firstPause) - Type (Closure) - Hash (firstPause:<sourceID>:19:24)
164+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
165+
- fooLexicalVariable2
166+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
167+
- firstPause
168+
- fooVarVariable1
169+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
170+
- globalLet2
171+
SCOPE: Name () - Type (Global) - Hash ()
172+
173+
---- Merged Scope Chain ----
174+
SCOPE: Name (firstPause) - Type (Local) - Hash (firstPause:<sourceID>:19:24)
175+
- fakeFirstPauseLexicalVariable
176+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
177+
- firstPause
178+
- fooVarVariable1
179+
- fooLexicalVariable2
180+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
181+
- globalLet2
182+
SCOPE: Name () - Type (Global) - Hash ()
183+
184+
CALLFRAME: entry
185+
186+
---- Scope Chain ----
187+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
188+
- fooLexicalVariable2
189+
SCOPE: Name (entry) - Type (Closure) - Hash (entry:<sourceID>:14:15)
190+
- firstPause
191+
- fooVarVariable1
192+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
193+
- globalLet2
194+
SCOPE: Name () - Type (Global) - Hash ()
195+
196+
---- Merged Scope Chain ----
197+
SCOPE: Name (entry) - Type (Local) - Hash (entry:<sourceID>:14:15)
198+
- firstPause
199+
- fooVarVariable1
200+
- fooLexicalVariable2
201+
SCOPE: Name () - Type (GlobalLexicalEnvironment) - Hash ()
202+
- globalLet2
203+
SCOPE: Name () - Type (Global) - Hash ()
204+
205+

‎LayoutTests/inspector/debugger/paused-scopes.html‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
5+
<script src="resources/paused-scopes.js"></script>
6+
<script>
7+
8+
function test()
9+
{
10+
function scopeTypeName(type) {
11+
switch (type) {
12+
case WebInspector.ScopeChainNode.Type.Local: return "Local";
13+
case WebInspector.ScopeChainNode.Type.Global: return "Global";
14+
case WebInspector.ScopeChainNode.Type.GlobalLexicalEnvironment: return "GlobalLexicalEnvironment";
15+
case WebInspector.ScopeChainNode.Type.With: return "With";
16+
case WebInspector.ScopeChainNode.Type.Closure: return "Closure";
17+
case WebInspector.ScopeChainNode.Type.Catch: return "Catch";
18+
case WebInspector.ScopeChainNode.Type.FunctionName: return "FunctionName";
19+
case WebInspector.ScopeChainNode.Type.Block: return "Block";
20+
default: return "Unknown!";
21+
};
22+
}
23+
24+
function sanitizeHash(hash) {
25+
return hash.replace(/:(.*?):/, ":<sourceID>:");
26+
}
27+
28+
function collectScopeChainProperties(scopeChain) {
29+
let scopeChainData = [];
30+
31+
let promises = [];
32+
for (let scope of scopeChain) {
33+
let data = {scope, properties: []};
34+
scopeChainData.push(data);
35+
if (scope.type === WebInspector.ScopeChainNode.Type.Global)
36+
continue;
37+
38+
for (let scopeObject of scope.objects) {
39+
promises.push(new Promise((resolve, reject) => {
40+
scopeObject.getAllPropertyDescriptors((propertyDescriptors) => {
41+
data.properties = data.properties.concat(propertyDescriptors);
42+
resolve();
43+
});
44+
}));
45+
}
46+
}
47+
48+
return Promise.all(promises)
49+
.then(() => scopeChainData);
50+
}
51+
52+
function dumpScopeChainData(scopeChainData) {
53+
for (let {scope, properties} of scopeChainData) {
54+
InspectorTest.log(` SCOPE: Name (${scope.name}) - Type (${scopeTypeName(scope.type)}) - Hash (${sanitizeHash(scope.hash)})`);
55+
for (let propertyDescriptor of properties)
56+
InspectorTest.log(` - ${propertyDescriptor.name}`);
57+
}
58+
}
59+
60+
function dumpCallFrame(callFrame) {
61+
return Promise.all([
62+
collectScopeChainProperties(callFrame.scopeChain),
63+
collectScopeChainProperties(callFrame.mergedScopeChain()),
64+
]).then((results) => {
65+
let [scopeChainData, mergedScopeChainData] = results;
66+
InspectorTest.log(`CALLFRAME: ${callFrame.functionName}`);
67+
InspectorTest.log("\n---- Scope Chain ----");
68+
dumpScopeChainData(scopeChainData);
69+
InspectorTest.log("\n---- Merged Scope Chain ----");
70+
dumpScopeChainData(mergedScopeChainData);
71+
InspectorTest.log("");
72+
});
73+
}
74+
75+
function dumpCallFrames() {
76+
let callFrames = WebInspector.debuggerManager.callFrames;
77+
let chain = Promise.resolve();
78+
for (let callFrame of callFrames)
79+
chain = chain.then(() => dumpCallFrame(callFrame));
80+
return chain;
81+
}
82+
83+
84+
let suite = InspectorTest.createAsyncSuite("PausedCallFrameScope");
85+
86+
suite.addTestCase({
87+
name: "TriggerFirstPause",
88+
description: "Verify CallFrames and Scopes with the first pause.",
89+
test: (resolve, reject) => {
90+
InspectorTest.evaluateInPage("setTimeout(entry)");
91+
WebInspector.debuggerManager.singleFireEventListener(WebInspector.DebuggerManager.Event.CallFramesDidChange, (event) => {
92+
dumpCallFrames().then(resolve, reject);
93+
});
94+
}
95+
});
96+
97+
suite.addTestCase({
98+
name: "TriggerSecondPause",
99+
description: "Verify CallFrames and Scopes with the first pause.",
100+
test: (resolve, reject) => {
101+
WebInspector.debuggerManager.resume();
102+
WebInspector.debuggerManager.singleFireEventListener(WebInspector.DebuggerManager.Event.CallFramesDidChange, (event) => {
103+
dumpCallFrames().then(() => {
104+
WebInspector.debuggerManager.resume();
105+
resolve();
106+
}, reject);
107+
});
108+
}
109+
});
110+
111+
suite.runTestCasesAndFinish();
112+
}
113+
</script>
114+
</head>
115+
<body onload="runTest()">
116+
<p>Check scope chains for different call frames at different pauses.</p>
117+
</body>
118+
</html>

‎LayoutTests/inspector/debugger/resources/paused-scopes.js‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
"use strict";
2+
3+
var globalVar1; // Global (globalVar1)
4+
let globalLet2; // GlobalLexicalEnvironment (globalLet2)
5+
6+
function secondPause() { // Global (secondPause)
7+
let shoeLexicalVariable1 = document; // ClosureScope (shoeLexicalVariable1)
8+
if (true) {
9+
let blockLexicalVariable = "block"; // NestedBlockScope (blockLexicalVariable)
10+
debugger;
11+
}
12+
}
13+
14+
function entry() { // Global (entry)
15+
var fooVarVariable1; // foo ClosureScope (fooVarVariable1)
16+
let fooLexicalVariable2; // foo ClosureScope (fooLexicalVariable2)
17+
firstPause();
18+
19+
function firstPause() { // foo ClosureScope (firstPause)
20+
let fakeFirstPauseLexicalVariable; // firstPause ClosureScope (fakeFirstPauseLexicalVariable)
21+
(function firstPause() {
22+
var barVarVariable1 = window.navigator; // firstPause ClosureScope (barVarVariable1)
23+
let barLexicalVariable2 = window.navigator; // firstPause ClosureScope (barLexicalVariable2)
24+
if (true) {
25+
let barLexicalVariable2 = window.navigator; // NestedBlockScope (barLexicalVariable2)
26+
debugger;
27+
secondPause();
28+
}
29+
})();
30+
}
31+
}

‎Source/JavaScriptCore/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ set(JavaScriptCore_SOURCES
239239

240240
debugger/Debugger.cpp
241241
debugger/DebuggerCallFrame.cpp
242+
debugger/DebuggerLocation.cpp
242243
debugger/DebuggerScope.cpp
243244

244245
dfg/DFGAbstractHeap.cpp

0 commit comments

Comments
 (0)