Closed Bug 833817 Opened 7 years ago Closed 7 years ago

Fix JSStackFrame API functions to work with baseline JIT frames

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(3 files)

We need this for JSD to work.

The plan is to make Valueify/Jsvalify return/take an AbstractFramePtr instead of js::StackFrame *.
Attachment #705463 - Flags: review?(luke)
Comment on attachment 705463 [details] [diff] [review]
Part 1: Remove dead code

Ah, my favorite type of patch to review.
Attachment #705463 - Flags: review?(luke) → review+
There's one caller and it always passes NULL.

Try is still running:

https://tbpl.mozilla.org/?tree=Try&rev=3cbd78955d46
Attachment #705820 - Flags: review?(bobbyholley+bmo)
Comment on attachment 705820 [details] [diff] [review]
Part 2: Remove JSStackFrame argument from GetFunctionObjectPrincipal

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

r=bholley
Attachment #705820 - Flags: review?(bobbyholley+bmo) → review+
Whiteboard: [leave open]
This patch replaces JSStackFrame * with JSAbstractFramePtr (a subset of AbstractFramePtr). API functions taking JSStackFrame * are now methods of JSAbstractFramePtr.

JSBrokenFrameIterator is a new class that wraps a ScriptFrameIter (it stores a pointer to StackIter::Data). Fortunately, the only places where these classes are used are JSD and XPCDebug. Hopefully we can remove them once we get rid of JSD.

I tried to keep the JSD changes as minimal as possible. Firebug still works and passes its own test suite.
Attachment #706157 - Flags: review?(luke)
Comment on attachment 706157 [details] [diff] [review]
Part 3: Replace JSStackFrame * with JSAbstractFramePtr

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

Great job slogging through all this!  Try server doesn't test jsd so well so, in the past with my scope work, I'd run the Firebug test suite (really quite easy https://getfirebug.com/wiki/index.php/Running_Automated_Test_Suite).  Sometimes it isn't all green, so I'd try a baseline run first :)

::: js/src/jspubtd.h
@@ -180,5 @@
>  typedef struct JSPropertyName               JSPropertyName;
>  typedef struct JSPropertySpec               JSPropertySpec;
>  typedef struct JSRuntime                    JSRuntime;
>  typedef struct JSSecurityCallbacks          JSSecurityCallbacks;
> -typedef struct JSStackFrame                 JSStackFrame;

yippe kiy yiy yay
Attachment #706157 - Flags: review?(luke) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8af2dfc0c2a

Green try run, modulo known failures: https://tbpl.mozilla.org/?tree=Try&rev=cee4479985b1

(Luke Wagner [:luke] from comment #8)
> Try server doesn't test jsd so well
> so, in the past with my scope work, I'd run the Firebug test suite (really
> quite easy
> https://getfirebug.com/wiki/index.php/Running_Automated_Test_Suite). 
> Sometimes it isn't all green, so I'd try a baseline run first :)

(Jan de Mooij [:jandem] from comment #6)
> Firebug still works and passes its own test suite.

:)
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/d8af2dfc0c2a
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.