Adds TextStyle decoration, decorationColor, decorationStyle

Adds TextStyle decoration, decorationColor, decorationStyle

Currently only the decoration property is actually translated to a CSS style.

R=abarth@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/881e6e8cef188b6cff3d73ef9fd2739108927118

[hansmuller, 2015-06-18 23:47:07]

hansmuller@chromium.org changed reviewers:
+ ianh@google.com

[hansmuller, 2015-06-19 00:02:04]

hansmuller@chromium.org changed reviewers:
+ hansmuller@chromium.org

[hansmuller, 2015-06-19 00:02:04]

PTAL

[abarth-chromium, 2015-06-19 00:06:10]

abarth@chromium.org changed reviewers:
+ abarth@chromium.org

[abarth-chromium, 2015-06-19 00:06:10]

lgtm

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
File sky/sdk/lib/painting/text_style.dart (right):

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
sky/sdk/lib/painting/text_style.dart:63: }
Should we just use an assert instead?

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
sky/sdk/lib/painting/text_style.dart:71: final Object decoration; //
TextDecoration | List<TextDecoration>
Why not always a List<TextDecoration> ?

The reason not to do what you have here is that you're going to generate
polymorphic code, which is less efficient.  Whenever possible, we want to
generate monomorphic code.

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
sky/sdk/lib/painting/text_style.dart:82: }
I'd just remove this code, and the check in the constructor and rely upon the
type system to do its job.  That way the constructor can become const again,
which is quite valuable.

[Hixie, 2015-06-19 00:14:32]

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
File sky/sdk/lib/painting/text_style.dart (right):

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
sky/sdk/lib/painting/text_style.dart:71: final Object decoration; //
TextDecoration | List<TextDecoration>
You almost never want to see "Object", "dynamic", or "var". Using those means
you lose type checking, and it's likely the code will end up less efficient.

[Hixie, 2015-06-19 00:14:32]

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
File sky/sdk/lib/painting/text_style.dart (right):

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
sky/sdk/lib/painting/text_style.dart:71: final Object decoration; //
TextDecoration | List<TextDecoration>
You almost never want to see "Object", "dynamic", or "var". Using those means
you lose type checking, and it's likely the code will end up less efficient.

[hansmuller, 2015-06-19 17:29:40]

The TextStyle constructor is const again, the value of decoration: must be
List<TextDecoration> and I've added underline,overline,lineThrough
List<TextDecoration> constants.

Typically developers will specify text styles with just a single decoration,
like const TextStyle(decoration: underline).

In the relatively unusual case where it's necessary to specify a list of
TextDecorations, the following incorrect code will silently fail: new
TextStyle(decoration: [underline, overline]). That's because the decoration:
argument's type is List<dynamic>. If the list argument is explicitly typed, the
error will be detected: new TextStyle(decoration: const <TextDecoration>
[underline, overline]). The correct and safe variation is new
TextStyle(decoration: <TextDecoration>[TextDecoration.underline,
TextDecoration.overline]). Hopefully, few people will ever have to write that.

Perhaps we should lazily check decoration's value before it's converted to CSS.

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
File sky/sdk/lib/painting/text_style.dart (right):

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
sky/sdk/lib/painting/text_style.dart:63: }
On 2015/06/19 00:06:10, abarth wrote:
> Should we just use an assert instead?

I've made the decoration field's type List<TextDecoration>.

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
sky/sdk/lib/painting/text_style.dart:71: final Object decoration; //
TextDecoration | List<TextDecoration>
On 2015/06/19 00:06:10, abarth wrote:
> Why not always a List<TextDecoration> ?
> 
> The reason not to do what you have here is that you're going to generate
> polymorphic code, which is less efficient.  Whenever possible, we want to
> generate monomorphic code.

Done.

https://codereview.chromium.org/1174213005/diff/1/sky/sdk/lib/painting/text_s...
sky/sdk/lib/painting/text_style.dart:82: }
On 2015/06/19 00:06:10, abarth wrote:
> I'd just remove this code, and the check in the constructor and rely upon the
> type system to do its job.  That way the constructor can become const again,
> which is quite valuable.

Done.

[hansmuller1, 2015-06-19 17:30:55]

Committed patchset #2 (id:20001) manually as
881e6e8cef188b6cff3d73ef9fd2739108927118 (presubmit successful).