-
Notifications
You must be signed in to change notification settings - Fork 686
Fix inserting pending breakpoints. #2163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix inserting pending breakpoints. #2163
Conversation
d6e52e3 to
bc874cb
Compare
| JERRY_DEBUGGER_WAIT_FOR_SOURCE = 23, /**< engine waiting for a source code */ | ||
| JERRY_DEBUGGER_OUTPUT_RESULT = 24, /**< output sent by the program to the debugger */ | ||
| JERRY_DEBUGGER_OUTPUT_RESULT_END = 25, /**< last output result data */ | ||
| JERRY_DEBUGGER_WAITING_AFTER_PARSE = 13, /**< engine waiting for a parser resume */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to add the enum value here? Can't we just add it to the end of the values? That way we don't need to renumber a lot of values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The #2166 should solve this issue.
The problem is that there are message groups, and you cannot break them. The message numbers are not random numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, so they are grouped together. I see.
| JERRY_DEBUGGER_CLIENT_SOURCE_PART = 7, /**< next message of client source */ | ||
| JERRY_DEBUGGER_NO_MORE_SOURCES = 8, /**< no more sources notification */ | ||
| JERRY_DEBUGGER_CONTEXT_RESET = 9, /**< context reset request */ | ||
| JERRY_DEBUGGER_PARSER_CONFIG = 4, /**< parser config */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above: it would be better to have the new values at the end of the existing ones.
| /* The following four messages are only available in client switch mode. */ | ||
| JERRY_DEBUGGER_CLIENT_SOURCE = 8, /**< first message of client source */ | ||
| JERRY_DEBUGGER_CLIENT_SOURCE_PART = 9, /**< next message of client source */ | ||
| JERRY_DEBUGGER_NO_MORE_SOURCES = 10, /**< no more sources notification */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above: it would be better to have the new values at the end of the existing ones.
galpeter
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
jerry-core/debugger/debugger.h
Outdated
| /** | ||
| * Set debugger flags. | ||
| */ | ||
| #define SET_DEBUGGER_FLAGS(flags) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the JERRY_ prefix in this header. Same to CLEAR_DEBUGGER_FLAGS and SET_CLEAR_DEBUGGER_FLAGS
Before this patch the JS execution is started right after the parsing is completed. The problem is that some parts of the JS code is executed before the debugger had any chance to insert pending breakpoints due to network latency. This patch adds a delay after parsing when at least one pendding breakpoint is available. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com
bc874cb to
1a16688
Compare
LaszloLango
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Before this patch the JS execution is started right after the parsing is completed. The problem is that some parts of the JS code is executed before the debugger had any chance to insert pending breakpoints due to network latency. This patch adds a delay after parsing when at least one pendding breakpoint is available.