Added classes to enable session sync functionality....

Added classes to enable session sync functionality.

TEST=ProfileSyncServiceSessionTest
BUG=30519

[jerrica, 2010-08-13 20:49:38]

Hey guys,

When you get a chance, could you review these for me?

Thanks,

Jerrica

[Paweł Hajdan Jr., 2010-08-13 21:08:44]

Drive-by with a test comment. Please let me take another look before committing.

http://codereview.chromium.org/3168009/diff/55003/71029
File chrome/browser/sync/profile_sync_service_session_unittest.cc (right):

http://codereview.chromium.org/3168009/diff/55003/71029#newcode73
chrome/browser/sync/profile_sync_service_session_unittest.cc:73:
PathService::Get(base::DIR_TEMP, &path_);
The right way to do it is to use ScopedTempDir.

[tim (not reviewing), 2010-08-13 21:18:12]

On 2010/08/13 21:08:44, Paweł Hajdan Jr. wrote:
> Drive-by with a test comment. Please let me take another look before
committing.
> 
> http://codereview.chromium.org/3168009/diff/55003/71029
> File chrome/browser/sync/profile_sync_service_session_unittest.cc (right):
> 
> http://codereview.chromium.org/3168009/diff/55003/71029#newcode73
> chrome/browser/sync/profile_sync_service_session_unittest.cc:73:
> PathService::Get(base::DIR_TEMP, &path_);
> The right way to do it is to use ScopedTempDir.

We tried to do this for a pretty long time last night.  Regardless of where we
tried to reset the ScopedTempDir, every test would yield an error,
"ScopedTempDir was unable to delete <path>", from the dtor.  We modeled this
code after the identical code in SessionServiceTest and confirmed that the
PathDeleter was doing it's job.  I'm not familiar enough with the differences
between the two to come up with an explanation for why they differ. I would
guess that a handle is staying open somewhere, but I'm not sure why the two
approaches yield different results. The fact that the other test was using this
method that we implemented made it seem appropriate.

[Paweł Hajdan Jr., 2010-08-13 21:31:59]

Without ScopedTempDir you're going to run into other problems, like temporary
file pollution, or collision. Please do not commit without my "LGTM".

[jerrica, 2010-08-14 00:55:29]

Hey guys,

Could you please review my changes when you get a chance?

Thanks,

Jerrica

[jerrica, 2010-08-14 00:56:31]

I changed it to use the scoped version.

Please let me know what you think.

Thanks,

Jerrica

http://codereview.chromium.org/3168009/diff/55003/71029
File chrome/browser/sync/profile_sync_service_session_unittest.cc (right):

http://codereview.chromium.org/3168009/diff/55003/71029#newcode73
chrome/browser/sync/profile_sync_service_session_unittest.cc:73:
PathService::Get(base::DIR_TEMP, &path_);
On 2010/08/13 21:08:44, Paweł Hajdan Jr. wrote:
> The right way to do it is to use ScopedTempDir.

Done.

[tim (not reviewing), 2010-08-14 01:01:15]

Pending Pawel's approval, this LGTM based on our full in person review of the
code yesterday.

[tim (not reviewing), 2010-08-15 19:43:20]

Hi Pawel, Friday was jerrica's last day so we'll have to abandon this patch. 
I've re-uploaded as my own issue at http://codereview.chromium.org/3184004. 
Please review there at your earliest convenience.  Thanks.

[tim (not reviewing), 2010-08-15 19:43:21]

Hi Pawel, Friday was jerrica's last day so we'll have to abandon this patch. 
I've re-uploaded as my own issue at http://codereview.chromium.org/3184004. 
Please review there at your earliest convenience.  Thanks.