Closed Bug 1525924 Opened 2 years ago Closed 2 years ago

Refactor XDRScript to better align with current JSScript struct


(Core :: JavaScript Engine, enhancement, P3)




Tracking Status
firefox67 --- fixed


(Reporter: tcampbell, Assigned: tcampbell)


(Blocks 1 open bug)



(9 files)

js::XDRScript has envolved over the years into a complex mess as JSScript has changed. With Bug 1485347 cleaning up a lot of stuff in the data structure, it makes sense to apply a similar structure to XDRScript (and later CopyScript).

The motivation is to make XDR more robust and make it easy to change JSScript for content-overhead and generated-interpreter projects.

Blocks: 1308252

Depends on D19005

Also change JSTryNote::kind to uint32_t to absorb alignment padding of

Depends on D19006

Depends on D19007

Depends on D19008

Comment on attachment 9042185 [details]
Bug 1525924 - Part 6: Factor out PrivateScriptData::XDR

Nicolas, I'm rewriting XDRScript in this stack to better match our data structures. Any major objections or strong desire to review? Otherwise I've reviewed a lot of Jan's recent JSScript changes so I'll make him review these tedious cleanups.

Attachment #9042185 - Flags: feedback?

Comment on attachment 9042185 [details]
Bug 1525924 - Part 6: Factor out PrivateScriptData::XDR

(It works better when I include people's names... )
See Comment 7.

Attachment #9042185 - Flags: feedback? → feedback?(nicolas.b.pierron)
Depends on: 1526324

Make the method static so that in a follow-up the allocation of the
object can be absorbed in the method.

This moves the allocation of the ScriptSource object into
ScriptSource::XDR instead of being done in the caller. A
MutableHandle<ScriptSourceHolder> is used to manage both
ScriptSource::refCount as well as tracing Atoms from BinAST.

Depends on D19163

This makes XDRScript better follow the field layout of JSScript.

Blocks: 1471062

This series of changes cleans up XDRScript to a lot more organized and much better aligned with the current data structures. The next set of changes (in another bug) will probably change XDRScript/CopyScript/JSScript/etc at the same time to clarify initialization sequences to be less error-prone.

Pushed by
Part 1: Factor out SharedScriptData::XDR r=jandem
Part 2: Factor out WithScope::XDR r=jandem
Part 3: Factor out JSTryNote/ScopeNote::XDR r=jandem
Part 4: Factor out js::XDRScope r=jandem
Part 5: Factor out js::XDRInnerObject r=jandem
Part 6: Factor out PrivateScriptData::XDR r=jandem
Part 7: Make ScriptSource::performXDR a static method r=jandem
Part 8: Move allocation into ScriptSource::XDR r=jandem
Part 9: Reorder steps of XDRScript r=jandem

Comment on attachment 9042185 [details]
Bug 1525924 - Part 6: Factor out PrivateScriptData::XDR

Nice refactoring! This would also make things easier to understand.

Attachment #9042185 - Flags: feedback?(nicolas.b.pierron) → feedback+
Regressions: 1545989
You need to log in before you can comment on or make changes to this bug.