Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upstyle: sync changes from mozilla-central #23503
Merged
+892
−910
Conversation
highfive
commented
Jun 3, 2019
|
Heads up! This PR modifies the following files:
|
highfive
commented
Jun 3, 2019
|
|
|
@bors-servo r+ |
|
|
This should be an idempotent patch. The way to come up with this patch has been: * Run the first script attached to the bug and pipe it to xclip, then paste it in color.rs * Add the relevant #[derive] annotations and remove the color.mako.rs definition. * Reorder the values to match the ColorID definition, on which some widget prefs and caching stuff relies on. * Manually port some documentation from nsLookAndFeel.h * Run `rg 'eColorID_' | cut -d : -f 1 | sort | uniq >files` * Run the second script attached to the bug. * Manually fix usage of `LAST_COLOR` (adding the `End` variant), and adding casts to integer as needed. * Add an static assert so that people remember to update the prefs, rather than a comment on the definition :) Differential Revision: https://phabricator.services.mozilla.com/D32610
This prevents exposing the value to web content. Differential Revision: https://phabricator.services.mozilla.com/D32611
Differential Revision: https://phabricator.services.mozilla.com/D32237
This doesn't clean up as much as a whole, but it's a step in the right
direction. In particular, it allows us to start using simple bindings for:
* Filters
* Shapes and images, almost. Need to:
* Get rid of the complex -moz- gradient parsing (let
layout.css.simple-moz-gradient.enabled get to release).
* Counters, almost. Need to:
* Share the Attr representation with Gecko, by not using Option<>.
* Just another variant should be enough (ContentItem::{Attr,Prefixedattr},
maybe).
Which in turn allows us to remove a whole lot of bindings in followups to this.
The setup changes a bit. This also removes the double pointer I complained about
while reviewing the shared UA sheet patches. The old setup is:
```
SpecifiedUrl
* CssUrl
* Arc<CssUrlData>
* String
* UrlExtraData
* UrlValueSource
* Arc<CssUrlData>
* load id
* resolved uri
* CORS mode.
* ...
```
The new one removes the double reference to the url data via URLValue, and looks
like:
```
SpecifiedUrl
* CssUrl
* Arc<CssUrlData>
* String
* UrlExtraData
* CorsMode
* LoadData
* load id
* resolved URI
```
The LoadData is the only mutable bit that C++ can change, and is not used from
Rust. Ideally, in the future, we could just use rust-url to resolve the URL
after parsing or something, and make it all immutable. Maybe.
I've verified that this approach still works with the UA sheet patches (via the
LoadDataSource::Lazy).
The reordering of mWillChange is to avoid nsStyleDisplay from going over the
size limit. We want to split it up anyway in bug 1552587, but mBinding gains a
tag member, which means that we were having a bit of extra padding.
One thing I want to explore is to see if we can abuse rustc's non-zero
optimizations to predict the layout from C++, but that's something to explore at
some other point in time and with a lot of care and help from Michael (who sits
next to me and works on rustc ;)).
Differential Revision: https://phabricator.services.mozilla.com/D31742
Had to implement some OwnedSlice bits that the canvas code used. Differential Revision: https://phabricator.services.mozilla.com/D31799
Differential Revision: https://phabricator.services.mozilla.com/D32173
We cannot compile with just feature(gecko + debug_assertions), since that's how debug rusttests get compiled and they don't have the refcount logging stuff. We were getting away with it for the pre-existing usage of the style crate, because it wasn't used during any test and presumably the linker didn't complain. But servo_arc is definitely used in tests. Differential Revision: https://phabricator.services.mozilla.com/D32691
…rule node children. I need to profile this a bit more, but talos was pretty happy about this, and it solves the known performance issues here such as the test-case from bug 1483963 for example. This also gets rid of a bunch of unsafe code which is nice. This still keeps the same GC scheme, removing the key from the hashmap when needed. I kept those as release assertions, but should probably be turned into debug-only assertions. Differential Revision: https://phabricator.services.mozilla.com/D6801
I think this is a good change regardless of other discussion in bug 1552587. If we decide to move `mColor` to the top-level of the struct that can be done separately. Differential Revision: https://phabricator.services.mozilla.com/D32726
Differential Revision: https://phabricator.services.mozilla.com/D33308
|
@bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Jun 4, 2019
style: sync changes from mozilla-central See each individual commit for details. This also cherry-picks #23463 with a few fixes that were needed in Gecko-only code. <!-- 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/23503) <!-- Reviewable:end -->
|
|
|
@bors-servo retry |
bors-servo
added a commit
that referenced
this pull request
Jun 4, 2019
style: sync changes from mozilla-central See each individual commit for details. This also cherry-picks #23463 with a few fixes that were needed in Gecko-only code. <!-- 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/23503) <!-- Reviewable:end -->
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
emilio commentedJun 3, 2019
•
edited by SimonSapin
See each individual commit for details. This also cherry-picks #23463 with a few fixes that were needed in Gecko-only code.
This change is