bindings: Stop using Nullable<T> in dictionary impl

bindings: Stop using Nullable<T> in dictionary impl

Nullable<T> requires complete definition of T.
This could be a problem when we want to use forward declaration
to avoid cycler dependency. Stop using Nullable<T> and use flags
instead to implement hasXXX() methods.

BUG=662403
Committed: https://crrev.com/827df14d32e901f5b7fee5a4e8fb05a56754ef44
Cr-Commit-Position: refs/heads/master@{#431720}

[bashi, 2016-11-11 05:37:54]

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

[commit-bot: I haz the power, 2016-11-11 05:38:05]

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

[bashi, 2016-11-11 07:05:36]

bashi@chromium.org changed reviewers:
+ haraken@chromium.org, peria@chromium.org, yukishiino@chromium.org

[bashi, 2016-11-11 07:05:37]

PTAL

Split from https://codereview.chromium.org/2471393004/

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/scripts/v8_dictionary.py (right):

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/v8_dictionary.py:224:
'has_flag_variable': has_flag_variable,
Will re-order this.

[commit-bot: I haz the power, 2016-11-11 07:10:07]

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

[commit-bot: I haz the power, 2016-11-11 07:10:08]

Dry run: This issue passed the CQ dry run.

[Yuki, 2016-11-11 07:34:39]

LGTM.

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/scripts/v8_dictionary.py (right):

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/v8_dictionary.py:224:
'has_flag_variable': has_flag_variable,
What about |nullable_indicator_name|?

[bashi, 2016-11-11 07:44:11]

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

[commit-bot: I haz the power, 2016-11-11 07:44:24]

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

[peria, 2016-11-11 07:45:16]

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/scripts/v8_dictionary.py (right):

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/v8_dictionary.py:191:
has_flag_variable = None
This variable name is confusing.

It is a |has_flag| in the context of template,
and it also figures the |variable|'s name in .h/.cpp.

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/templates/dictionary_impl.cpp.tmpl
(right):

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/templates/dictionary_impl.cpp.tmpl:38: {%
else %}
{%endif%} here.

otherwise, it keeps unused m_{{member.cpp_name}}.

[bashi, 2016-11-11 07:51:59]

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

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

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

[bashi, 2016-11-11 07:52:57]

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/scripts/v8_dictionary.py (right):

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/v8_dictionary.py:224:
'has_flag_variable': has_flag_variable,
On 2016/11/11 07:34:39, Yuki wrote:
> What about |nullable_indicator_name|?

Done.

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/templates/dictionary_impl.cpp.tmpl
(right):

https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/templates/dictionary_impl.cpp.tmpl:38: {%
else %}
On 2016/11/11 07:45:16, peria wrote:
> {%endif%} here.
> 
> otherwise, it keeps unused m_{{member.cpp_name}}.

IIUC the purpose of this method is make hasXXX() return false and in that case
accessing m_xxx is illegal anyway. Added DCHECK().

[peria, 2016-11-11 07:59:23]

LGTM

On 2016/11/11 07:52:57, bashi1 wrote:
>
https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
> File third_party/WebKit/Source/bindings/templates/dictionary_impl.cpp.tmpl
> (right):
> 
>
https://codereview.chromium.org/2494973002/diff/1/third_party/WebKit/Source/b...
> third_party/WebKit/Source/bindings/templates/dictionary_impl.cpp.tmpl:38: {%
> else %}
> On 2016/11/11 07:45:16, peria wrote:
> > {%endif%} here.
> > 
> > otherwise, it keeps unused m_{{member.cpp_name}}.
> 
> IIUC the purpose of this method is make hasXXX() return false and in that case
> accessing m_xxx is illegal anyway. Added DCHECK().

I see.  I meant if it could leak memory, but we don't have to clear it
if the behavior is not in spec.

[haraken, 2016-11-11 08:54:48]

LGTM

[commit-bot: I haz the power, 2016-11-11 09:24:37]

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

[commit-bot: I haz the power, 2016-11-11 09:24:39]

Dry run: This issue passed the CQ dry run.

[bashi, 2016-11-12 01:09:34]

The CQ bit was checked by bashi@chromium.org

[bashi, 2016-11-12 01:09:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/2494973002/#ps40001
(title: "Comments")

[commit-bot: I haz the power, 2016-11-12 01:10:06]

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-11-12 01:16:32]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-12 01:19:05]

Description was changed from

==========
bindings: Stop using Nullable<T> in dictionary impl

Nullable<T> requires complete definition of T.
This could be a problem when we want to use forward declaration
to avoid cycler dependency. Stop using Nullable<T> and use flags
instead to implement hasXXX() methods.

BUG=662403
==========

to

==========
bindings: Stop using Nullable<T> in dictionary impl

Nullable<T> requires complete definition of T.
This could be a problem when we want to use forward declaration
to avoid cycler dependency. Stop using Nullable<T> and use flags
instead to implement hasXXX() methods.

BUG=662403

Committed: https://crrev.com/827df14d32e901f5b7fee5a4e8fb05a56754ef44
Cr-Commit-Position: refs/heads/master@{#431720}
==========

[commit-bot: I haz the power, 2016-11-12 01:19:06]

Patchset 3 (id:??) landed as
https://crrev.com/827df14d32e901f5b7fee5a4e8fb05a56754ef44
Cr-Commit-Position: refs/heads/master@{#431720}