1224464 - convert keyword tables in nsCSSProps to use a struct

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The keyword tables (tables of KTableValue) in nsCSSProps currently use arrays where the even elements of the array are the keywords and the odd elements are the values.  This is silly; it should just be a struct.

I have patches to do this.

I might end up making bug 1223653 depend on this, although I'm not entirely sure how yet.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=26c5cf8c22e7&group_state=expanded

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: MozReview Request: Bug 1224464 patch 1 - Make nsCSSKeyword explicitly int16_t. r?heycam

Bug 1224464 patch 1 - Make nsCSSKeyword explicitly int16_t.  r?heycam

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: MozReview Request: Bug 1224464 patch 2 - Make nsCSSProps keyword tables be arrays of structs, to represent what they logically are. r?heycam

Bug 1224464 patch 2 - Make nsCSSProps keyword tables be arrays of structs, to represent what they logically are.  r?heycam

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: MozReview Request: Bug 1224464 patch 3 - Rename KTableValue to KTableEntry now that it is a struct. r?heycam

Bug 1224464 patch 3 - Rename KTableValue to KTableEntry now that it is a struct.  r?heycam

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8687150 [details]
MozReview Request: Bug 1224464 patch 1 - Make nsCSSKeyword explicitly int16_t.  r?heycam

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25117/diff/1-2/

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8687151 [details]
MozReview Request: Bug 1224464 patch 2 - Make nsCSSProps keyword tables be arrays of structs, to represent what they logically are.  r?heycam

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25119/diff/1-2/

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8687152 [details]
MozReview Request: Bug 1224464 patch 3 - Rename KTableValue to KTableEntry now that it is a struct.  r?heycam

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25121/diff/1-2/

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: MozReview Request: Bug 1223653 patch 1 - Fix incorrect function being called, caught by enum class type checking in next patch. r?jfkthame

Bug 1223653 patch 1 - Fix incorrect function being called, caught by enum class type checking in next patch.  r?jfkthame

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: MozReview Request: Bug 1223653 patch 2 - Use an enum class for NS_STYLE_BOX_SIZING_*. r?heycam

Bug 1223653 patch 2 - Use an enum class for NS_STYLE_BOX_SIZING_*.  r?heycam

The casts in nsCSSProps.cpp (defining kBoxSizingKTable) and in
nsComputedDOMStyle::DoGetBoxSizing (using
nsCSSProps::ValueToKeywordEnum) are a little bit annoying, though aren't
a net reduction in typesafety.

The casts in nsRuleNode.cpp (SetDiscrete) are a little more annoying,
though the change in this patch should be sufficient for converting all
properties -- but that may also mean reducing typesafety a bit for all
properties.

I'd like to find something better to do about them, but I think I'm ok
landing this before doing that.  Bug 1224918 covers doing better.

Comment 10

[Cameron McCormack (:heycam)]

Comment on attachment 8687150 [details]
MozReview Request: Bug 1224464 patch 1 - Make nsCSSKeyword explicitly int16_t.  r?heycam

https://reviewboard.mozilla.org/r/25117/#review22719

Comment 11

[Cameron McCormack (:heycam)]

Comment on attachment 8687151 [details]
MozReview Request: Bug 1224464 patch 2 - Make nsCSSProps keyword tables be arrays of structs, to represent what they logically are.  r?heycam

https://reviewboard.mozilla.org/r/25119/#review22721

::: layout/inspector/inDOMUtils.cpp:588
(Diff revision 2)
>        // element is a nsCSSKeyword.

This comment can be removed.

Comment 12

[Cameron McCormack (:heycam)]

Comment on attachment 8687152 [details]
MozReview Request: Bug 1224464 patch 3 - Rename KTableValue to KTableEntry now that it is a struct.  r?heycam

https://reviewboard.mozilla.org/r/25121/#review22723

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2cb53b74518d271a2874502ad129b7b4f799c5ae
Bug 1224464 patch 1 - Make nsCSSKeyword explicitly int16_t.  r=heycam

https://hg.mozilla.org/integration/mozilla-inbound/rev/fbde9f78976bf61462c3069abdbe87a37303ea59
Bug 1224464 patch 2 - Make nsCSSProps keyword tables be arrays of structs, to represent what they logically are.  r=heycam

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4f4af049f7d08ccf6d74e45b8aee9ac79801ebf
Bug 1224464 patch 3 - Rename KTableValue to KTableEntry now that it is a struct.  r=heycam

Comment 14

[Xidorn Quan [:xidorn] UTC+11]

https://hg.mozilla.org/integration/mozilla-inbound/rev/80dc69d21994a554b3041c5ebc7f5572e4a42c09
Bug 1224464 followup - Fix bustage on opt builds on CLOSED TREE

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2cb53b74518d
https://hg.mozilla.org/mozilla-central/rev/fbde9f78976b
https://hg.mozilla.org/mozilla-central/rev/b4f4af049f7d
https://hg.mozilla.org/mozilla-central/rev/80dc69d21994