Fix EntryKernel leak if sync database load fails

Fix leaks found by sync unit tests.

1. Fix EntryKernel leak if sync database load fails.
2. DirectoryBackingStore: always close the load handle.
3. DirectoryBackingStore unittests: free when .clear()ing test index
4. Fix real bad leak in Directory::PurgeEntriesWithTypeIn.

BUG=50334, 50335, 50336, 50347, 50348, 50349
TEST=sync_unit_tests

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

[ncarter (slow), 2010-07-28 18:35:02]

Patch set updated, please take a look.

[tim (not reviewing), 2010-07-28 19:00:54]

http://codereview.chromium.org/3026029/diff/7001/8001
File chrome/browser/sync/syncable/directory_backing_store.cc (right):

http://codereview.chromium.org/3026029/diff/7001/8001#newcode228
chrome/browser/sync/syncable/directory_backing_store.cc:228: if
(!LoadEntries(entry_bucket))
I don't think this is what you were suggesting before as another option (sorry
if I misunderstood), but looking at this, shouldn't we either do the
error-check-and-cleanup in LoadEntries, or do the following in here (DoLoad) to
nip it in the bud?

MetahandlesIndex rv;
STLElementDeleter d(&rv);
{
...
if (!LoadEntries(&rv))
  return FAILED
}

entry_bucket->swap(rv);
return OPENED;

[tim (not reviewing), 2010-07-28 19:16:01]

LGTM

[ncarter (slow), 2010-07-28 19:17:19]

http://codereview.chromium.org/3026029/diff/7001/8001
File chrome/browser/sync/syncable/directory_backing_store.cc (right):

http://codereview.chromium.org/3026029/diff/7001/8001#newcode228
chrome/browser/sync/syncable/directory_backing_store.cc:228: if
(!LoadEntries(entry_bucket))
On 2010/07/28 19:00:54, timsteele wrote:
> I don't think this is what you were suggesting before as another option (sorry
> if I misunderstood), but looking at this, shouldn't we either do the
> error-check-and-cleanup in LoadEntries, or do the following in here (DoLoad)
to
> nip it in the bud?
> 
> MetahandlesIndex rv;
> STLElementDeleter d(&rv);
> {
> ...
> if (!LoadEntries(&rv))
>   return FAILED
> }
> 
> entry_bucket->swap(rv);
> return OPENED;
> 

It's a clever technique, but we need to handle the failure higher up: there
could be a failure later (e.g. in LoadInfo).  My preference is to leave things
as they are.

[Timur Iskhodzhanov, 2010-07-29 07:19:22]

Thanks for fixing the leaks!