Web Inspector: FunctionCall timeline records omit profile data if the debugger has...
authorburg@cs.washington.edu <burg@cs.washington.edu@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Sep 2014 02:29:50 +0000 (02:29 +0000)
committerburg@cs.washington.edu <burg@cs.washington.edu@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Sep 2014 02:29:50 +0000 (02:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136805

Reviewed by Timothy Hatcher.

Source/WebCore:

TimelineAgent was mismanaging its call stack depth counter, which caused nested FunctionCall
records to steal the parent FunctionCall's captured profile in the child's didCallFunction().
Thus, the top FunctionCall node had no profile data and nested FunctionCall nodes each had
their own profiles. The frontend expected just one profile, so it didn't show anything when
it couldn't be found.

Test: inspector/timeline/debugger-paused-while-recording.html

* inspector/InspectorTimelineAgent.cpp: Rename m_recordingProfileDepth to m_callStackDepth.
(WebCore::InspectorTimelineAgent::willCallFunction): Fix the call stack depth management.
(WebCore::InspectorTimelineAgent::didCallFunction):
(WebCore::InspectorTimelineAgent::willEvaluateScript):
(WebCore::InspectorTimelineAgent::didEvaluateScript):
(WebCore::InspectorTimelineAgent::InspectorTimelineAgent):
* inspector/InspectorTimelineAgent.h:

Source/WebInspectorUI:

* UserInterface/Test.html: Add missing include for ScopeChainNode.js.

LayoutTests:

Add a test to see that script timeline records contain profiles even when
the debugger pauses during timeline capturing.

* inspector/timeline/debugger-paused-while-recording-expected.txt: Added.
* inspector/timeline/debugger-paused-while-recording.html: Added.
* inspector/timeline/resources/timeline-helper.js: Added.
(callFunction):
(hook):

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

LayoutTests/ChangeLog
LayoutTests/inspector/timeline/debugger-paused-while-recording-expected.txt [new file with mode: 0644]
LayoutTests/inspector/timeline/debugger-paused-while-recording.html [new file with mode: 0644]
LayoutTests/inspector/timeline/resources/timeline-helper.js [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorTimelineAgent.cpp
Source/WebCore/inspector/InspectorTimelineAgent.h
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Test.html

index e04934320f79d57641b59f04763d743086f75736..01aeef09acd89543f02b3ee23099220f404248e4 100644 (file)
@@ -1,3 +1,19 @@
+2014-09-25  Brian J. Burg  <burg@cs.washington.edu>
+
+        Web Inspector: FunctionCall timeline records omit profile data if the debugger has paused
+        https://bugs.webkit.org/show_bug.cgi?id=136805
+
+        Reviewed by Timothy Hatcher.
+
+        Add a test to see that script timeline records contain profiles even when
+        the debugger pauses during timeline capturing.
+
+        * inspector/timeline/debugger-paused-while-recording-expected.txt: Added.
+        * inspector/timeline/debugger-paused-while-recording.html: Added.
+        * inspector/timeline/resources/timeline-helper.js: Added.
+        (callFunction):
+        (hook):
+
 2014-09-25  Brian J. Burg  <burg@cs.washington.edu>
 
         StorageTracker::deleteOrigin being called off the main thread (ASSERTs in inspector/test-harness-trivially-works.html test)
diff --git a/LayoutTests/inspector/timeline/debugger-paused-while-recording-expected.txt b/LayoutTests/inspector/timeline/debugger-paused-while-recording-expected.txt
new file mode 100644 (file)
index 0000000..c01db32
--- /dev/null
@@ -0,0 +1,8 @@
+Testing that profiling data is correctly generated and attached to Timeline records when the debugger pauses and resumes while capturing timelines.
+
+Added a breakpoint inside hook().
+Debugger paused; resuming...
+Debugger resumed; stopping timeline capture.
+Timeline capturing stopped. Inspecting the active recording....
+TimerFired timeline record has profile attached: TRUE
+
diff --git a/LayoutTests/inspector/timeline/debugger-paused-while-recording.html b/LayoutTests/inspector/timeline/debugger-paused-while-recording.html
new file mode 100644 (file)
index 0000000..03deb56
--- /dev/null
@@ -0,0 +1,82 @@
+<!doctype html>
+<html>
+<head>
+<meta http-equiv="Content-Security-Policy" content="script-src 'self' 'unsafe-inline'">
+<script type="text/javascript" src="../../http/tests/inspector/inspector-test.js"></script>
+<script type="text/javascript" src="./resources/timeline-helper.js"></script>
+<script>
+function installTimer()
+{
+    setTimeout(function() {
+        callFunction(mul, add(1, 3), 3);
+        hook();
+    });
+}
+
+function add(a, b)
+{
+    InspectorTestProxy.addResult("Calling add(): " + a + " + " + b);
+    return a + b;
+}
+
+function mul(a, b)
+{
+    InspectorTestProxy.addResult("Calling mul(): " + a + " * " + b);
+    return a * b;
+}
+
+function test()
+{
+    // First, set up the breakpoint, start timeline capturing, and trigger execution of installTimer().
+    WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.ScriptAdded, function(event) {
+        var scriptObject = event.data.script;
+
+        if (!/timeline-helper\.js$/.test(scriptObject.url))
+            return;
+
+        var location = scriptObject.createSourceCodeLocation(17, 0);  // Inside timeline-helper.js:hook()
+        var breakpoint = new WebInspector.Breakpoint(location);
+        WebInspector.debuggerManager.addBreakpoint(breakpoint);
+        InspectorTest.addResult("Added a breakpoint inside hook().")
+
+        WebInspector.timelineManager.startCapturing();
+        InspectorTest.evaluateInPage("installTimer()");
+    });
+
+    // Second, the debugger will pause during timeline capturing. Resume, then stop timeline capturing.
+    WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.Paused, function(event) {
+            InspectorTest.addResult("Debugger paused; resuming...");
+
+            WebInspector.debuggerManager.resume().then(function() {
+                InspectorTest.addResult("Debugger resumed; stopping timeline capture.");
+                WebInspector.timelineManager.stopCapturing();
+            })
+    });
+
+    // When timeline capturing stops, inspect the resulting timeline records for a profile.
+    WebInspector.timelineManager.addEventListener(WebInspector.TimelineManager.Event.CapturingStopped, function(event) {
+        var recording = WebInspector.timelineManager.activeRecording;
+        var scriptTimeline = recording.timelines.get(WebInspector.TimelineRecord.Type.Script);
+        console.assert(scriptTimeline);
+
+        InspectorTest.addResult("Timeline capturing stopped. Inspecting the active recording....");
+
+        for (var record of scriptTimeline.records) {
+            if (record.eventType !== WebInspector.ScriptTimelineRecord.EventType.TimerFired)
+                continue;
+
+            var result = record.profile ? "TRUE" : "FALSE";
+            InspectorTest.addResult("TimerFired timeline record has profile attached: " + result);
+        }
+
+        InspectorTest.completeTest();
+    });
+
+    InspectorTest.reloadPage();
+}
+</script>
+</head>
+<body onload="runTest()">
+    <p>Testing that profiling data is correctly generated and attached to Timeline records when the debugger pauses and resumes while capturing timelines.</p>
+</body>
+</html>
diff --git a/LayoutTests/inspector/timeline/resources/timeline-helper.js b/LayoutTests/inspector/timeline/resources/timeline-helper.js
new file mode 100644 (file)
index 0000000..c2eaf58
--- /dev/null
@@ -0,0 +1,19 @@
+// WARNING: some tests blindly set breakpoints in this file by line number.
+// So, if you modify the code, make sure to adjust any createSourceCodeLocation
+// calls from tests in the ../ directory. Callsites should include a description
+// of the function/statement to set a breakpoint at, so that it's easy to fix them.
+
+function callFunction(fn)
+{
+    if (!(fn instanceof Function))
+        return;
+
+    var argsArray = Array.prototype.slice.call(arguments);
+    Array.prototype.splice.call(argsArray, 0, 1);
+    fn.call(this, argsArray);
+}
+
+function hook()
+{
+    return 42;
+}
index 58935a76d94217ae48c5ccc3ddc8c117d10120f4..d3d274bf57b88877c15c31e3343f30aac699d05c 100644 (file)
@@ -1,3 +1,26 @@
+2014-09-25  Brian J. Burg  <burg@cs.washington.edu>
+
+        Web Inspector: FunctionCall timeline records omit profile data if the debugger has paused
+        https://bugs.webkit.org/show_bug.cgi?id=136805
+
+        Reviewed by Timothy Hatcher.
+
+        TimelineAgent was mismanaging its call stack depth counter, which caused nested FunctionCall
+        records to steal the parent FunctionCall's captured profile in the child's didCallFunction().
+        Thus, the top FunctionCall node had no profile data and nested FunctionCall nodes each had
+        their own profiles. The frontend expected just one profile, so it didn't show anything when
+        it couldn't be found.
+
+        Test: inspector/timeline/debugger-paused-while-recording.html
+
+        * inspector/InspectorTimelineAgent.cpp: Rename m_recordingProfileDepth to m_callStackDepth.
+        (WebCore::InspectorTimelineAgent::willCallFunction): Fix the call stack depth management.
+        (WebCore::InspectorTimelineAgent::didCallFunction):
+        (WebCore::InspectorTimelineAgent::willEvaluateScript):
+        (WebCore::InspectorTimelineAgent::didEvaluateScript):
+        (WebCore::InspectorTimelineAgent::InspectorTimelineAgent):
+        * inspector/InspectorTimelineAgent.h:
+
 2014-09-25  Brian J. Burg  <burg@cs.washington.edu>
 
         StorageTracker::deleteOrigin being called off the main thread (ASSERTs in inspector/test-harness-trivially-works.html test)
index 016d678ed50c845919a8571f006a5719a62d3d88..807dd2865e7da884ec923b3aa3bf53b3303d6762 100644 (file)
@@ -231,19 +231,19 @@ void InspectorTimelineAgent::willCallFunction(const String& scriptName, int scri
 {
     pushCurrentRecord(TimelineRecordFactory::createFunctionCallData(scriptName, scriptLine), TimelineRecordType::FunctionCall, true, frame);
 
-    if (frame && !m_recordingProfileDepth) {
-        ++m_recordingProfileDepth;
+    if (frame && !m_callStackDepth)
         startProfiling(frame, ASCIILiteral("Timeline FunctionCall"));
-    }
+
+    ++m_callStackDepth;
 }
 
 void InspectorTimelineAgent::didCallFunction(Frame* frame)
 {
-    if (frame && m_recordingProfileDepth) {
-        --m_recordingProfileDepth;
-        ASSERT(m_recordingProfileDepth >= 0);
+    if (frame && m_callStackDepth) {
+        --m_callStackDepth;
+        ASSERT(m_callStackDepth >= 0);
 
-        if (!m_recordingProfileDepth) {
+        if (!m_callStackDepth) {
             if (m_recordStack.isEmpty())
                 return;
 
@@ -405,19 +405,19 @@ void InspectorTimelineAgent::willEvaluateScript(const String& url, int lineNumbe
 {
     pushCurrentRecord(TimelineRecordFactory::createEvaluateScriptData(url, lineNumber), TimelineRecordType::EvaluateScript, true, frame);
 
-    if (frame && !m_recordingProfileDepth) {
-        ++m_recordingProfileDepth;
+    if (frame && !m_callStackDepth) {
+        ++m_callStackDepth;
         startProfiling(frame, ASCIILiteral("Timeline EvaluateScript"));
     }
 }
 
 void InspectorTimelineAgent::didEvaluateScript(Frame* frame)
 {
-    if (frame && m_recordingProfileDepth) {
-        --m_recordingProfileDepth;
-        ASSERT(m_recordingProfileDepth >= 0);
+    if (frame && m_callStackDepth) {
+        --m_callStackDepth;
+        ASSERT(m_callStackDepth >= 0);
         
-        if (!m_recordingProfileDepth) {
+        if (!m_callStackDepth) {
             if (m_recordStack.isEmpty())
                 return;
 
@@ -691,10 +691,10 @@ InspectorTimelineAgent::InspectorTimelineAgent(InstrumentingAgents* instrumentin
     , m_pageAgent(pageAgent)
     , m_scriptDebugServer(nullptr)
     , m_id(1)
+    , m_callStackDepth(0)
     , m_maxCallStackDepth(5)
     , m_inspectorType(type)
     , m_client(client)
-    , m_recordingProfileDepth(0)
     , m_enabled(false)
     , m_enabledFromFrontend(false)
 {
index 1fca5ee14df8b278b88629cedac3b4e7764303e2..524f9728cc055b5c53cd3e95fa981cded10070e3 100644 (file)
@@ -279,13 +279,13 @@ private:
     Vector<TimelineRecordEntry> m_recordStack;
 
     int m_id;
+    int m_callStackDepth;
     int m_maxCallStackDepth;
     InspectorType m_inspectorType;
     InspectorClient* m_client;
 
     Vector<TimelineRecordEntry> m_pendingConsoleProfileRecords;
 
-    int m_recordingProfileDepth;
     bool m_enabled;
     bool m_enabledFromFrontend;
 };
index 6daf732afc0eea8e5e4042ef06897359ef8d6d42..4efdc395a0c1f38766ab9cb152fad7b801ddb87e 100644 (file)
@@ -1,3 +1,12 @@
+2014-09-25  Brian J. Burg  <burg@cs.washington.edu>
+
+        Web Inspector: FunctionCall timeline records omit profile data if the debugger has paused
+        https://bugs.webkit.org/show_bug.cgi?id=136805
+
+        Reviewed by Timothy Hatcher.
+
+        * UserInterface/Test.html: Add missing include for ScopeChainNode.js.
+
 2014-09-25  Brian J. Burg  <burg@cs.washington.edu>
 
         Web Inspector: sort probe details sidebar sections by source code location string
index fa909505e4e4611eff50ed9fbc02850039926db7..4dc1e87d674a44ad92c580e64d37d5c8ccd92971 100644 (file)
     <script src="Models/ResourceCollection.js"></script>
     <script src="Models/ResourceTimelineRecord.js"></script>
     <script src="Models/Revision.js"></script>
+    <script src="Models/ScopeChainNode.js"></script>
     <script src="Models/Script.js"></script>
     <script src="Models/ScriptSyntaxTree.js"></script>
     <script src="Models/ScriptTimelineRecord.js"></script>