Mojo C++ bindings: Fix bug: array<>, map<> should only initialize elements if they're not null when…

Mojo C++ bindings: Fix bug: array<>, map<> should only initialize elements if they're not null when Deserialize()ing.
Simiarly, map's GetSerializedSize() tries to compute size of things that
are null (leads to a seg fault).  Fix this.

R=viettrungluu@chromium.org, jeffbrown@google.com
BUG=Fixes #578

Committed: https://chromium.googlesource.com/external/mojo/+/72eedddf746a2b62be78067a4c361cb172c71350

[viettrungluu, 2015-12-07 22:03:51]

lgtm w/nits

https://codereview.chromium.org/1509703002/diff/1/mojo/public/cpp/bindings/li...
File mojo/public/cpp/bindings/lib/array_serialization.h (right):

https://codereview.chromium.org/1509703002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/array_serialization.h:411: if (input != nullptr) {
nit: probably "if (input)" should be fine and more idiomatic?

https://codereview.chromium.org/1509703002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/array_serialization.h:420: if (input != nullptr) {
"

https://codereview.chromium.org/1509703002/diff/1/mojo/public/cpp/bindings/li...
File mojo/public/cpp/bindings/lib/map_serialization.h (right):

https://codereview.chromium.org/1509703002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/map_serialization.h:71: return !item.is_null()
nit: Is S not testable? (I.e., would |return item ? ... : ...;| work instead?)

[vardhan, 2015-12-07 22:43:32]

Description was changed from

==========
Mojo C++ bindings: Fix bug: array<>, map<> should only initialize elements if
they're not null when Deserialize()ing.
Simiarly, map's GetSerializedSize() tries to compute size of things that
are null (leads to a seg fault).  Fix this.

R=viettrungluu@chromium.org,jeffbrown@google.com
BUG=Fixes #578
==========

to

==========
Mojo C++ bindings: Fix bug: array<>, map<> should only initialize elements if
they're not null when Deserialize()ing.
Simiarly, map's GetSerializedSize() tries to compute size of things that
are null (leads to a seg fault).  Fix this.

R=viettrungluu@chromium.org, jeffbrown@google.com
BUG=Fixes #578

Committed:
https://chromium.googlesource.com/external/mojo/+/72eedddf746a2b62be78067a4c3...
==========

[vardhan, 2015-12-07 22:43:34]

Committed patchset #1 (id:1) manually as
72eedddf746a2b62be78067a4c361cb172c71350 (presubmit successful).

[vardhan, 2015-12-07 22:48:31]

(sorry, forgot to publish comments before submitting CL)

https://codereview.chromium.org/1509703002/diff/1/mojo/public/cpp/bindings/li...
File mojo/public/cpp/bindings/lib/array_serialization.h (right):

https://codereview.chromium.org/1509703002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/array_serialization.h:411: if (input != nullptr) {
On 2015/12/07 22:03:51, viettrungluu wrote:
> nit: probably "if (input)" should be fine and more idiomatic?

Done.

I was trying to be more explicit about what type |input| is (ie a  pointer), but
I realize now that that should be abstracted away, in the same way
|WrapperTraits<>::DataType| is abstracted away.

https://codereview.chromium.org/1509703002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/array_serialization.h:420: if (input != nullptr) {
On 2015/12/07 22:03:51, viettrungluu wrote:
> "

Done.

https://codereview.chromium.org/1509703002/diff/1/mojo/public/cpp/bindings/li...
File mojo/public/cpp/bindings/lib/map_serialization.h (right):

https://codereview.chromium.org/1509703002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/map_serialization.h:71: return !item.is_null()
On 2015/12/07 22:03:51, viettrungluu wrote:
> nit: Is S not testable? (I.e., would |return item ? ... : ...;| work instead?)

Done.
It's testable.  I was trying to be more explicit about |item| not being a
pointer, but now realize that should be abstracted away.