add double support to base::Pickle

add double support to base::Pickle

Since WebTimeRange now uses doubles instead of floats[1], we should
probably add double support to base::Pickle.

[1] https://codereview.chromium.org/337923006

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

[Mostyn Bramley-Moore, 2014-07-14 12:25:31]

@acolwell: does this look OK to you?

@willchan: PTAL for OWNER approval.

[acolwell GONE FROM CHROMIUM, 2014-07-14 19:25:42]

lgtm % nit

How does WebTimeRange come into play here? Is something serializing it? Is
WebTimeRange really the first instance of double usage this class has had to
deal with?

https://codereview.chromium.org/388213002/diff/1/base/pickle.cc
File base/pickle.cc (right):

https://codereview.chromium.org/388213002/diff/1/base/pickle.cc#newcode123
base/pickle.cc:123: // The source data may not be properly aligned, and
unaligned float reads
nit: s/float/double/ ?

[Mostyn Bramley-Moore, 2014-07-14 19:48:03]

On 2014/07/14 19:25:42, acolwell wrote:
> How does WebTimeRange come into play here? Is something serializing it?

We have a set of local patches for video integration aimed at embedded linux
devices (which I assume you're not interested in) which include serializing
WebTimeRange.

> Is WebTimeRange really the first instance of double usage this class has had
to
> deal with?

As far as I know this is not done anywhere in mainline chromium yet, but
ipc/ipc_message_utils.h does have an explicit template specialization for
ParamTraits<double> at least, so it feels like something that should be
supported.

https://codereview.chromium.org/388213002/diff/1/base/pickle.cc
File base/pickle.cc (right):

https://codereview.chromium.org/388213002/diff/1/base/pickle.cc#newcode123
base/pickle.cc:123: // The source data may not be properly aligned, and
unaligned float reads
On 2014/07/14 19:25:42, acolwell wrote:
> nit: s/float/double/ ?

Done.

[acolwell GONE FROM CHROMIUM, 2014-07-14 20:15:54]

ok. Thanks for the info. :)

[willchan no longer on Chromium, 2014-07-14 20:22:33]

I'm going to send this to Darin instead. It weirds me out that double wasn't
added before, so I'm worried that it was by design for portability reasons.
Since the primary consumer is IPC, and Darin's much more familiar with that, I'm
sending it his way.

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

LGTM

I could have sworn I already wrote this CL.

[darin (slow to review), 2014-07-14 20:25:23]

On 2014/07/14 20:24:04, darin wrote:
> LGTM
> 
> I could have sworn I already wrote this CL.

In fact, this change would be useful in
content/common/page_state_serialization.cc. Take a look at the WriteReal
function.

[Mostyn Bramley-Moore, 2014-07-14 20:31:09]

On 2014/07/14 20:25:23, darin wrote:
> On 2014/07/14 20:24:04, darin wrote:
> > LGTM
> > 
> > I could have sworn I already wrote this CL.
> 
> In fact, this change would be useful in
> content/common/page_state_serialization.cc. Take a look at the WriteReal
> function.

If this lands, and sticks, I'll take a look at that.

[Mostyn Bramley-Moore, 2014-07-14 20:31:11]

The CQ bit was checked by mostynb@opera.com

[commit-bot: I haz the power, 2014-07-14 20:32:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mostynb@opera.com/388213002/20001

[commit-bot: I haz the power, 2014-07-14 22:50:36]

Change committed as 283064