Closed Bug 1498755 Opened Last year Closed Last year

clean up and document Servo binding header files

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: heycam, Assigned: heycam)

Details

Attachments

(16 files)

46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
1.38 KB, patch
birtles
: review+
Details | Diff | Splinter Review
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.
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 ;)
I'll stick the patch queue up in a minute.  Can rebase if I have to.
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.
Pushed by cmccormack@mozilla.com:
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
Attached patch followup fixSplinter Review
Attachment #9017066 - Flags: review?(bbirtles)
Attachment #9017066 - Attachment is patch: true
Attachment #9017066 - Attachment mime type: application/octet-stream → text/plain
Attachment #9017066 - Flags: review?(bbirtles) → review+
You need to log in before you can comment on or make changes to this bug.