Added classes to enable session sync functionality.

Added classes to enable session sync functionality.

TEST=ProfileSyncServiceSessionTest
BUG=30519
Original patch by jerrica@chromium.org
Original review: http://codereview.chromium.org/3168009

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=56423

[tim (not reviewing), 2010-08-15 19:53:52]

+Pawel to continue his review of unittests from original patch.

[Paweł Hajdan Jr., 2010-08-16 18:20:16]

Tests LGTM. Looks like the intern has fixed it nicely. :)

[tim (not reviewing), 2010-08-17 07:03:07]

On 2010/08/16 18:20:16, Paweł Hajdan Jr. wrote:
> Tests LGTM. Looks like the intern has fixed it nicely. :)

+akalin for model_type.cc changes.  I'm not sure I understand the "hack" added
in a recent patch that I just merged on rebase.  Can you please review?

[tim (not reviewing), 2010-08-17 07:03:43]

[actually +akalin now]

[akalin, 2010-08-17 07:06:42]

basically, 'UNKNOWN' is just another data type for which tango send
notifications (it is sent for anything tango doesn't recognize, including
top-level nodes and nodes for new (i.e., unknown to cosmo) data types

http://codereview.chromium.org/3184004/diff/15001/16034
File chrome/browser/sync/syncable/model_type.cc (left):

http://codereview.chromium.org/3184004/diff/15001/16034#oldcode239
chrome/browser/sync/syncable/model_type.cc:239: return true;
put this back in

http://codereview.chromium.org/3184004/diff/15001/16034
File chrome/browser/sync/syncable/model_type.cc (right):

http://codereview.chromium.org/3184004/diff/15001/16034#newcode169
chrome/browser/sync/syncable/model_type.cc:169: const char
kSessionNotificationType[] = "SESSION";
move this before unknown

[tim (not reviewing), 2010-08-17 07:29:36]

Is it cause for concern that NotificationTypeToReal and RealToNotification
aren't inverses right now?  one maps the SESSIONS type to "unknown" while the
other does map "sessions" to SESSIONS.

http://codereview.chromium.org/3184004/diff/15001/16034
File chrome/browser/sync/syncable/model_type.cc (left):

http://codereview.chromium.org/3184004/diff/15001/16034#oldcode239
chrome/browser/sync/syncable/model_type.cc:239: return true;
On 2010/08/17 07:06:42, akalin wrote:
> put this back in

Done.

http://codereview.chromium.org/3184004/diff/15001/16034
File chrome/browser/sync/syncable/model_type.cc (right):

http://codereview.chromium.org/3184004/diff/15001/16034#newcode169
chrome/browser/sync/syncable/model_type.cc:169: const char
kSessionNotificationType[] = "SESSION";
On 2010/08/17 07:06:42, akalin wrote:
> move this before unknown

Done.

[akalin, 2010-08-17 07:32:18]

Yeah, they should be inverses.

http://codereview.chromium.org/3184004/diff/21001/22034
File chrome/browser/sync/syncable/model_type.cc (right):

http://codereview.chromium.org/3184004/diff/21001/22034#newcode205
chrome/browser/sync/syncable/model_type.cc:205: case SESSIONS:
add mapping to kSessionsNotifcationType here

[tim (not reviewing), 2010-08-17 15:17:03]

http://codereview.chromium.org/3184004/diff/21001/22034
File chrome/browser/sync/syncable/model_type.cc (right):

http://codereview.chromium.org/3184004/diff/21001/22034#newcode205
chrome/browser/sync/syncable/model_type.cc:205: case SESSIONS:
On 2010/08/17 07:32:18, akalin wrote:
> add mapping to kSessionsNotifcationType here

I was doing that at first, but if not sessions then what constitutes a "new"
data type that should use 'unknown"?

[akalin, 2010-08-17 17:15:49]

On 2010/08/17 15:17:03, timsteele wrote:
> http://codereview.chromium.org/3184004/diff/21001/22034
> File chrome/browser/sync/syncable/model_type.cc (right):
> 
> http://codereview.chromium.org/3184004/diff/21001/22034#newcode205
> chrome/browser/sync/syncable/model_type.cc:205: case SESSIONS:
> On 2010/08/17 07:32:18, akalin wrote:
> > add mapping to kSessionsNotifcationType here
> 
> I was doing that at first, but if not sessions then what constitutes a "new"
> data type that should use 'unknown"?

So "new" means "recently added, but Cosmo doesn't know about it".  So you should
add cases for your new datatype client-side, but until Cosmo does a deploy which
includes it, notifications for it will get marked as "unknown" instead of the
real data type.

[tim (not reviewing), 2010-08-17 20:51:58]

On 2010/08/17 17:15:49, akalin wrote:
> On 2010/08/17 15:17:03, timsteele wrote:
> > http://codereview.chromium.org/3184004/diff/21001/22034
> > File chrome/browser/sync/syncable/model_type.cc (right):
> > 
> > http://codereview.chromium.org/3184004/diff/21001/22034#newcode205
> > chrome/browser/sync/syncable/model_type.cc:205: case SESSIONS:
> > On 2010/08/17 07:32:18, akalin wrote:
> > > add mapping to kSessionsNotifcationType here
> > 
> > I was doing that at first, but if not sessions then what constitutes a "new"
> > data type that should use 'unknown"?
> 
> So "new" means "recently added, but Cosmo doesn't know about it".  So you
should
> add cases for your new datatype client-side, but until Cosmo does a deploy
which
> includes it, notifications for it will get marked as "unknown" instead of the
> real data type.

Added the SESSIONS clause.

[akalin, 2010-08-17 21:34:47]

LGTM

http://codereview.chromium.org/3184004/diff/17005/26034
File chrome/browser/sync/syncable/model_type.cc (right):

http://codereview.chromium.org/3184004/diff/17005/26034#newcode251
chrome/browser/sync/syncable/model_type.cc:251: else if (notification_type ==
kUnknownNotificationType) {
move "else if" to be on previous line, i.e. "} else if"