Removed ordering dependencies for EDisplay

Removed ordering dependencies for EDisplay

Removed ordering dependencies for EDisplay from CSSValueKeywords.h.
This involved updating ComputedStyleConstants.h to to not use arithmetic
operations to convert from CSSValue to the EDisplay enum, and removed
the comment requiring the enums match the order in CSSValueKeywords.in.

Also added a comment to clarify that the fast paths still rely on the
ordering, so not to update them unless the fast paths are updated too.

BUG=665272
Committed: https://crrev.com/a9bca4b5fe68c25941b4e89b19227e99d6d4df7e
Cr-Commit-Position: refs/heads/master@{#436863}

[sashab, 2016-12-05 04:48:38]

sashab@chromium.org changed reviewers:
+ alancutter@chromium.org

[sashab, 2016-12-05 04:48:38]

sashab@chromium.org changed reviewers:
+ alancutter@chromium.org

[alancutter (OOO until 2018), 2016-12-05 05:45:30]

Is CSS parsing performance unaffected by this change?

[alancutter (OOO until 2018), 2016-12-05 05:46:39]

On 2016/12/05 at 05:45:30, alancutter wrote:
> Is CSS parsing performance unaffected by this change?

Just realised this is for computed value -> CSSValue. This shouldn't have much
affect on performance.

lgtm

[alancutter (OOO until 2018), 2016-12-05 05:47:31]

On 2016/12/05 at 05:46:39, alancutter wrote:
> On 2016/12/05 at 05:45:30, alancutter wrote:
> > Is CSS parsing performance unaffected by this change?
> 
> Just realised this is for computed value -> CSSValue. This shouldn't have much
affect on performance.
> 
> lgtm

Sorry, I really can't read. This is CSSValue -> computed value. I think it's
worth checking the impact this will have on performance.

[sashab, 2016-12-07 02:48:45]

Thanks for the concern -- that was a good observartion.

I ran a small microbenchmark to compare the arithmetic cast with the switch
statement: http://ideone.com/tTo8XB

The results locally were:
╰─$ clang++ -O3 --std=c++11 benchmark.cpp -o benchmark.o && ./benchmark.o
Time (switch): 46.244 ms
Time (cast): 45.86 ms
33

╰─$ clang++ -O2 --std=c++11 benchmark.cpp -o benchmark.o && ./benchmark.o
Time (switch): 37.551 ms
Time (cast): 37.455 ms
33

╰─$ clang++ -O1 --std=c++11 benchmark.cpp -o benchmark.o && ./benchmark.o
Time (switch): 0.001 ms
Time (cast): 0 ms
33

I think because of the way my code is written, on -O1 I get 0ms for both loops.
But Chrome is compiled with -O2, and on that (and -O3) I get comparable runtimes
for both.

The perf bots are not working right now (crbug.com/671462) but I'm confident
this won't introduce any regression. When we come to generating this file in the
future, I'll be more concerned with the microbenchmarks and trying to get the
absolute fastest possible code.

Won't land until you acknowledge all this and LGTM again. :)

[alancutter (OOO until 2018), 2016-12-07 02:58:45]

lgtm

[sashab, 2016-12-07 03:02:00]

The CQ bit was checked by sashab@chromium.org

[commit-bot: I haz the power, 2016-12-07 03:02:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-07 04:48:52]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1481079720018680, "parent_rev":
"bbebeb6632561e91c77cbc59d905633d879cf5cd", "commit_rev":
"fa8ef40005cd69817d0e8bea2b182f930cd0c80e"}

[commit-bot: I haz the power, 2016-12-07 04:49:16]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-07 04:51:28]

Description was changed from

==========
Removed ordering dependencies for EDisplay

Removed ordering dependencies for EDisplay from CSSValueKeywords.h.
This involved updating ComputedStyleConstants.h to to not use arithmetic
operations to convert from CSSValue to the EDisplay enum, and removed
the comment requiring the enums match the order in CSSValueKeywords.in.

Also added a comment to clarify that the fast paths still rely on the
ordering, so not to update them unless the fast paths are updated too.

BUG=665272
==========

to

==========
Removed ordering dependencies for EDisplay

Removed ordering dependencies for EDisplay from CSSValueKeywords.h.
This involved updating ComputedStyleConstants.h to to not use arithmetic
operations to convert from CSSValue to the EDisplay enum, and removed
the comment requiring the enums match the order in CSSValueKeywords.in.

Also added a comment to clarify that the fast paths still rely on the
ordering, so not to update them unless the fast paths are updated too.

BUG=665272

Committed: https://crrev.com/a9bca4b5fe68c25941b4e89b19227e99d6d4df7e
Cr-Commit-Position: refs/heads/master@{#436863}
==========

[commit-bot: I haz the power, 2016-12-07 04:51:29]

Patchset 1 (id:??) landed as
https://crrev.com/a9bca4b5fe68c25941b4e89b19227e99d6d4df7e
Cr-Commit-Position: refs/heads/master@{#436863}