IDL: Clean-up "should use Nullable<>" logic a little

IDL: Clean-up "should use Nullable<>" logic a little

Some duplication had sneaked in as part of the IDL dictionary work, with
IdlTypeBase.impl_should_use_nullable_container meaning almost the same
thing as IdlTypeBase.is_explicit_nullable, except the latter also implies
that the type is nullable.

Common up the actual logic in the new IdlTypeBase.cpp_type_has_null_value
property, and use it for both impl_should_use_nullable_container and
is_{implicit,explicit}_nullable.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180287

[Jens Widell, 2014-08-14 11:08:38]

bashi@: PTAL and comment on whether you think the logic is still correct for the
use in v8_dictionary.py.

[bashi, 2014-08-14 13:16:06]

On 2014/08/14 11:08:38, Jens Widell wrote:
> bashi@: PTAL and comment on whether you think the logic is still correct for
the
> use in v8_dictionary.py.

LGTM. Thanks!

I think we can remove impl_should_use_nullable_container. I'll do it later, but
feel free to do it in this CL if you want:)

[Jens Widell, 2014-08-14 13:24:50]

On 2014/08/14 13:16:06, bashi1 wrote:
> I think we can remove impl_should_use_nullable_container. I'll do it later,
but
> feel free to do it in this CL if you want:)

I kept it around for readability / as documentation, essentially. For instance,

    def getter_expression():
        if idl_type.impl_should_use_nullable_container:
            return 'm_%s.get()' % member.name
        return 'm_%s' % member.name

is somewhat more obvious when you read it than

    def getter_expression():
        if not idl_type.cpp_type_has_null_value:
            return 'm_%s.get()' % member.name
        return 'm_%s' % member.name

would be, IMO. Or we could add some helpful comments in the code. :-)

[haraken, 2014-08-14 13:29:23]

LGTM2

[Jens Widell, 2014-08-14 13:36:52]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2014-08-14 13:38:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jl@opera.com/472803002/1

[bashi, 2014-08-14 13:53:31]

On 2014/08/14 13:24:50, Jens Widell wrote:
> On 2014/08/14 13:16:06, bashi1 wrote:
> > I think we can remove impl_should_use_nullable_container. I'll do it later,
> but
> > feel free to do it in this CL if you want:)
> 
> I kept it around for readability / as documentation, essentially. For
instance,
> 
>     def getter_expression():
>         if idl_type.impl_should_use_nullable_container:
>             return 'm_%s.get()' % member.name
>         return 'm_%s' % member.name
> 
> is somewhat more obvious when you read it than
> 
>     def getter_expression():
>         if not idl_type.cpp_type_has_null_value:
>             return 'm_%s.get()' % member.name
>         return 'm_%s' % member.name
> 
> would be, IMO. Or we could add some helpful comments in the code. :-)

makes sense. Thank you for the explanation!

[commit-bot: I haz the power, 2014-08-14 14:42:09]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_db...)
  android_blink_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_re...)
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/1...)
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)
  linux_chromium_gn_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_chromium_gn_rel/bu...)
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19832)
  win_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22775)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/41718)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/46936)

[commit-bot: I haz the power, 2014-08-14 14:43:57]

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

[commit-bot: I haz the power, 2014-08-14 14:43:58]

Try jobs failed on following builders:
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19836)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/41734)

[Jens Widell, 2014-08-14 15:56:14]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2014-08-14 15:57:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jl@opera.com/472803002/1

[commit-bot: I haz the power, 2014-08-14 17:32:58]

Committed patchset #1 (1) as 180287