Diff
Modified: trunk/LayoutTests/ChangeLog (176816 => 176817)
--- trunk/LayoutTests/ChangeLog 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/LayoutTests/ChangeLog 2014-12-04 22:20:08 UTC (rev 176817)
@@ -1,3 +1,16 @@
+2014-12-02 Brian J. Burg <[email protected]>
+
+ Web Inspector: timeline probe records have inaccurate per-probe hit counts
+ https://bugs.webkit.org/show_bug.cgi?id=138976
+
+ Reviewed by Joseph Pecoraro.
+ Patch by Katie Madonna <[email protected]>
+
+ Update test to also cover expected probe sampleId behavior.
+
+ * inspector-protocol/debugger/didSampleProbe-multiple-probes-expected.txt:
+ * inspector-protocol/debugger/didSampleProbe-multiple-probes.html:
+
2014-12-04 Adenilson Cavalcanti <[email protected]>
Groove/inset/outset borders show solid if the color is black
Modified: trunk/LayoutTests/inspector-protocol/debugger/didSampleProbe-multiple-probes-expected.txt (176816 => 176817)
--- trunk/LayoutTests/inspector-protocol/debugger/didSampleProbe-multiple-probes-expected.txt 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/LayoutTests/inspector-protocol/debugger/didSampleProbe-multiple-probes-expected.txt 2014-12-04 22:20:08 UTC (rev 176817)
@@ -3,8 +3,12 @@
Found breakpoint.js
Running breakpointActions to trigger probe samples.
inside breakpointActions a:(12) b:([object Object])
+inside breakpointActions a:(12) b:([object Object])
Received probe sample payload: {"type":"number","value":12,"description":"12"}
Received probe sample payload: {"type":"number","value":12,"description":"12"}
+Received probe sample payload: {"type":"number","value":12,"description":"12"}
+Received probe sample payload: {"type":"number","value":12,"description":"12"}
PASS: Samples from different probe actions should have unique action identifiers.
PASS: Samples from probe actions on the same breakpoint should have the same batch identifiers.
+PASS: SampleIds from a any probe are sequential and start counting from one.
Modified: trunk/LayoutTests/inspector-protocol/debugger/didSampleProbe-multiple-probes.html (176816 => 176817)
--- trunk/LayoutTests/inspector-protocol/debugger/didSampleProbe-multiple-probes.html 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/LayoutTests/inspector-protocol/debugger/didSampleProbe-multiple-probes.html 2014-12-04 22:20:08 UTC (rev 176817)
@@ -10,16 +10,15 @@
InspectorTest.sendCommand("Debugger.enable", {});
var samples = [];
- const expectedSampleCount = 2;
+ const expectedSampleCount = 4;
function receivedAllExpectedSamples() {
return samples.length === expectedSampleCount;
}
- function dumpSamples()
- {
- InspectorTest.log("Sample 1: " + JSON.stringify(samples[0]));
- InspectorTest.log("Sample 2: " + JSON.stringify(samples[1]));
+ function dumpSamples() {
+ for (var i = 0; i < samples.length; i++)
+ InspectorTest.log("Sample " + (i + 1) + ": " + JSON.stringify(samples[i]));
}
var tests = [
@@ -37,6 +36,13 @@
},
error: dumpSamples
},
+ {
+ message: "SampleIds from a any probe are sequential and start counting from one.",
+ predicate: function samplesHaveSequentialIds() {
+ return samples.every(function(sample, idx) { return sample.sampleId === idx + 1; });
+ },
+ error: dumpSamples
+ },
];
InspectorTest.eventHandler["Debugger.scriptParsed"] = function(messageObject)
@@ -58,6 +64,7 @@
InspectorTest.log("Running breakpointActions to trigger probe samples.");
InspectorTest.sendCommand("Runtime.evaluate", {_expression_: "breakpointActions(12, {x:1,y:2})"});
+ InspectorTest.sendCommand("Runtime.evaluate", {_expression_: "breakpointActions(12, {x:1,y:2})"});
});
}
}
Modified: trunk/Source/_javascript_Core/ChangeLog (176816 => 176817)
--- trunk/Source/_javascript_Core/ChangeLog 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-12-04 22:20:08 UTC (rev 176817)
@@ -1,3 +1,29 @@
+2014-12-02 Brian J. Burg <[email protected]>
+
+ Web Inspector: timeline probe records have inaccurate per-probe hit counts
+ https://bugs.webkit.org/show_bug.cgi?id=138976
+
+ Reviewed by Joseph Pecoraro.
+
+ Previously, the DebuggerAgent was responsible for assigning unique ids to samples.
+ However, this makes it impossible for the frontend's Timeline manager to associate
+ a Probe Sample timeline record with the corresponding probe sample data. The record
+ only included the probe batchId (misnamed as hitCount in ScriptDebugServer).
+
+ This patch moves both the batchId and sampleId counters into ScriptDebugServer, so
+ any client of ScriptDebugListener will get the correct sampleId for each sample.
+
+ * inspector/ScriptDebugListener.h:
+ * inspector/ScriptDebugServer.cpp:
+ (Inspector::ScriptDebugServer::ScriptDebugServer):
+ (Inspector::ScriptDebugServer::dispatchBreakpointActionProbe):
+ (Inspector::ScriptDebugServer::handleBreakpointHit):
+ * inspector/ScriptDebugServer.h:
+ * inspector/agents/InspectorDebuggerAgent.cpp:
+ (Inspector::InspectorDebuggerAgent::InspectorDebuggerAgent):
+ (Inspector::InspectorDebuggerAgent::breakpointActionProbe):
+ * inspector/agents/InspectorDebuggerAgent.h:
+
2014-12-04 Oliver Hunt <[email protected]>
Serialization of MapData object provides unsafe access to internal types
Modified: trunk/Source/_javascript_Core/inspector/ScriptDebugListener.h (176816 => 176817)
--- trunk/Source/_javascript_Core/inspector/ScriptDebugListener.h 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/_javascript_Core/inspector/ScriptDebugListener.h 2014-12-04 22:20:08 UTC (rev 176817)
@@ -75,7 +75,7 @@
virtual void breakpointActionLog(JSC::ExecState*, const String&) = 0;
virtual void breakpointActionSound(int breakpointActionIdentifier) = 0;
- virtual void breakpointActionProbe(JSC::ExecState*, const ScriptBreakpointAction&, int hitCount, const Deprecated::ScriptValue& result) = 0;
+ virtual void breakpointActionProbe(JSC::ExecState*, const ScriptBreakpointAction&, unsigned batchId, unsigned sampleId, const Deprecated::ScriptValue& result) = 0;
};
} // namespace Inspector
Modified: trunk/Source/_javascript_Core/inspector/ScriptDebugServer.cpp (176816 => 176817)
--- trunk/Source/_javascript_Core/inspector/ScriptDebugServer.cpp 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/_javascript_Core/inspector/ScriptDebugServer.cpp 2014-12-04 22:20:08 UTC (rev 176817)
@@ -50,8 +50,6 @@
ScriptDebugServer::ScriptDebugServer(bool isInWorkerThread)
: Debugger(isInWorkerThread)
- , m_doneProcessingDebuggerEvents(true)
- , m_callingListeners(false)
{
}
@@ -176,7 +174,7 @@
listener->breakpointActionSound(breakpointActionIdentifier);
}
-void ScriptDebugServer::dispatchBreakpointActionProbe(ExecState* exec, const ScriptBreakpointAction& action, const Deprecated::ScriptValue& sample)
+void ScriptDebugServer::dispatchBreakpointActionProbe(ExecState* exec, const ScriptBreakpointAction& action, const Deprecated::ScriptValue& sampleValue)
{
if (m_callingListeners)
return;
@@ -187,10 +185,12 @@
TemporaryChange<bool> change(m_callingListeners, true);
+ unsigned sampleId = m_nextProbeSampleId++;
+
Vector<ScriptDebugListener*> listenersCopy;
copyToVector(listeners, listenersCopy);
for (auto* listener : listenersCopy)
- listener->breakpointActionProbe(exec, action, m_hitCount, sample);
+ listener->breakpointActionProbe(exec, action, m_currentProbeBatchId, sampleId, sampleValue);
}
void ScriptDebugServer::dispatchDidContinue(ScriptDebugListener* listener)
@@ -288,7 +288,7 @@
void ScriptDebugServer::handleBreakpointHit(const JSC::Breakpoint& breakpoint)
{
- m_hitCount++;
+ m_currentProbeBatchId++;
BreakpointIDToActionsMap::iterator it = m_breakpointIDToActions.find(breakpoint.id);
if (it != m_breakpointIDToActions.end()) {
BreakpointActions& actions = it->value;
Modified: trunk/Source/_javascript_Core/inspector/ScriptDebugServer.h (176816 => 176817)
--- trunk/Source/_javascript_Core/inspector/ScriptDebugServer.h 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/_javascript_Core/inspector/ScriptDebugServer.h 2014-12-04 22:20:08 UTC (rev 176817)
@@ -93,7 +93,7 @@
void dispatchBreakpointActionSound(JSC::ExecState*, int breakpointActionIdentifier);
void dispatchBreakpointActionProbe(JSC::ExecState*, const ScriptBreakpointAction&, const Deprecated::ScriptValue& sample);
- bool m_doneProcessingDebuggerEvents;
+ bool m_doneProcessingDebuggerEvents {true};
private:
typedef HashMap<JSC::BreakpointID, BreakpointActions> BreakpointIDToActionsMap;
@@ -107,9 +107,12 @@
Deprecated::ScriptValue exceptionOrCaughtValue(JSC::ExecState*);
- unsigned m_hitCount;
- bool m_callingListeners;
+ bool m_callingListeners {false};
+
BreakpointIDToActionsMap m_breakpointIDToActions;
+
+ unsigned m_nextProbeSampleId {1};
+ unsigned m_currentProbeBatchId {0};
};
} // namespace Inspector
Modified: trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.cpp (176816 => 176817)
--- trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.cpp 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.cpp 2014-12-04 22:20:08 UTC (rev 176817)
@@ -60,13 +60,7 @@
InspectorDebuggerAgent::InspectorDebuggerAgent(InjectedScriptManager* injectedScriptManager)
: InspectorAgentBase(ASCIILiteral("Debugger"))
, m_injectedScriptManager(injectedScriptManager)
- , m_listener(nullptr)
- , m_pausedScriptState(nullptr)
, m_continueToLocationBreakpointID(JSC::noBreakpointID)
- , m_enabled(false)
- , m_javaScriptPauseScheduled(false)
- , m_hasExceptionValue(false)
- , m_nextProbeSampleId(1)
{
// FIXME: make breakReason optional so that there was no need to init it with "other".
clearBreakDetails();
@@ -667,16 +661,14 @@
m_frontendDispatcher->playBreakpointActionSound(breakpointActionIdentifier);
}
-void InspectorDebuggerAgent::breakpointActionProbe(JSC::ExecState* scriptState, const ScriptBreakpointAction& action, int hitCount, const Deprecated::ScriptValue& sample)
+void InspectorDebuggerAgent::breakpointActionProbe(JSC::ExecState* scriptState, const ScriptBreakpointAction& action, unsigned batchId, unsigned sampleId, const Deprecated::ScriptValue& sample)
{
- int sampleId = m_nextProbeSampleId++;
-
InjectedScript injectedScript = m_injectedScriptManager->injectedScriptFor(scriptState);
RefPtr<Protocol::Runtime::RemoteObject> payload = injectedScript.wrapObject(sample, objectGroupForBreakpointAction(action));
RefPtr<Protocol::Debugger::ProbeSample> result = Protocol::Debugger::ProbeSample::create()
.setProbeId(action.identifier)
+ .setBatchId(batchId)
.setSampleId(sampleId)
- .setBatchId(hitCount)
.setTimestamp(m_injectedScriptManager->inspectorEnvironment().executionStopwatch()->elapsedTime())
.setPayload(payload.release());
Modified: trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.h (176816 => 176817)
--- trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.h 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/_javascript_Core/inspector/agents/InspectorDebuggerAgent.h 2014-12-04 22:20:08 UTC (rev 176817)
@@ -138,7 +138,7 @@
virtual void failedToParseSource(const String& url, const String& data, int firstLine, int errorLine, const String& errorMessage) override final;
virtual void breakpointActionSound(int breakpointActionIdentifier) override;
- virtual void breakpointActionProbe(JSC::ExecState*, const ScriptBreakpointAction&, int hitCount, const Deprecated::ScriptValue& sample) override final;
+ virtual void breakpointActionProbe(JSC::ExecState*, const ScriptBreakpointAction&, unsigned batchId, unsigned sampleId, const Deprecated::ScriptValue& sample) override final;
PassRefPtr<Inspector::Protocol::Debugger::Location> resolveBreakpoint(const String& breakpointIdentifier, JSC::SourceID, const ScriptBreakpoint&);
bool assertPaused(ErrorString&);
@@ -156,8 +156,8 @@
InjectedScriptManager* m_injectedScriptManager;
std::unique_ptr<InspectorDebuggerFrontendDispatcher> m_frontendDispatcher;
RefPtr<InspectorDebuggerBackendDispatcher> m_backendDispatcher;
- Listener* m_listener;
- JSC::ExecState* m_pausedScriptState;
+ Listener* m_listener {nullptr};
+ JSC::ExecState* m_pausedScriptState {nullptr};
Deprecated::ScriptValue m_currentCallStack;
ScriptsMap m_scripts;
BreakpointIdentifierToDebugServerBreakpointIDsMap m_breakpointIdentifierToDebugServerBreakpointIDs;
@@ -165,11 +165,10 @@
JSC::BreakpointID m_continueToLocationBreakpointID;
InspectorDebuggerFrontendDispatcher::Reason m_breakReason;
RefPtr<InspectorObject> m_breakAuxData;
- bool m_enabled;
- bool m_javaScriptPauseScheduled;
- bool m_hasExceptionValue;
+ bool m_enabled {false};
+ bool m_javaScriptPauseScheduled {false};
+ bool m_hasExceptionValue {false};
RefPtr<WTF::Stopwatch> m_stopwatch;
- int m_nextProbeSampleId;
};
} // namespace Inspector
Modified: trunk/Source/WebCore/ChangeLog (176816 => 176817)
--- trunk/Source/WebCore/ChangeLog 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/WebCore/ChangeLog 2014-12-04 22:20:08 UTC (rev 176817)
@@ -1,3 +1,20 @@
+2014-12-02 Brian J. Burg <[email protected]>
+
+ Web Inspector: timeline probe records have inaccurate per-probe hit counts
+ https://bugs.webkit.org/show_bug.cgi?id=138976
+
+ Reviewed by Joseph Pecoraro.
+
+ Update the signature for breakpointActionProbe to take batchId and sampleId.
+ Covered by existing test inspector-protocol/debugger/didSampleProbe-multiple-probes.html.
+
+ * inspector/InspectorTimelineAgent.cpp:
+ (WebCore::InspectorTimelineAgent::breakpointActionProbe):
+ * inspector/InspectorTimelineAgent.h:
+ * inspector/TimelineRecordFactory.cpp:
+ (WebCore::TimelineRecordFactory::createProbeSampleData):
+ * inspector/TimelineRecordFactory.h:
+
2014-12-04 Adenilson Cavalcanti <[email protected]>
Groove/inset/outset borders show solid if the color is black
Modified: trunk/Source/WebCore/inspector/InspectorTimelineAgent.cpp (176816 => 176817)
--- trunk/Source/WebCore/inspector/InspectorTimelineAgent.cpp 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/WebCore/inspector/InspectorTimelineAgent.cpp 2014-12-04 22:20:08 UTC (rev 176817)
@@ -421,7 +421,7 @@
if (frame && m_callStackDepth) {
--m_callStackDepth;
ASSERT(m_callStackDepth >= 0);
-
+
if (!m_callStackDepth) {
if (m_recordStack.isEmpty())
return;
@@ -552,11 +552,12 @@
// ScriptDebugListener
-void InspectorTimelineAgent::breakpointActionProbe(JSC::ExecState* exec, const Inspector::ScriptBreakpointAction& action, int hitCount, const Deprecated::ScriptValue&)
+void InspectorTimelineAgent::breakpointActionProbe(JSC::ExecState* exec, const Inspector::ScriptBreakpointAction& action, unsigned batchId, unsigned sampleId, const Deprecated::ScriptValue&)
{
+ UNUSED_PARAM(batchId);
ASSERT(exec);
- appendRecord(TimelineRecordFactory::createProbeSampleData(action, hitCount), TimelineRecordType::ProbeSample, false, frameFromExecState(exec));
+ appendRecord(TimelineRecordFactory::createProbeSampleData(action, sampleId), TimelineRecordType::ProbeSample, false, frameFromExecState(exec));
}
static Inspector::Protocol::Timeline::EventType toProtocol(TimelineRecordType type)
Modified: trunk/Source/WebCore/inspector/InspectorTimelineAgent.h (176816 => 176817)
--- trunk/Source/WebCore/inspector/InspectorTimelineAgent.h 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/WebCore/inspector/InspectorTimelineAgent.h 2014-12-04 22:20:08 UTC (rev 176817)
@@ -214,7 +214,7 @@
virtual void breakpointActionLog(JSC::ExecState*, const String&) override { }
virtual void breakpointActionSound(int) override { }
- virtual void breakpointActionProbe(JSC::ExecState*, const Inspector::ScriptBreakpointAction&, int hitCount, const Deprecated::ScriptValue& result) override;
+ virtual void breakpointActionProbe(JSC::ExecState*, const Inspector::ScriptBreakpointAction&, unsigned batchId, unsigned sampleId, const Deprecated::ScriptValue& result) override;
private:
friend class TimelineRecordStack;
Modified: trunk/Source/WebCore/inspector/TimelineRecordFactory.cpp (176816 => 176817)
--- trunk/Source/WebCore/inspector/TimelineRecordFactory.cpp 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/WebCore/inspector/TimelineRecordFactory.cpp 2014-12-04 22:20:08 UTC (rev 176817)
@@ -95,11 +95,11 @@
return data.release();
}
-PassRefPtr<InspectorObject> TimelineRecordFactory::createProbeSampleData(const ScriptBreakpointAction& action, int hitCount)
+PassRefPtr<InspectorObject> TimelineRecordFactory::createProbeSampleData(const ScriptBreakpointAction& action, unsigned sampleId)
{
RefPtr<InspectorObject> data = ""
data->setInteger(ASCIILiteral("probeId"), action.identifier);
- data->setInteger(ASCIILiteral("hitCount"), hitCount);
+ data->setInteger(ASCIILiteral("sampleId"), sampleId);
return data.release();
}
Modified: trunk/Source/WebCore/inspector/TimelineRecordFactory.h (176816 => 176817)
--- trunk/Source/WebCore/inspector/TimelineRecordFactory.h 2014-12-04 22:01:30 UTC (rev 176816)
+++ trunk/Source/WebCore/inspector/TimelineRecordFactory.h 2014-12-04 22:20:08 UTC (rev 176817)
@@ -63,7 +63,7 @@
static PassRefPtr<Inspector::InspectorObject> createFunctionCallData(const String& scriptName, int scriptLine);
static PassRefPtr<Inspector::InspectorObject> createConsoleProfileData(const String& title);
- static PassRefPtr<Inspector::InspectorObject> createProbeSampleData(const Inspector::ScriptBreakpointAction&, int hitCount);
+ static PassRefPtr<Inspector::InspectorObject> createProbeSampleData(const Inspector::ScriptBreakpointAction&, unsigned sampleId);
static PassRefPtr<Inspector::InspectorObject> createEventDispatchData(const Event&);
