Tweak memory management in the serializer.

Tweak memory management in the serializer.

R=vogelheim@chromium.org

Committed: https://crrev.com/ea02c3339d6461baddd5f1710ee03bfa13ffa029
Cr-Commit-Position: refs/heads/master@{#26947}

[vogelheim, 2015-02-25 14:00:19]

Maybe I'm overlooking something, but the &global_procy_object thing doesn't look
quite right to me.

https://codereview.chromium.org/957703003/diff/1/src/serialize.cc
File src/serialize.cc (right):

https://codereview.chromium.org/957703003/diff/1/src/serialize.cc#newcode711
src/serialize.cc:711: SetAttachedObjects(attached_objects);
I don't get this. My understanding is:

- SetAttachedObjects will make a (shallow) copy of attached_objects.
- So attached_objects can go out of scope, but its content can't.
- But global_proxy_object will go out of scope - it's stack allocated.

Won't Deserializer::attached_objects_ now have an invalid reference to it after
exiting this method?

[Yang, 2015-02-25 16:06:21]

https://codereview.chromium.org/957703003/diff/1/src/serialize.cc
File src/serialize.cc (right):

https://codereview.chromium.org/957703003/diff/1/src/serialize.cc#newcode711
src/serialize.cc:711: SetAttachedObjects(attached_objects);
On 2015/02/25 14:00:19, vogelheim wrote:
> I don't get this. My understanding is:
> 
> - SetAttachedObjects will make a (shallow) copy of attached_objects.
> - So attached_objects can go out of scope, but its content can't.
> - But global_proxy_object will go out of scope - it's stack allocated.
> 
> Won't Deserializer::attached_objects_ now have an invalid reference to it
after
> exiting this method?

You are right. It will indeed go out of scope, though it's not used outside of
this method, so everything still works. I'll restore the old behavior for
attached objects though.

[Yang, 2015-03-02 09:50:26]

On 2015/02/25 16:06:21, Yang wrote:
> https://codereview.chromium.org/957703003/diff/1/src/serialize.cc
> File src/serialize.cc (right):
> 
> https://codereview.chromium.org/957703003/diff/1/src/serialize.cc#newcode711
> src/serialize.cc:711: SetAttachedObjects(attached_objects);
> On 2015/02/25 14:00:19, vogelheim wrote:
> > I don't get this. My understanding is:
> > 
> > - SetAttachedObjects will make a (shallow) copy of attached_objects.
> > - So attached_objects can go out of scope, but its content can't.
> > - But global_proxy_object will go out of scope - it's stack allocated.
> > 
> > Won't Deserializer::attached_objects_ now have an invalid reference to it
> after
> > exiting this method?
> 
> You are right. It will indeed go out of scope, though it's not used outside of
> this method, so everything still works. I'll restore the old behavior for
> attached objects though.

Uploaded new patch set.

[vogelheim, 2015-03-02 17:51:24]

lgtm

[yangguo, 2015-03-02 19:46:57]

The CQ bit was checked by yangguo@google.com

[commit-bot: I haz the power, 2015-03-02 19:54:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/957703003/40001

[commit-bot: I haz the power, 2015-03-02 19:57:45]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-02 19:58:04]

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