Refactor ScriptDebugPrologue and ScriptDebugEpilogue to use AbstractFramePtr

RESOLVED FIXED in mozilla21

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

unspecified
mozilla21
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 701819 [details] [diff] [review]
Patch
Attachment #701819 - Flags: review?(kvijayan)
(Assignee)

Comment 1

5 years ago
Created attachment 701820 [details] [diff] [review]
Patch
Attachment #701819 - Attachment is obsolete: true
Attachment #701819 - Flags: review?(kvijayan)
Attachment #701820 - Flags: review?(kvijayan)
Comment on attachment 701820 [details] [diff] [review]
Patch

Review of attachment 701820 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsdbgapi.cpp
@@ +114,4 @@
>      JSBool ok = okArg;
>  
> +    if (void *hookData = frame.maybeHookData()) {
> +        if (frame.isGlobalFrame() || frame.isEvalFrame()) {

It would seem to be clearer if the code defined isFramePushedByExecute() on AbstractFamePointer, and used it here.

::: js/src/vm/Stack.h
@@ +1935,1 @@
>  };

When was AbstractFramePtr defined?  I don't have the changeset with the initial definition of AbstractFramePtr in either my inbound or ionmonkey tree.
Attachment #701820 - Flags: review?(kvijayan) → review+
(Assignee)

Comment 3

5 years ago
(In reply to Kannan Vijayan [:djvj] from comment #2)
> When was AbstractFramePtr defined?  I don't have the changeset with the
> initial definition of AbstractFramePtr in either my inbound or ionmonkey
> tree.

Sorry, I should have mentioned this. I added it as TaggedFramePtr and renamed it to AbstractFramePtr in bug 829554 (just pushed the second patch there to inbound).
https://hg.mozilla.org/mozilla-central/rev/65747e07d9df
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.