Bug 1498755 - Part 8: Add comments to ServoBindingTypes.h r=emilio

Depends on D8649

Differential Revision: https://phabricator.services.mozilla.com/D8650

--HG--
extra : moz-landing-system : lando

layout/style/ServoArcTypeList.h

@@ -4,7 +4,21 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-/* a list of all Servo Arc types used in stylo bindings for preprocessing */
+/* a list of all Servo Arc<T> types used across bindings, for preprocessing */
+
+// The first argument is the name of the Servo type used inside the Arc.
+// This doesn't need to be accurate; it's only used to generate nice looking
+// FFI function names.
+//
+// The second argument is the name of an opaque Gecko type that will
+// correspond to the Servo type used inside the Arc.  The convention for the
+// the name of the opaque Gecko type is "RawServo{Type}", where {Type} is
+// the name of the Servo type or something close to it.
+//
+// See the comment at the top of ServoBindingTypes.h for how to use these.
+//
+// If you add an entry to this file, you should also add an impl_arc_ffi!()
+// call to servo/components/style/gecko/arc_types.rs.
 
 SERVO_ARC_TYPE(CssRules, ServoCssRules)
 SERVO_ARC_TYPE(StyleSheetContents, RawServoStyleSheetContents)

layout/style/ServoBindingTypes.h

@@ -4,6 +4,146 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+/* C++ types corresponding to Servo and Gecko types used across bindings,
+   with some annotations to indicate ownership expectations */
+
+// This file defines a number of C++ types used to represent borrowed,
+// strong, and owning references to Servo and Gecko objects that might
+// be used across bindings and FFI.
+//
+// By convention, the types defined here are named "RawServo{Type}" and
+// "RawGecko{Type}".  The {Type} should be something close to the real Rust or
+// C++ name of the type, but need not be.  The "Raw" is really just used to
+// avoid clashing with other names.
+//
+// For Servo types, each "RawServo{ServoTypeName}" is generated as an opaque,
+// declared but not defined struct.
+//
+// For Gecko types, each "RawGecko{GeckoTypeName}" is a typedef that aliases
+// the actual C++ type.
+//
+// Each of these types can have a number of different typedefs generated for
+// them, representing different notions of ownership when passing or receiving
+// these values across bindings and FFI:
+//
+//   Raw{Gecko,Servo}{Type}Borrowed
+//   Raw{Gecko,Servo}{Type}BorrowedOrNull
+//     immutable, borrowed reference (or null)
+//
+//   Raw{Gecko,Servo}{Type}BorrowedMut
+//   Raw{Gecko,Servo}{Type}BorrowedMutOrNull
+//     mutable, borrowed reference (or null)
+//
+//   RawServo{Type}Strong
+//     strong reference to an Arc-managed value
+//
+//   RawServo{Type}Owned
+//   RawServo{Type}OwnedOrNull
+//     owned reference to a Box-managed value (or null)
+//
+// All of these borrowed, strong, and owned types are generated by adding
+// entries to one of these files:
+//
+//   BorrowedTypeList.h
+//     generates some or all of the Borrowed types
+//
+//   ServoArcTypeList.h
+//     generates all of the Borrowed types and the Strong type
+//
+//   ServoBoxedTypeList.h
+//     generates all of the Borrowed types and the Owned & OwnedNull types
+//
+// The borrowed, strong, and owned reference types should be used in FFI
+// function signatures where possible, to help indicate the ownership properties
+// that both sides of the function call must adhere to.
+//
+// There are some special cases defined at the bottom of this file that don't
+// fit neatly into these three categories.
+//
+//
+// Using these types in C++
+// ========================
+//
+// All of the Borrowed types are C++ typedefs for raw pointers, and can be used
+// directly.  Since the types they point to are opaque, there isn't much that
+// can be done with these apart from passing them around, holding on to them,
+// checking them for equality, etc.  If they are Arc-managed or Box-managed
+// Servo types, they can be assigned to a RefPtr<> or UniquePtr<>.
+//
+// The Strong types are a C++ struct that wraps a raw pointer.  When receiving
+// a Strong value from a Servo_* FFI function, you must call Consume() on it
+// to convert it into an already_AddRefed<RawServo{Type}>, otherwise it will
+// leak.
+//
+// We don't currently have any cases where we pass a Strong value to Servo; this
+// could be done by creating a RawServo{Type}Strong struct value whose mPtr is
+// initialized to the result of calling `.forget().take()` on a
+// RefPtr<RawServo{Type}>, but it's probably easier just to pass a Borrowed
+// value and let the Rust code turn it into an Arc.
+//
+// The Owned types are C++ typedefs for raw pointers.  When receiving an Owned
+// value from a Servo_* FFI function, it should be assigned to a UniquePtr<>,
+// otherwise it will leak.
+//
+// To pass an Owned value to Servo, call `release()` on the UniquePtr<> it's
+// living in (to take ownership of it), and pass that pointer in directly.
+//
+// TODO(heycam): We should perhaps have a similar struct for Owned types with
+// a Consume() method to convert them into a UniquePtr.  The struct for Strong
+// types at least have MOZ_MUST_USE_TYPE on them.
+//
+//
+// Using these types in Rust
+// =========================
+//
+// The FFI type names are available in Rust in the gecko_bindings::bindings mod,
+// which is generated by servo/components/style/build_gecko.rs.
+//
+// The Borrowed types are defined as Rust reference types.
+//
+// Borrowed types for Gecko values are references to the bindgened versions of
+// the C++ types, so when receiving references over FFI into a Servo_* function,
+// they can be used directly, and when returning them back to Gecko, the
+// reference can be returned directly.
+//
+// Borrowed types for Servo values are references to an opaque type, which must
+// be converted to or from the appropriate Rust type:
+//
+//   For an Arc-owned value, a RawServoFooBorrowed received from FFI can be
+//   converted into a `&Arc<Foo>` by calling `Foo::as_arc(&raw_servo_foo)`.
+//   Returning a RawServoFooBorrowed over FFI back to Gecko can be done by
+//   calling `as_borrowed()` on the `Arc<Foo>`.
+//
+//   For a Box-owned value, a RawServoFooBorrowed received from FFI can be
+//   converted into a `&Foo` by calling `Foo::from_ffi(&raw_servo_foo)`.
+//   Returning a RawServoFooBorrowed over FFI back to Gecko can be done by
+//   calling `as_ffi()` on the `Foo`.
+//
+// The Strong types are defined as gecko_bindings::sugar::ownership::Strong<T>.
+//
+// This is an FFI safe type that represents the value with a strong reference
+// already added to it.  Dropping a Strong<T> will leak the strong reference.
+//
+// A RawServoFooStrong received from FFI can be converted into a
+// `RawOffsetArc<Foo>` by calling `into_arc()` or `into_arc_opt()` on it.
+// To pass a RawServoFooStrong back to Gecko, call `into_strong()` on the
+// `Arc<Foo>`.
+//
+// The Owned types are defined as gecko_bindings::sugar::ownership::Owned<T>
+// (or OwnedOrNull<T>).
+//
+// This is another FFI safe type that represents the owning reference to the
+// value.  Dropping an Owned<T> will leak the value.
+//
+// A RawServoFooOwned received from FFI can be converted into a `Box<Foo>`
+// by calling `into_box()` or `into_box_opt()` on it.  To pass a
+// RawServoFooOwned back to Gecko, call `HasBoxFFI::into_ffi()` passing in
+// the `Box<Foo>` value.
+//
+//
+// Reading through servo/components/style/gecko_bindings/sugar/ownership.rs
+// is also instructive in understanding all this.
+
 #ifndef mozilla_ServoBindingTypes_h
 #define mozilla_ServoBindingTypes_h
 

layout/style/ServoBindings.h

@@ -139,6 +139,11 @@ void Servo_StyleSheet_GetSourceURL(
 // they wrap the value in a struct.
 uint8_t Servo_StyleSheet_GetOrigin(RawServoStyleSheetContentsBorrowed sheet);
 
+// TODO(heycam): RawGeckoPresContextOwned feels like the wrong type to use here
+// to indicate what we're doing. nsPresContext is a refcounted type, but we hold
+// a weak reference to it in the style set we create. Using
+// RawGeckoPresContextOwned makes it seem like we are passing ownership to
+// Servo_StyleSet_Init, which is not true.
 RawServoStyleSet* Servo_StyleSet_Init(RawGeckoPresContextOwned pres_context);
 void Servo_StyleSet_RebuildCachedData(RawServoStyleSetBorrowed set);
 

servo/components/style/build_gecko.rs

@@ -310,7 +310,9 @@ mod bindings {
             .expect("Fail to read ServoArcTypeList.h");
         // Remove comments
         let block_comment_re = Regex::new(r#"(?s)/\*.*?\*/"#).unwrap();
+        let line_comment_re = Regex::new(r#"//.*"#).unwrap();
         let content = block_comment_re.replace_all(&content, "");
+        let content = line_comment_re.replace_all(&content, "");
         // Extract the list
         let re = Regex::new(r#"^SERVO_ARC_TYPE\(\w+,\s*(\w+)\)$"#).unwrap();
         content