Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

style: Sync changes from mozilla-central. #26202

Merged
merged 74 commits into from Apr 16, 2020
Merged

Conversation

@emilio
Copy link
Member

emilio commented Apr 16, 2020

See individual commits for details.

https://bugzilla.mozilla.org/show_bug.cgi?id=1630676

emilio and others added 30 commits Feb 10, 2020
ImageLayer is almost the only usage of Image, so keeping them in the same enum
makes the resulting C++ struct smaller, and makes it map more cleanly to
nsStyleImage.

Differential Revision: https://phabricator.services.mozilla.com/D62161
We include it everywhere because it's included from gfxTypes.h.

This should avoid including all the generated bindings _everywhere_.

Differential Revision: https://phabricator.services.mozilla.com/D62174
… clip-path.

Tweak the ShapeSourceRepresentation so that it doesn't store Option<>s.

Some renames so that GeometryBox doesn't conflict with the Gecko type, and some
other usual bits / re-exports to deal with cbindgen and generics.

Also, drive-by derive parsing of GeometryBox as it's trivial.

Doing this unfortunately is not possible without removing nsStyleImage first, so
let's do that before.

This makes us serialize in the shortest form for shape-outside, but that's what
we should do anyway.

(aside: the shapes code is a bit too generic, maybe we should unify
ClippingShape and FloatAreaShape...)

Differential Revision: https://phabricator.services.mozilla.com/D62163
The trickier part is that we represent -moz-image-rect as a Rect() type instead
of image with non-null clip-rect. So we need to add a bit of code to
distinguish "image request types" from other types of images.

But it's not too annoying, and we need to do the same for fancier images like
image-set and such whenever we implement it, so seems nice to get rid of
most explicit usages of nsStyleImage::GetType().

Differential Revision: https://phabricator.services.mozilla.com/D62164
We don't actually share _that_ much code across them. This makes callers clearer
and code less confusing, IMHO.

This also has the benefit of not autocompleting path from devtools for
shape-outside.

Differential Revision: https://phabricator.services.mozilla.com/D62373
Still keep the discriminant checks to avoid generating terrible code.

Differential Revision: https://phabricator.services.mozilla.com/D62329
So as to avoid serializing as identifiers font-families with spaces as part of
the identifier. This avoids serializing confusing escaped sequences if the
beginning of the stuff after the space happens to not be a valid ident start.

This is an slightly more restrictive version of the existing logic, which
happens to also match other browsers in my testing.

Differential Revision: https://phabricator.services.mozilla.com/D62376
I suggested the compat_mode bit in D62923 but it was somehow only applied to one
of the branches.

Also rustfmt the code for consistency, and add a local alias.

Differential Revision: https://phabricator.services.mozilla.com/D63015
I don't think we want to keep the ugly widget hacks forever. Let me know if
you'd rather keep the property behind a pref but I don't think there's a point
in doing that.

Differential Revision: https://phabricator.services.mozilla.com/D62649
This assert was wrong. The assert may fire if we resurrect the node from a
different thread and insert a kid fast enough.

We allow resurrecting nodes (bumping the nodes from zero to one) to avoid
allocation churn.

In particular, while the thread dropping the node gets to read the children (so
after the fetch_sub from the refcount, but before the read() of the children),
another thread could plausibly bumped the refcount back, and added a children.

This is a very big edge case of course, but I'm kinda sad I hadn't realized
before.

Differential Revision: https://phabricator.services.mozilla.com/D63286
We'll use `CalcNode` as the specified value representation for <length> and
<length-percentage> values, so they'll have to implement ToCss.

There's one minor issue (two calls to to_css() instead of to_css_impl() which
are addressed later in the series).

Differential Revision: https://phabricator.services.mozilla.com/D63395
We'll have different leaf nodes as we progress in the value computation stage.

Differential Revision: https://phabricator.services.mozilla.com/D63396
…ngth-percentage> values.

This is the meat of the patch. There are a couple improvements done in a couple
later patches which should hopefully be straight-forward.

Differential Revision: https://phabricator.services.mozilla.com/D63397
So as to avoid allocating an intermediate tree in Rust to resolve
`<length-percentage>` values.

Differential Revision: https://phabricator.services.mozilla.com/D63399
…res.

We were serializing calc(10% + 4px) as calc(10% + calc(4px)).

Differential Revision: https://phabricator.services.mozilla.com/D63400
We never fast-reject numbers (because they could be part of a product). Without
this refactoring we'd accept stuff like calc(10) and crash during the evaluation
for obvious reasons.

Differential Revision: https://phabricator.services.mozilla.com/D63401
* Use debug_unreachable for really unreachable code (having a release
   unreachable!() there gives us little to no benefit, as a borked union can
   already confuse us into reading an arbitrary pointer as a CalcPercentage).

 * Avoid a clone of the calc variant when clamping. We only need to mutate the
   clamping mode. This was the only clamp_to_non_negative function that didn't
   consume the value.

Differential Revision: https://phabricator.services.mozilla.com/D63584
@bors-servo
Copy link
Contributor

bors-servo commented Apr 16, 2020

💔 Test failed - status-taskcluster

@emilio
Copy link
Member Author

emilio commented Apr 16, 2020

@jdm
Copy link
Member

jdm commented Apr 16, 2020

I have re-run the task that failed with a network interruption.

@emilio
Copy link
Member Author

emilio commented Apr 16, 2020

ah, thanks @jdm!

@emilio
Copy link
Member Author

emilio commented Apr 16, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Apr 16, 2020

📌 Commit b25865c has been approved by emilio

@highfive highfive assigned emilio and unassigned asajeffrey Apr 16, 2020
bors-servo added a commit that referenced this pull request Apr 16, 2020
style: Sync changes from mozilla-central.

See individual commits for details.

https://bugzilla.mozilla.org/show_bug.cgi?id=1630676
@bors-servo
Copy link
Contributor

bors-servo commented Apr 16, 2020

Testing commit b25865c with merge 6c06a73...

@bors-servo
Copy link
Contributor

bors-servo commented Apr 16, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Apr 16, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Apr 16, 2020

Testing commit b25865c with merge 2829945...

@bors-servo
Copy link
Contributor

bors-servo commented Apr 16, 2020

☀️ Test successful - status-taskcluster
Approved by: emilio
Pushing 2829945 to master...

@bors-servo bors-servo merged commit 2829945 into servo:master Apr 16, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.