Report script loads from eval when debugging #3391
Conversation
lib/Jsrt/JsrtDebugManager.cpp
Outdated
| JsrtDebugUtils::AddFileNameOrScriptTypeToObject(eventDataObject, utf8SourceInfo); | ||
| JsrtDebugUtils::AddLineCountToObject(eventDataObject, utf8SourceInfo); | ||
| JsrtDebugUtils::AddPropertyToObject(eventDataObject, JsrtDebugPropertyId::sourceLength, utf8SourceInfo->GetCchLength(), utf8SourceInfo->GetScriptContext()); | ||
|
|
There was a problem hiding this comment.
I see this operations in order and also appear on the other functions in this file. Could you refactor to one and call in the other functions as well?
kfarnung
force-pushed
the
evaldebug
branch
2 times, most recently
from
2b29225
to
26299ec
Compare
lib/Runtime/Debug/DebugContext.h
Outdated
| @@ -60,11 +60,14 @@ namespace Js | |||
|
|
|||
| HostDebugContext * GetHostDebugContext() const { return hostDebugContext; } | |||
|
|
|||
| bool GetIsReparsingSource() { return this->isReparsingSource; } | |||
lib/Runtime/Debug/DebugContext.cpp
Outdated
| @@ -141,6 +142,23 @@ namespace Js | |||
|
|
|||
| HRESULT DebugContext::RundownSourcesAndReparse(bool shouldPerformSourceRundown, bool shouldReparseFunctions) | |||
| { | |||
| struct AutoRestore | |||
There was a problem hiding this comment.
AutoRestore [](start = 15, length = 11)
Rename to AutoRestoreIsReparsingSource
lib/Runtime/Debug/DebugContext.cpp
Outdated
| AutoRestore(DebugContext* debugContext) | ||
| :debugContext(debugContext) | ||
| { | ||
| this->debugContext->isReparsingSource = true; |
There was a problem hiding this comment.
this->debugContext->isReparsingSource = true; [](start = 15, length = 46)
We should restrict this to only when shouldReparseFunctions is true. Not needed during PerformSourceRundown
lib/Jsrt/JsrtDebugManager.cpp
Outdated
| { | ||
| JsrtDebugEventObject debugEventObject(scriptContext); | ||
| Js::DynamicObject* eventDataObject = debugEventObject.GetEventDataObject(); | ||
| JsrtDebugUtils::AddSourceMetadataToObject(eventDataObject, utf8SourceInfo); |
There was a problem hiding this comment.
AddSourceMetadataToObject [](start = 28, length = 25)
When ever we use AddSourceMetadataToObject we also add Scriptid. Move the HasDebugDocument check and AddScriptIdToObject inside AddSourceMetadataToObject
There was a problem hiding this comment.
This is the case here, but not elsewhere in the file. That's why I kept is separate.
lib/Jsrt/JsrtDebugManager.cpp
Outdated
| @@ -154,6 +154,21 @@ HRESULT JsrtDebugManager::DbgRegisterFunction(Js::ScriptContext* scriptContext, | |||
| { | |||
| utf8SourceInfo->SetDebugDocument(debugDocument); | |||
| } | |||
|
|
|||
| if (this->debugEventCallback != nullptr && !scriptContext->GetDebugContext()->GetIsReparsingSource()) | |||
There was a problem hiding this comment.
!scriptContext->GetDebugContext()->GetIsReparsingSource()) [](start = 51, length = 58)
Add a comment here explaining why this check
lib/Jsrt/JsrtDebugManager.cpp
Outdated
| @@ -154,6 +154,18 @@ HRESULT JsrtDebugManager::DbgRegisterFunction(Js::ScriptContext* scriptContext, | |||
| { | |||
| utf8SourceInfo->SetDebugDocument(debugDocument); | |||
| } | |||
|
|
|||
| // Only raise an event in the case where we have a registered callback and we are not | |||
| // currently in the middle of a source reparse (e.g. attaching/detaching). | |||
There was a problem hiding this comment.
middle of a source reparse (e.g. attaching/detaching). [](start = 28, length = 54)
Please explain why we are adding isParsingSource check, will help somebody understand the code/logic when we are not around.
|
|
* Added code to report the load of scripts on eval in order to support stepping into the eval code. * Added protections to prevent re-entrancy when reparsing scripts * Updated debugger test baselines
Merge pull request #3391 from kfarnung:evaldebug * Added code to report the load of scripts on eval in order to support stepping into the eval code. * Added protections to prevent re-entrancy when reparsing scripts * Updated debugger test baselines
…en debugging Merge pull request #3391 from kfarnung:evaldebug * Added code to report the load of scripts on eval in order to support stepping into the eval code. * Added protections to prevent re-entrancy when reparsing scripts * Updated debugger test baselines
Added code to report the load of scripts on eval in order to support
stepping into the eval code.