Closed Bug 830369 Opened 7 years ago Closed 7 years ago

Refactor ScriptDebugPrologue and ScriptDebugEpilogue to use AbstractFramePtr

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch (obsolete) — Splinter Review
No description provided.
Attachment #701819 - Flags: review?(kvijayan)
Attached patch PatchSplinter Review
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+
(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
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.