The Color attributes for HTMLDocument should have [TreatNullAs=EmptyString]

The Color attributes for HTMLDocument should have [TreatNullAs=EmptyString]

As per spec link:
https://html.spec.whatwg.org/#Document-partial,
Color attributes for HTMLDocument should have
[TreatNullAs=EmptyString]. These change has no new
test to cover, so not added any new tests.

TEST= document-attribute-js-null.html and
element-attribute-js-null.html test would fail,
if the attributes were removed.

Firefox already suppourts these change.

BUG=460722
Committed: https://crrev.com/fa50402230d37afe53a6e2d15d36a8285e1081e0
git-svn-id: svn://svn.chromium.org/blink/trunk@199706 bbb929c8-8fbe-4397-9dbb-9b2b20218538

[shiva.jm, 2015-07-29 11:16:34]

shiva.jm@samsung.com changed reviewers:
+ philipj@opera.com, tkent@chromium.org

[shiva.jm, 2015-07-29 11:16:35]

pls have a look.

[philipj_slow, 2015-07-29 11:26:45]

Does the test fail without these changes? I don't think this can be observable,
unless somewhere internally a distinction is made between a null string and an
empty string.

There are many cases where we use [TreatNullAs=NullString] instead of
[TreatNullAs=EmptyString], and in some cases the difference can be seen in
microbenchmarks:
https://codereview.chromium.org/139293004/

Anyway, I think we should convert the cases where it doesn't matter, and this is
one such case.

[shiva.jm, 2015-07-29 11:35:47]

On 2015/07/29 11:26:45, philipj wrote:
> Does the test fail without these changes? I don't think this can be
observable,
> unless somewhere internally a distinction is made between a null string and an
> empty string.
> 
> There are many cases where we use [TreatNullAs=NullString] instead of
> [TreatNullAs=EmptyString], and in some cases the difference can be seen in
> microbenchmarks:
> https://codereview.chromium.org/139293004/
> 
> Anyway, I think we should convert the cases where it doesn't matter, and this
is
> one such case.

Yes tests passes without change as well, so we can remove tests and just keep
idl file changes?.

[philipj_slow, 2015-07-29 11:56:08]

On 2015/07/29 11:35:47, shiva.jm wrote:
> On 2015/07/29 11:26:45, philipj wrote:
> > Does the test fail without these changes? I don't think this can be
> observable,
> > unless somewhere internally a distinction is made between a null string and
an
> > empty string.
> > 
> > There are many cases where we use [TreatNullAs=NullString] instead of
> > [TreatNullAs=EmptyString], and in some cases the difference can be seen in
> > microbenchmarks:
> > https://codereview.chromium.org/139293004/
> > 
> > Anyway, I think we should convert the cases where it doesn't matter, and
this
> is
> > one such case.
> 
> Yes tests passes without change as well, so we can remove tests and just keep
> idl file changes?.

Yes, just the IDL changes are OK in this case, but it would be nice if you can
find some existing tests of these attributes that prove that the change doesn't
entirely break them. (It doesn't, of course.)

[shiva.jm, 2015-07-29 12:02:11]

updated patch, pls have a look.

[philipj_slow, 2015-07-29 12:08:56]

LGTM, but please also put
TEST=some-existing-test-that-would-fail-if-the-attributes-were-removed in the
description before landing.

[shiva.jm, 2015-07-30 03:54:25]

The CQ bit was checked by shiva.jm@samsung.com

[commit-bot: I haz the power, 2015-07-30 03:54:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1261773003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1261773003/60001

[commit-bot: I haz the power, 2015-07-30 05:14:06]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199706

[commit-bot: I haz the power, 2015-09-23 11:55:11]

Patchset 4 (id:??) landed as
https://crrev.com/fa50402230d37afe53a6e2d15d36a8285e1081e0