Skip to content

Comments

Stylo: Add FFI interfaces for exposing style sources.#16328

Merged
bors-servo merged 1 commit intoservo:masterfrom
bradwerth:computedStyles
Apr 10, 2017
Merged

Stylo: Add FFI interfaces for exposing style sources.#16328
bors-servo merged 1 commit intoservo:masterfrom
bradwerth:computedStyles

Conversation

@bradwerth
Copy link
Contributor

@bradwerth bradwerth commented Apr 10, 2017

@highfive
Copy link

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @metajack (or someone else) soon.

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @bholley: components/style/gecko_bindings/bindings.rs, components/style/gecko_bindings/structs_debug.rs, components/style/build_gecko.rs, components/style/gecko_bindings/structs_release.rs, components/style/gecko/arc_types.rs
  • @emilio: components/style/gecko_bindings/bindings.rs, components/style/gecko_bindings/structs_debug.rs, ports/geckolib/glue.rs, components/style/build_gecko.rs, components/style/gecko_bindings/structs_release.rs, components/style/gecko/arc_types.rs

@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify style code, but no tests are modified. Please consider adding a test!

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 10, 2017
@Manishearth
Copy link
Member

@bors-servo delegate+

use @bors-servo r=whoever to approve for landing

@bors-servo
Copy link
Contributor

📌 Commit 549a6a0 has been approved by `whoever``

@bors-servo
Copy link
Contributor

✌️ @bradwerth can now approve this pull request

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 10, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 549a6a0 with merge b84d079...

bors-servo pushed a commit that referenced this pull request Apr 10, 2017
Stylo: Add FFI interfaces for exposing style sources.

https://bugzilla.mozilla.org/show_bug.cgi?id=1346256
https://reviewboard.mozilla.org/r/119044/

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16328)
<!-- Reviewable:end -->
@Manishearth
Copy link
Member

@bors-servo r=heycam

@bors-servo
Copy link
Contributor

💡 This pull request was already approved, no need to approve it again.

@bors-servo
Copy link
Contributor

📌 Commit 549a6a0 has been approved by heycam

@highfive highfive assigned heycam and unassigned metajack Apr 10, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 549a6a0 with merge 661e1a51e4a560d329faff2214f34b2d7aa60f48...

@Manishearth
Copy link
Member

@bors-servo r- retry clean r=heycam

@bors-servo
Copy link
Contributor

💡 This pull request was already approved, no need to approve it again.

@bors-servo
Copy link
Contributor

📌 Commit 549a6a0 has been approved by heycam

@bors-servo
Copy link
Contributor

⌛ Testing commit 549a6a0 with merge 19203ec...

bors-servo pushed a commit that referenced this pull request Apr 10, 2017
Stylo: Add FFI interfaces for exposing style sources.

https://bugzilla.mozilla.org/show_bug.cgi?id=1346256
https://reviewboard.mozilla.org/r/119044/

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16328)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - windows-msvc-dev

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 10, 2017
aSource: *const nsStyleFont);
}
extern "C" {
pub fn Gecko_GetBaseSize(lang: *mut nsIAtom) -> FontSizePrefs;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(undo this change)

@Manishearth
Copy link
Member

error[E0432]: unresolved import `gecko_bindings::bindings::Gecko_GetBaseSize`
   --> C:\buildbot\slave\windows-msvc-dev\build\components\style\gecko\wrapper.rs:460:13
    |
460 |         use gecko_bindings::bindings::Gecko_GetBaseSize;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `Gecko_GetBaseSize` in `gecko_bindings::bindings`

error[E0432]: unresolved import `gecko_bindings::bindings::Gecko_GetBaseSize`
     --> c:\buildbot\slave\windows-msvc-dev\build\target\geckolib\debug\build\style-d5c8007cf63b1594\out/properties.rs:23716:21
      |
23716 |                 use gecko_bindings::bindings::Gecko_GetBaseSize;
      |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `Gecko_GetBaseSize` in `gecko_bindings::bindings`

error[E0412]: cannot find type `FontSizePrefs` in module `gecko_bindings::structs`
   --> C:\buildbot\slave\windows-msvc-dev\build\components\style\gecko\wrapper.rs:442:45
    |
442 |     pub font_size_cache: RefCell<Vec<(Atom, ::gecko_bindings::structs::FontSizePrefs)>>,
    |                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in `gecko_bindings::structs`

error[E0412]: cannot find type `FontSizePrefs` in module `structs`
   --> C:\buildbot\slave\windows-msvc-dev\build\components\style\gecko\wrapper.rs:473:6
    |
473 | impl structs::FontSizePrefs {
    |      ^^^^^^^^^^^^^^^^^^^^^^ not found in `structs`

error[E0531]: cannot find unit struct/variant or constant `kPresContext_DefaultVariableFont_ID` in module `structs`
   --> C:\buildbot\slave\windows-msvc-dev\build\components\style\gecko\wrapper.rs:476:13
    |
476 |             structs::kPresContext_DefaultVariableFont_ID => self.mDefaultVariableSize,
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in `structs`

error[E0531]: cannot find unit struct/variant or constant `kPresContext_DefaultFixedFont_ID` in module `structs`
   --> C:\buildbot\slave\windows-msvc-dev\build\components\style\gecko\wrapper.rs:477:13
    |
477 |             structs::kPresContext_DefaultFixedFont_ID => self.mDefaultFixedSize,
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in `structs`

error: struct `RawServoStyleRule` is private
  --> C:\buildbot\slave\windows-msvc-dev\build\components\style\gecko\arc_types.rs:12:52
   |
12 | use gecko_bindings::bindings::{RawServoStyleSheet, RawServoStyleRule, RawServoImportRule};
   |                                                    ^^^^^^^^^^^^^^^^^

error: the type of this value must be known in this context
   --> C:\buildbot\slave\windows-msvc-dev\build\components\style\gecko\wrapper.rs:462:30
    |
462 |         if let Some(sizes) = cache.iter().find(|el| el.0 == *font_name) {
    |                              ^^^^^^^^^^^^

error: aborting due to previous error

use gecko_bindings::structs::ComputedTimingFunction_BeforeFlag;
use gecko_bindings::structs::FontFamilyList;
use gecko_bindings::structs::FontFamilyType;
use gecko_bindings::structs::FontSizePrefs;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undo this change

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 10, 2017
@bradwerth
Copy link
Contributor Author

@bors-servo r=heycam

@bors-servo
Copy link
Contributor

📌 Commit 4a9ac44 has been approved by heycam

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 10, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 4a9ac44 with merge c33a98f...

bors-servo pushed a commit that referenced this pull request Apr 10, 2017
Stylo: Add FFI interfaces for exposing style sources.

https://bugzilla.mozilla.org/show_bug.cgi?id=1346256
https://reviewboard.mozilla.org/r/119044/

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16328)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-dev

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 10, 2017
@emilio
Copy link
Member

emilio commented Apr 10, 2017

./ports/geckolib/glue.rs:63: use statement is not in alphabetical order
	expected: style::gecko_bindings::structs::RawGeckoPresContextOwned
	found: style::gecko_bindings::structs::{RawServoStyleRule, ServoStyleSheet}

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 10, 2017
@bradwerth
Copy link
Contributor Author

@bors-servo r=heycam

@bors-servo
Copy link
Contributor

📌 Commit bfc7e84 has been approved by heycam

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 10, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit bfc7e84 with merge a0a60ba...

bors-servo pushed a commit that referenced this pull request Apr 10, 2017
Stylo: Add FFI interfaces for exposing style sources.

https://bugzilla.mozilla.org/show_bug.cgi?id=1346256
https://reviewboard.mozilla.org/r/119044/

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16328)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: heycam
Pushing a0a60ba to master...

@bors-servo bors-servo merged commit bfc7e84 into servo:master Apr 10, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 10, 2017
@bradwerth bradwerth deleted the computedStyles branch April 10, 2017 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants