Closed Bug 1403030 Opened 2 years ago Closed 2 years ago

stylo: C++ declarations of Servo_GetArcStringData and friends don't match Rust definitions

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

Details

Attachments

(1 file)

Some pointer arguments should be const.
(I'm updating the checked-in bindings files in https://github.com/servo/servo/pull/18623.)
Assignee: nobody → cam
Status: NEW → ASSIGNED
Comment on attachment 8912071 [details]
Bug 1403030 - stylo: Fix some mismatching FFI declarations.

https://reviewboard.mozilla.org/r/183454/#review188588
Attachment #8912071 - Flags: review?(xidorn+moz) → review+
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf266c3319e8
stylo: Fix some mismatching FFI declarations. r=xidorn
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/45cfb2a32cde
stylo: Followup for another missing const. r=me
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bd5866690091
stylo: One more followup to fix a build error. r=me
We may need to uplift this if this affects binding tests.
Yes, if we want to regenerate bindings on Beta then we'll need this.
Comment on attachment 8912071 [details]
Bug 1403030 - stylo: Fix some mismatching FFI declarations.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: no user impact, but without uplifting this, we won't be able to run Rust/C++ object layout tests on Beta, which we want to do to ensure Stylo can safely poke at C++ objects
[Is this code covered by automated tests?]: indirectly, tests will end up calling these functions
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: all three commits that landed in this bug are needed
[Is the change risky?]: no
[Why is the change risky/not risky?]: small patch that just tweaks some FFI function declarations; no functionality changes
[String changes made/needed]: none
Attachment #8912071 - Flags: approval-mozilla-beta?
Comment on attachment 8912071 [details]
Bug 1403030 - stylo: Fix some mismatching FFI declarations.

Fix tests, taking it.
Should be in 57b4, gtb tomorrow Thursday
Attachment #8912071 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Cameron McCormack (:heycam) from comment #10)
> [Is this code covered by automated tests?]: indirectly, tests will end up
> calling these functions
> [Has the fix been verified in Nightly?]: yes
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Cameron's assessment on manual testing needs and the fact that this fix has automated coverage, although indirectly.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.