833817 - Fix JSStackFrame API functions to work with baseline JIT frames

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

We need this for JSD to work.

The plan is to make Valueify/Jsvalify return/take an AbstractFramePtr instead of js::StackFrame *.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Part 1: Remove dead code

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 705463 [details] [diff] [review]
Part 1: Remove dead code

Ah, my favorite type of patch to review.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Part 2: Remove JSStackFrame argument from GetFunctionObjectPrincipal

There's one caller and it always passes NULL.

Try is still running:

https://tbpl.mozilla.org/?tree=Try&rev=3cbd78955d46

Comment 4

[Bobby Holley (:bholley)]

Comment on attachment 705820 [details] [diff] [review]
Part 2: Remove JSStackFrame argument from GetFunctionObjectPrincipal

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

r=bholley

Comment 5

[Jan de Mooij [:jandem]]

Thanks for the quick reviews.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c5b29fe24a0f
https://hg.mozilla.org/integration/mozilla-inbound/rev/c84500983512

(Green Linux64 Try run: https://tbpl.mozilla.org/?tree=Try&rev=3cbd78955d46)

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Part 3: Replace JSStackFrame * with JSAbstractFramePtr

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.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c5b29fe24a0f
https://hg.mozilla.org/mozilla-central/rev/c84500983512

Comment 8

[Luke Wagner [: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

Comment 9

[Jan de Mooij [:jandem]]

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.

:)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d8af2dfc0c2a