s/Value/value/g for type trait value field in WebGLImageConversion.cpp

s/Value/value/g for type trait value field in WebGLImageConversion.cpp

Lowercase |value| is desirable:

- For consistency with other type traits:
  - in WTF - e.g. WTF::IsAssignable<T>::value
  - in standard library - e.g. std::is_base_of<T1, T2>::value
  - in v8 - e.g. IsIntegerArithmeticSafe<T, Lhs, Rhs>::value

- To help |rewrite_to_chrome_style| tool.

BUG=640749
Committed: https://crrev.com/1c4e7baae394ba46842f177de2829face052faa5
Cr-Commit-Position: refs/heads/master@{#418134}

[Łukasz Anforowicz, 2016-09-09 00:06:04]

lukasza@chromium.org changed reviewers:
+ danakj@chromium.org

[Łukasz Anforowicz, 2016-09-09 00:06:05]

Dana, can you take a look please?

[danakj, 2016-09-09 00:07:40]

https://codereview.chromium.org/2325563005/diff/1/tools/clang/rewrite_to_chro...
File tools/clang/rewrite_to_chrome_style/tests/fields-expected.cc (right):

https://codereview.chromium.org/2325563005/diff/1/tools/clang/rewrite_to_chro...
tools/clang/rewrite_to_chrome_style/tests/fields-expected.cc:100: static const
bool Value = true;
shouldn't it be |value| ?

[danakj, 2016-09-09 00:08:31]

https://codereview.chromium.org/2325563005/diff/1/tools/clang/rewrite_to_chro...
File tools/clang/rewrite_to_chrome_style/tests/fields-expected.cc (right):

https://codereview.chromium.org/2325563005/diff/1/tools/clang/rewrite_to_chro...
tools/clang/rewrite_to_chrome_style/tests/fields-expected.cc:100: static const
bool Value = true;
On 2016/09/09 00:07:40, danakj wrote:
> shouldn't it be |value| ?

Maybe we should just change these to lowercase up front and then it works? I
think that matches most other type traits?

[Łukasz Anforowicz, 2016-09-09 16:53:47]

Description was changed from

==========
Avoid rewriting type traits where the field is spelled |Value|.

third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp
contains multiple type traits where the value field is named |Value|
(with uppercase V).  We should stop renaming this field, just
as we don't rename the fields in other type traits (i.e. in
WTF::IsCopyAssignable<T>).

After the fix, the tool won't anymore rename Value -> kValue in type
traits.  Without this fix, Value -> kValue happens in some places:
    template<int Format>
    struct Is16bppFormat {
        STATIC_ONLY(Is16bppFormat);
        static const bool kValue =
            Format == WebGLImageConversion::kDataFormatRGBA5551
            || Format == WebGLImageConversion::kDataFormatRGBA4444
            || Format == WebGLImageConversion::kDataFormatRGB565;
    };
but not in other places (because heuristics in
https://crrev.com/2274653002 and https://crrev.com/2256913002
are not currently capable of recognizing [based solely on the name
and expression] whether they are dealing with a static field that
should have "k" prefix prepended):
    template<int Format,
        bool IsInt8Format = IsInt8Format<Format>::Value,  // <- no rewrite.
        ...>
    struct DataTypeForFormat {

BUG=640749
==========

to

==========
s/Value/value/g for type trait value field in WebGLImageConversion.cpp

Lowercase |value| is desirable:

- For consistency with other type traits:
  - in WTF - e.g. WTF::IsAssignable<T>::value
  - in standard library - e.g. std::is_base_of<T1, T2>::value
  - in v8 - e.g. IsIntegerArithmeticSafe<T, Lhs, Rhs>::value

- To help |rewrite_to_chrome_style| tool.

BUG=640749
==========

[Łukasz Anforowicz, 2016-09-09 16:54:13]

Description was changed from

==========
s/Value/value/g for type trait value field in WebGLImageConversion.cpp

Lowercase |value| is desirable:

- For consistency with other type traits:
  - in WTF - e.g. WTF::IsAssignable<T>::value
  - in standard library - e.g. std::is_base_of<T1, T2>::value
  - in v8 - e.g. IsIntegerArithmeticSafe<T, Lhs, Rhs>::value

- To help |rewrite_to_chrome_style| tool.

BUG=640749
==========

to

==========
s/Value/value/g for type trait value field in WebGLImageConversion.cpp

Lowercase |value| is desirable:

- For consistency with other type traits:
  - in WTF - e.g. WTF::IsAssignable<T>::value
  - in standard library - e.g. std::is_base_of<T1, T2>::value
  - in v8 - e.g. IsIntegerArithmeticSafe<T, Lhs, Rhs>::value

- To help |rewrite_to_chrome_style| tool.

BUG=640749
==========

[Łukasz Anforowicz, 2016-09-09 16:55:46]

Thanks for the review - I've switched this CL to be just a manual rename of
|Value| into |value|.

https://codereview.chromium.org/2325563005/diff/1/tools/clang/rewrite_to_chro...
File tools/clang/rewrite_to_chrome_style/tests/fields-expected.cc (right):

https://codereview.chromium.org/2325563005/diff/1/tools/clang/rewrite_to_chro...
tools/clang/rewrite_to_chrome_style/tests/fields-expected.cc:100: static const
bool Value = true;
On 2016/09/09 00:08:31, danakj wrote:
> On 2016/09/09 00:07:40, danakj wrote:
> > shouldn't it be |value| ?
> 
> Maybe we should just change these to lowercase up front and then it works? I
> think that matches most other type traits?

Good point.  I've tried to look for other cases of |Value| static field under
blink and is seems that platform/graphics/gpu/WebGLImageConversion.cpp is the
only case.

[Łukasz Anforowicz, 2016-09-09 16:56:24]

lukasza@chromium.org changed reviewers:
+ chrishtr@chromium.org

[Łukasz Anforowicz, 2016-09-09 16:56:24]

chrishtr@, could you please take a look?

[chrishtr, 2016-09-09 17:54:57]

Is this compatible with the chromium style guide? Deferring to danakj on that
question..

[danakj, 2016-09-09 18:28:00]

It's common practice to make type traits like this. Examples:
https://cs.chromium.org/chromium/src/base/template_util.h?rcl=0&l=112
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/wtf/TypeTraits...

I think technically the style guide would say kValue but we super don't do that
anywhere for type traits, so this lgtm

[Łukasz Anforowicz, 2016-09-09 18:29:59]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 18:30:20]

Dry run: 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-09-09 20:18:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 20:18:44]

Dry run: This issue passed the CQ dry run.

[chrishtr, 2016-09-12 23:11:55]

lgtm

[Łukasz Anforowicz, 2016-09-12 23:40:27]

The CQ bit was checked by lukasza@chromium.org

[commit-bot: I haz the power, 2016-09-12 23:40:54]

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-09-13 01:30:11]

Description was changed from

==========
s/Value/value/g for type trait value field in WebGLImageConversion.cpp

Lowercase |value| is desirable:

- For consistency with other type traits:
  - in WTF - e.g. WTF::IsAssignable<T>::value
  - in standard library - e.g. std::is_base_of<T1, T2>::value
  - in v8 - e.g. IsIntegerArithmeticSafe<T, Lhs, Rhs>::value

- To help |rewrite_to_chrome_style| tool.

BUG=640749
==========

to

==========
s/Value/value/g for type trait value field in WebGLImageConversion.cpp

Lowercase |value| is desirable:

- For consistency with other type traits:
  - in WTF - e.g. WTF::IsAssignable<T>::value
  - in standard library - e.g. std::is_base_of<T1, T2>::value
  - in v8 - e.g. IsIntegerArithmeticSafe<T, Lhs, Rhs>::value

- To help |rewrite_to_chrome_style| tool.

BUG=640749
==========

[commit-bot: I haz the power, 2016-09-13 01:30:12]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-13 01:31:58]

Description was changed from

==========
s/Value/value/g for type trait value field in WebGLImageConversion.cpp

Lowercase |value| is desirable:

- For consistency with other type traits:
  - in WTF - e.g. WTF::IsAssignable<T>::value
  - in standard library - e.g. std::is_base_of<T1, T2>::value
  - in v8 - e.g. IsIntegerArithmeticSafe<T, Lhs, Rhs>::value

- To help |rewrite_to_chrome_style| tool.

BUG=640749
==========

to

==========
s/Value/value/g for type trait value field in WebGLImageConversion.cpp

Lowercase |value| is desirable:

- For consistency with other type traits:
  - in WTF - e.g. WTF::IsAssignable<T>::value
  - in standard library - e.g. std::is_base_of<T1, T2>::value
  - in v8 - e.g. IsIntegerArithmeticSafe<T, Lhs, Rhs>::value

- To help |rewrite_to_chrome_style| tool.

BUG=640749

Committed: https://crrev.com/1c4e7baae394ba46842f177de2829face052faa5
Cr-Commit-Position: refs/heads/master@{#418134}
==========

[commit-bot: I haz the power, 2016-09-13 01:32:00]

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