1498755 - clean up and document Servo binding header files

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P3)

Description

[Cameron McCormack (:heycam)]

ServoBindings.h, ServoBindingList.h, ServoBindingTypes.h, and ServoTypes.h are all a bit of a mess, and could do with cleaning up and documenting.  Currently it's pretty hard for people unfamiliar with our FFI setup how to declare and use the borrowed/owned FFI types.

I have some patches for this that reduce the amount of duplication involved in declaring these FFI types.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Bug 1495669 is somewhat related here as well, but I hope it wouldn't conflict with your patches.

Thanks so much for doing this! I always get annoyed when adding new FFI stuff, but never annoyed enough to rewrite them ;)

Comment 2

[Cameron McCormack (:heycam)]

I'll stick the patch queue up in a minute.  Can rebase if I have to.

Comment 3

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 1: Turn ServoBindingList.h macros into regular function declarations

Comment 4

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 2a: Rename ServoBindings.{h,cpp} to GeckoBindings.{h,cpp}

Depends on D8641

Comment 5

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 2b: Rename ServoBindingList.h to ServoBindings.h

Depends on D8642

Comment 6

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 2c: Allow ServoBindings.h and GeckoBindings.h to be used separately

Depends on D8643

Comment 7

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 3: Merge AssertIsMainThreadOrServo{FontMetrics,LangFontPrefsCache}Locked

Depends on D8644

Comment 8

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 4: Clean up includes and forward declarations in Servo bindings headers

Depends on D8645

Comment 9

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 5: Split ServoCell out into a separate header, rename it, and clean it up a bit

Depends on D8646

Comment 10

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 6: Move some C++ types from ServoBindingTypes.h to ServoTypes.h

Depends on D8647

Comment 11

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 7: Tweak a few things in ServoBindingTypes.h

Depends on D8648

Comment 12

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 8: Add comments to ServoBindingTypes.h

Depends on D8649

Comment 13

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 9: Remove an unnecessary entry from servo-owned-types

Depends on D8650

Comment 14

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 10: Move list of Servo Boxed types to a separate header file

Depends on D8651

Comment 15

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 11: Move Gecko borrowed FFI types to a separate header file

Depends on D8652

Comment 16

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 12: Remove unused main thread FFI refcounting

Depends on D8653

Comment 17

[Cameron McCormack (:heycam)]

Attachment: Bug 1498755 - Part 13: Format GeckoBindings.h

Depends on D8654

Comment 18

[Cameron McCormack (:heycam)]

It probably would be even better if we got rid of these header-include files (like ServoArcTypeList.h) and moved their information into ServoBindings.toml, rather than the other way around.  And then generate ServoArcTypeList.h etc. from it.  Might do that as a next step some time.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8579a92f5cc1
Part 1: Turn ServoBindingList.h macros into regular function declarations r=emilio
https://hg.mozilla.org/integration/autoland/rev/75293d1bfad5
Part 2a: Rename ServoBindings.{h,cpp} to GeckoBindings.{h,cpp} r=emilio
https://hg.mozilla.org/integration/autoland/rev/7f74e0e42703
Part 2b: Rename ServoBindingList.h to ServoBindings.h r=emilio
https://hg.mozilla.org/integration/autoland/rev/84fe3307a273
Part 2c: Allow ServoBindings.h and GeckoBindings.h to be used separately r=emilio
https://hg.mozilla.org/integration/autoland/rev/ab1c2c7f0239
Part 3: Merge AssertIsMainThreadOrServo{FontMetrics,LangFontPrefsCache}Locked r=emilio
https://hg.mozilla.org/integration/autoland/rev/fb4ec4a85293
Part 4: Clean up includes and forward declarations in Servo bindings headers r=emilio
https://hg.mozilla.org/integration/autoland/rev/7c22eed0a6de
Part 5: Split ServoCell out into a separate header, rename it, and clean it up a bit r=emilio
https://hg.mozilla.org/integration/autoland/rev/77e7f45a9c3d
Part 6: Move some C++ types from ServoBindingTypes.h to ServoTypes.h r=emilio
https://hg.mozilla.org/integration/autoland/rev/0c14eb604558
Part 7: Tweak a few things in ServoBindingTypes.h r=emilio
https://hg.mozilla.org/integration/autoland/rev/5ed32166743a
Part 8: Add comments to ServoBindingTypes.h r=emilio
https://hg.mozilla.org/integration/autoland/rev/a15068898e08
Part 9: Remove an unnecessary entry from servo-owned-types r=emilio
https://hg.mozilla.org/integration/autoland/rev/5adb545ddcfd
Part 10: Move list of Servo Boxed types to a separate header file r=emilio
https://hg.mozilla.org/integration/autoland/rev/8819dd315519
Part 11: Move Gecko borrowed FFI types to a separate header file r=emilio
https://hg.mozilla.org/integration/autoland/rev/6609c511837d
Part 12: Remove unused main thread FFI refcounting r=emilio
https://hg.mozilla.org/integration/autoland/rev/2f412849ad38
Part 13: Format GeckoBindings.h r=emilio

Comment 20

[Cameron McCormack (:heycam)]

Attachment: followup fix

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d86e80f3d177
Followup build fix. r=birtles

Comment 22

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/8579a92f5cc1
https://hg.mozilla.org/mozilla-central/rev/75293d1bfad5
https://hg.mozilla.org/mozilla-central/rev/7f74e0e42703
https://hg.mozilla.org/mozilla-central/rev/84fe3307a273
https://hg.mozilla.org/mozilla-central/rev/ab1c2c7f0239
https://hg.mozilla.org/mozilla-central/rev/fb4ec4a85293
https://hg.mozilla.org/mozilla-central/rev/7c22eed0a6de
https://hg.mozilla.org/mozilla-central/rev/77e7f45a9c3d
https://hg.mozilla.org/mozilla-central/rev/0c14eb604558
https://hg.mozilla.org/mozilla-central/rev/5ed32166743a
https://hg.mozilla.org/mozilla-central/rev/a15068898e08
https://hg.mozilla.org/mozilla-central/rev/5adb545ddcfd
https://hg.mozilla.org/mozilla-central/rev/8819dd315519
https://hg.mozilla.org/mozilla-central/rev/6609c511837d
https://hg.mozilla.org/mozilla-central/rev/2f412849ad38
https://hg.mozilla.org/mozilla-central/rev/d86e80f3d177