1496486 - Remove a lot of nsCSSValue usage.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1496486 - Remove a bunch of unused nsCSSValue code.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1496486 - Remove nsCSSValue usage from font code.

This needs a cssparser update which I already landed on inbound, to
include https://github.com/servo/rust-cssparser/pull/228.

Really sorry for the size of the patch.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1496486 - Remove GridTemplateAreas stuff from nsCSSValue.

It's unused.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1496486 - Remove some more leftover code.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1496486 - Bump cbindgen.

This will actually be a proper version bump with the updated configure
check and such once that PR (https://github.com/eqrion/cbindgen/pull/213) is in
a release. I don't plan to land these changes before that.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1496486 - Remove now outdated comments.

We can't remove the defaulted constructors because we use them in
FindMatchingFontFaces.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab92ed3e0a23
Remove a bunch of unused nsCSSValue code. r=heycam
https://hg.mozilla.org/integration/autoland/rev/86382b2249f6
Remove nsCSSValue usage from font code. r=heycam
https://hg.mozilla.org/integration/autoland/rev/8e465202c355
Remove GridTemplateAreas stuff from nsCSSValue. r=heycam
https://hg.mozilla.org/integration/autoland/rev/5cf44e254ac3
Remove some more leftover code. r=heycam
https://hg.mozilla.org/integration/autoland/rev/6b740afea403
Bump cbindgen. r=heycam

Comment 8

[Emilio Cobos Álvarez (:emilio)]

leave-open to land the last bit.

Comment 9

[Cristian Brindusan [:cbrindusan]]

Backed out 5 changesets (Bug 1496486) for build bustages on gfxUserFontSet.h.

Backout: https://hg.mozilla.org/integration/autoland/rev/be1729663405c3f81c34ef5008cda8893d32e04e

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending,running,success,testfailed,busted,exception&selectedJob=204233309&revision=6b740afea4037c8d83d8ff3329a0c72689f13867

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=204233309&repo=autoland&lineNumber=33074

Comment 10

[Narcis Beleuzu [:NarcisB]]

There were also bustages on FontFaceSet.cpp
Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=204234332&repo=autoland&lineNumber=26438

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Wow, somehow managed to break all builds but linux64, which is what I pushed with...

There's a mixture of:

 * Old libclang causing bindgen issues.
 * Missing include in gfxUserFontSet.h
 * Some compiler seems unhappy about doing switch on an enum class, and it doesn't consider it exhaustive enough... I'll add a MOZ_ASSERT_UNREACHABLE or something.

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=7193661b37b069c36b19dd518d8238c8d5856088

Should be enough to fix it.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8d8e2f3f00b
Remove a bunch of unused nsCSSValue code. r=heycam
https://hg.mozilla.org/integration/autoland/rev/4dd15fa31474
Remove nsCSSValue usage from font code. r=heycam
https://hg.mozilla.org/integration/autoland/rev/2f629a60f12c
Remove GridTemplateAreas stuff from nsCSSValue. r=heycam
https://hg.mozilla.org/integration/autoland/rev/7f843f4ee162
Remove some more leftover code. r=heycam
https://hg.mozilla.org/integration/autoland/rev/d2f1e35ee4b7
Bump cbindgen. r=heycam

Comment 13

[Narcis Beleuzu [:NarcisB]]

Backed out for valgrind bustages

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=204250739&repo=autoland&lineNumber=43395

Comment 14

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/35f51b769efe
Backed out 5 changesets for valgrind bustages. CLOSED TREE

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Attachment: Valgrind suppresion.

I rewrote the computed value implementation to avoid allocating and this tricked
Valgrind.
    
There's nothing uninitialized or unsafe from the code in:
    
    https://hg.mozilla.org/integration/autoland/rev/4dd15fa31474#l28.68

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/02e58f0b7d60
Remove a bunch of unused nsCSSValue code. r=heycam
https://hg.mozilla.org/integration/autoland/rev/473ff44d5f12
Remove nsCSSValue usage from font code. r=heycam
https://hg.mozilla.org/integration/autoland/rev/9ac1296e4cc0
Remove GridTemplateAreas stuff from nsCSSValue. r=heycam
https://hg.mozilla.org/integration/autoland/rev/5febc1595fd4
Remove some more leftover code. r=heycam
https://hg.mozilla.org/integration/autoland/rev/836472045b3b
Bump cbindgen. r=heycam
https://hg.mozilla.org/integration/autoland/rev/8ecaee03d38e
Valgrind suppression. rs=froydnj

Comment 17

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/02e58f0b7d60
https://hg.mozilla.org/mozilla-central/rev/473ff44d5f12
https://hg.mozilla.org/mozilla-central/rev/9ac1296e4cc0
https://hg.mozilla.org/mozilla-central/rev/5febc1595fd4
https://hg.mozilla.org/mozilla-central/rev/836472045b3b
https://hg.mozilla.org/mozilla-central/rev/8ecaee03d38e

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Per discussion with jwatt I'll move the last patch to bug 1498571.