Support C++98 std::vector in mojo surfaces code

Support C++98 std::vector in mojo surfaces code

std::vector<>::data() is a C++11 feature and not available on all
toolchains. This uses &vector[0] instead.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284558

[darin (slow to review), 2014-07-14 19:58:23]

LGTM

[darin (slow to review), 2014-07-14 19:59:25]

Same question about other use of [0]

https://codereview.chromium.org/393583003/diff/1/mojo/services/public/cpp/sur...
File mojo/services/public/cpp/surfaces/lib/surfaces_type_converters.cc (right):

https://codereview.chromium.org/393583003/diff/1/mojo/services/public/cpp/sur...
mojo/services/public/cpp/surfaces/lib/surfaces_type_converters.cc:156:
&texture_quad_state->vertex_opacity.storage()[0],
Wait, do you need to ensure that vertex_opacity is non-empty? [0] will assert
non-empty.

[Peter Kasting, 2014-07-14 20:02:16]

https://codereview.chromium.org/393583003/diff/1/mojo/services/public/cpp/sur...
File mojo/services/public/cpp/surfaces/lib/surfaces_type_converters.cc (right):

https://codereview.chromium.org/393583003/diff/1/mojo/services/public/cpp/sur...
mojo/services/public/cpp/surfaces/lib/surfaces_type_converters.cc:156:
&texture_quad_state->vertex_opacity.storage()[0],
On 2014/07/14 19:59:25, darin wrote:
> Wait, do you need to ensure that vertex_opacity is non-empty? [0] will assert
> non-empty.

The standard says that data() is well-defined and gives you some valid pointer
even when the vector is empty.  So yeah, this changes behavior.

[jamesr, 2014-07-14 20:04:16]

On 2014/07/14 19:59:25, darin wrote:
> Same question about other use of [0]
> 
>
https://codereview.chromium.org/393583003/diff/1/mojo/services/public/cpp/sur...
> File mojo/services/public/cpp/surfaces/lib/surfaces_type_converters.cc
(right):
> 
>
https://codereview.chromium.org/393583003/diff/1/mojo/services/public/cpp/sur...
> mojo/services/public/cpp/surfaces/lib/surfaces_type_converters.cc:156:
> &texture_quad_state->vertex_opacity.storage()[0],
> Wait, do you need to ensure that vertex_opacity is non-empty? [0] will assert
> non-empty.

vertex_opacity is defined as

  float[4] vertex_opacity;

in the mojom bindings, so it could be empty or have 4 elements.  If it's empty
the message is bogus, however.  I'm not sure where this sort of validation
should happen for an incoming message, though, since it's an application-level
constraint.  Similarly a mailbox with an empty name field would be bogus.

Maybe type converters need a way to communicate errors?  It'd be a bit annoying
to have to walk for application-level validity and then walk again to do the
conversion.

[darin (slow to review), 2014-07-14 20:19:04]

On 2014/07/14 20:04:16, jamesr wrote:
> On 2014/07/14 19:59:25, darin wrote:
> > Same question about other use of [0]
> > 
> >
>
https://codereview.chromium.org/393583003/diff/1/mojo/services/public/cpp/sur...
> > File mojo/services/public/cpp/surfaces/lib/surfaces_type_converters.cc
> (right):
> > 
> >
>
https://codereview.chromium.org/393583003/diff/1/mojo/services/public/cpp/sur...
> > mojo/services/public/cpp/surfaces/lib/surfaces_type_converters.cc:156:
> > &texture_quad_state->vertex_opacity.storage()[0],
> > Wait, do you need to ensure that vertex_opacity is non-empty? [0] will
assert
> > non-empty.
> 
> vertex_opacity is defined as
> 
>   float[4] vertex_opacity;
> 
> in the mojom bindings, so it could be empty or have 4 elements.  If it's empty
> the message is bogus, however.  I'm not sure where this sort of validation
> should happen for an incoming message, though, since it's an application-level
> constraint.  Similarly a mailbox with an empty name field would be bogus.
> 
> Maybe type converters need a way to communicate errors?  It'd be a bit
annoying
> to have to walk for application-level validity and then walk again to do the
> conversion.

Oh, I see. There is no way to communicate errors from type converters. It is
assumed that the type you are converting to will either have a way to represent
the same states or that it will be reasonable to lose some information (e.g.,
converting a null mojo::String to an empty std::string).

If stricter validation is required, then we should probably look at making it
possible to extend the validation performed at the message validation level.
I could imagine an InterfacePtr<..> or InterfaceImpl<..> enabling you to add
a custom MessageValidator that runs after the normal set of validators.

[jamesr, 2014-07-21 20:50:26]

Sorry for the delay.  The most recent patch checks for empty arrays (and a few
other missing fields) and propagates that back to a null return value from the
converter.  It'd be nice to be able to do things like "fail to convert if XXX is
missing" in a more general way, but it depends on knowing the correct empty
value to use for the type which may be different (i.e. gpu::Mailbox is a value
type so NULL isn't really an option, etc).

[darin (slow to review), 2014-07-21 21:08:15]

LGTM

[jamesr, 2014-07-21 21:09:51]

The CQ bit was checked by jamesr@chromium.org

[commit-bot: I haz the power, 2014-07-21 21:11:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jamesr@chromium.org/393583003/40001

[commit-bot: I haz the power, 2014-07-22 00:29:33]

Change committed as 284558