[Sync] Reimplement proto value conversions on top of field visitors.

[Sync] Reimplement proto value conversions on top of field visitors.

Sync component can allocate nontrivial amounts of memory, and browser
memory team would like to add MemoryDumpProvider to track sync memory
usage.

While ~1/4 of all used memory comes from protobuf messages (see the bug),
there is no easy way to get memory usage of a proto message. All we can
do is to estimate on per-field basis. For that we need a way to enumerate
fields of a given proto.

Fortunately, proto_value_conversions.cc already has functions that go
through fields for all sync protos. The only problem is that those
functions (ProtoToValue) are tied to a specific purpose of generating
base::DictionaryValues.

This CL refactors field enumeration out of ProtoToValue functions so that
next CL could use that to estimate proto memory usage.

In particular, this CL:

1. Extracts field enumeration from ProtoToValue functions into
   VisitProtoFields(visitor, proto) function templates.

2. Implements ToValueVisitor that uses VisitProtoFields() to convert
   protos to base::DictionaryValues. ToValueVisitor also includes all
   logic from ProtoToValue functions.

3. Reimplements ProtoToValue() functions on top of ToValueVisitor.

Additionally this CL renames functions that convert proto enums to strings
to better play with templates.

BUG=649065
Committed: https://crrev.com/05476f9a8e8c076a8752b549d14269a6eca006c1
Cr-Commit-Position: refs/heads/master@{#426655}

[Dmitry Skiba, 2016-10-19 00:14:47]

The CQ bit was checked by dskiba@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-19 00:15:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-19 00:17:21]

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

[commit-bot: I haz the power, 2016-10-19 00:17:21]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Dmitry Skiba, 2016-10-19 08:53:53]

Description was changed from

==========
[Sync] Reimplement proto value conversions on top of field visitors.

BUG=649065
==========

to

==========
[Sync] Reimplement proto value conversions on top of field visitors.

Sync component can allocate nontrivial amounts of memory, and browser
memory team would like to add MemoryDumpProvider to track sync memory
usage.

While ~1/4 of all used memory comes from protobuf messages (see the bug),
there is no easy way to get memory usage of a proto message. All we can
do is to estimate on per-field basis. For that we need a way to enumerate
fields of a given proto.

Fortunately, proto_value_conversions.cc already has functions that go
through fields for all sync protos. The only problem is that those
functions (ProtoToValue) are tied to a specific purpose of generating
base::DictionaryValues.

This CL refactors field enumeration out of ProtoToValue functions so that
next CL could use that to estimate proto memory usage.

In particular, this CL:

1. Extracts field enumeration from ProtoToValue functions into
   VisitProtoFields(visitor, proto) function templates.

2. Implements ToValueVisitor that uses VisitProtoFields() to convert
   protos to base::DictionaryValues. ToValueVisitor also includes all
   logic from ProtoToValue functions.

3. Reimplements ProtoToValue() functions on top of ToValueVisitor.

Additionally this CL renames functions that convert proto enums to strings
to better play with templates.

BUG=649065
==========

[Dmitry Skiba, 2016-10-19 10:09:04]

The CQ bit was checked by dskiba@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-19 10:09:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dmitry Skiba, 2016-10-19 10:14:26]

dskiba@google.com changed reviewers:
+ pavely@chromium.org, zea@chromium.org

[Dmitry Skiba, 2016-10-19 10:14:27]

Guys please review.

As stated there will be another CL (which will look like
https://crrev.com/2382443006) that will use VisitProtoFields() functions to
estimate proto memory usage (as part of implementing MemoryDumpProvider for
sync).

Since the changes to proto_value_conversions.cc are so dramatic, and existing
tests don't cover all the fields, I added temporary tests that compare new
ToValue functions to their old variants on a set of randomly generated protos.
I'll remove these tests (and supporting files) before submitting:

protocol/proto_old_value_conversions.cc
protocol/proto_old_value_conversions.h
protocol/proto_compare_value_conversions_unittest.cc
protocol/proto_unittest_serialized_data.cc
protocol/proto_unittest_serialized_data.h

[commit-bot: I haz the power, 2016-10-19 10:46:49]

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

[commit-bot: I haz the power, 2016-10-19 10:46:50]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[pavely, 2016-10-20 05:10:40]

lgtm

https://codereview.chromium.org/2433563002/diff/40001/components/sync/protoco...
File components/sync/protocol/proto_visitors.h (right):

https://codereview.chromium.org/2433563002/diff/40001/components/sync/protoco...
components/sync/protocol/proto_visitors.h:1: // Copyright 2016 The Chromium
Authors. All rights reserved.
You need to add this file to BUILD.gn too.

[Dmitry Skiba, 2016-10-20 18:31:19]

Thanks for reviewing!

I want to see all bots green, after that I'll remove temporary tests and submit.

https://codereview.chromium.org/2433563002/diff/40001/components/sync/protoco...
File components/sync/protocol/proto_visitors.h (right):

https://codereview.chromium.org/2433563002/diff/40001/components/sync/protoco...
components/sync/protocol/proto_visitors.h:1: // Copyright 2016 The Chromium
Authors. All rights reserved.
On 2016/10/20 05:10:39, pavely wrote:
> You need to add this file to BUILD.gn too.

Done.

[Dmitry Skiba, 2016-10-20 19:58:35]

The CQ bit was checked by dskiba@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 19:59:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-20 21:27:07]

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

[commit-bot: I haz the power, 2016-10-20 21:27:08]

Dry run: This issue passed the CQ dry run.

[Dmitry Skiba, 2016-10-20 22:29:59]

The CQ bit was checked by dskiba@google.com

[Dmitry Skiba, 2016-10-20 22:29:59]

The patchset sent to the CQ was uploaded after l-g-t-m from pavely@chromium.org
Link to the patchset: https://codereview.chromium.org/2433563002/#ps100001
(title: "Remove temporary tests")

[commit-bot: I haz the power, 2016-10-20 22:30:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 00:08:31]

Description was changed from

==========
[Sync] Reimplement proto value conversions on top of field visitors.

Sync component can allocate nontrivial amounts of memory, and browser
memory team would like to add MemoryDumpProvider to track sync memory
usage.

While ~1/4 of all used memory comes from protobuf messages (see the bug),
there is no easy way to get memory usage of a proto message. All we can
do is to estimate on per-field basis. For that we need a way to enumerate
fields of a given proto.

Fortunately, proto_value_conversions.cc already has functions that go
through fields for all sync protos. The only problem is that those
functions (ProtoToValue) are tied to a specific purpose of generating
base::DictionaryValues.

This CL refactors field enumeration out of ProtoToValue functions so that
next CL could use that to estimate proto memory usage.

In particular, this CL:

1. Extracts field enumeration from ProtoToValue functions into
   VisitProtoFields(visitor, proto) function templates.

2. Implements ToValueVisitor that uses VisitProtoFields() to convert
   protos to base::DictionaryValues. ToValueVisitor also includes all
   logic from ProtoToValue functions.

3. Reimplements ProtoToValue() functions on top of ToValueVisitor.

Additionally this CL renames functions that convert proto enums to strings
to better play with templates.

BUG=649065
==========

to

==========
[Sync] Reimplement proto value conversions on top of field visitors.

Sync component can allocate nontrivial amounts of memory, and browser
memory team would like to add MemoryDumpProvider to track sync memory
usage.

While ~1/4 of all used memory comes from protobuf messages (see the bug),
there is no easy way to get memory usage of a proto message. All we can
do is to estimate on per-field basis. For that we need a way to enumerate
fields of a given proto.

Fortunately, proto_value_conversions.cc already has functions that go
through fields for all sync protos. The only problem is that those
functions (ProtoToValue) are tied to a specific purpose of generating
base::DictionaryValues.

This CL refactors field enumeration out of ProtoToValue functions so that
next CL could use that to estimate proto memory usage.

In particular, this CL:

1. Extracts field enumeration from ProtoToValue functions into
   VisitProtoFields(visitor, proto) function templates.

2. Implements ToValueVisitor that uses VisitProtoFields() to convert
   protos to base::DictionaryValues. ToValueVisitor also includes all
   logic from ProtoToValue functions.

3. Reimplements ProtoToValue() functions on top of ToValueVisitor.

Additionally this CL renames functions that convert proto enums to strings
to better play with templates.

BUG=649065
==========

[commit-bot: I haz the power, 2016-10-21 00:08:36]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-21 13:24:49]

Description was changed from

==========
[Sync] Reimplement proto value conversions on top of field visitors.

Sync component can allocate nontrivial amounts of memory, and browser
memory team would like to add MemoryDumpProvider to track sync memory
usage.

While ~1/4 of all used memory comes from protobuf messages (see the bug),
there is no easy way to get memory usage of a proto message. All we can
do is to estimate on per-field basis. For that we need a way to enumerate
fields of a given proto.

Fortunately, proto_value_conversions.cc already has functions that go
through fields for all sync protos. The only problem is that those
functions (ProtoToValue) are tied to a specific purpose of generating
base::DictionaryValues.

This CL refactors field enumeration out of ProtoToValue functions so that
next CL could use that to estimate proto memory usage.

In particular, this CL:

1. Extracts field enumeration from ProtoToValue functions into
   VisitProtoFields(visitor, proto) function templates.

2. Implements ToValueVisitor that uses VisitProtoFields() to convert
   protos to base::DictionaryValues. ToValueVisitor also includes all
   logic from ProtoToValue functions.

3. Reimplements ProtoToValue() functions on top of ToValueVisitor.

Additionally this CL renames functions that convert proto enums to strings
to better play with templates.

BUG=649065
==========

to

==========
[Sync] Reimplement proto value conversions on top of field visitors.

Sync component can allocate nontrivial amounts of memory, and browser
memory team would like to add MemoryDumpProvider to track sync memory
usage.

While ~1/4 of all used memory comes from protobuf messages (see the bug),
there is no easy way to get memory usage of a proto message. All we can
do is to estimate on per-field basis. For that we need a way to enumerate
fields of a given proto.

Fortunately, proto_value_conversions.cc already has functions that go
through fields for all sync protos. The only problem is that those
functions (ProtoToValue) are tied to a specific purpose of generating
base::DictionaryValues.

This CL refactors field enumeration out of ProtoToValue functions so that
next CL could use that to estimate proto memory usage.

In particular, this CL:

1. Extracts field enumeration from ProtoToValue functions into
   VisitProtoFields(visitor, proto) function templates.

2. Implements ToValueVisitor that uses VisitProtoFields() to convert
   protos to base::DictionaryValues. ToValueVisitor also includes all
   logic from ProtoToValue functions.

3. Reimplements ProtoToValue() functions on top of ToValueVisitor.

Additionally this CL renames functions that convert proto enums to strings
to better play with templates.

BUG=649065

Committed: https://crrev.com/05476f9a8e8c076a8752b549d14269a6eca006c1
Cr-Commit-Position: refs/heads/master@{#426655}
==========

[commit-bot: I haz the power, 2016-10-21 13:24:51]

Patchset 6 (id:??) landed as
https://crrev.com/05476f9a8e8c076a8752b549d14269a6eca006c1
Cr-Commit-Position: refs/heads/master@{#426655}