sync: Improve handling of bad UniquePos (retry)

sync: Improve handling of bad UniquePos (retry)

Retry: The first attempt had a memory leak.  It seems that this leak existed
before this CL, but no code exercised it.  This updated CL includes a scoped
deleter that should fix it.

Original commits message was:

Makes the client assign a valid position to incoming bookmarks if the
server has not populated the required fields.  This code should never be
triggered unless there is a bug in the server.  This risks reordering
users' bookmarks, but that's probably preferable to a crash.  This
fallback code is still protected by a NOTREACHED(), as before.

Detects bookmarks that do not have valid position information during
database load.  If these corrupted bookmarks are detected, the entire
database is declared to be corrupt.  Sync will then re-download
all of the user's data, which should fix the problem.

BUG=367247
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270837

[rlarocque, 2014-05-14 22:09:47]

This should fix the issue that resulted in the previous patch being reverted. 
The change is:

========
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index cebb2d4..e523bcd 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -167,6 +167,11 @@ DirOpenResult Directory::OpenImpl(
   // Temporary indices before kernel_ initialized in case Load fails. We 0(1)
   // swap these later.
   Directory::MetahandlesMap tmp_handles_map;
+
+  // Avoids mem leaks on failure.  Harmlessly deletes the empty hash map after
+  // the swap in the success case.
+  STLValueDeleter<Directory::MetahandlesMap> deleter(&tmp_handles_map);
+
   JournalIndex delete_journals;
 
   DirOpenResult result =
==========

Please review.

[maniscalco, 2014-05-14 22:34:33]

lgtm

[rlarocque, 2014-05-14 22:41:42]

The CQ bit was checked by rlarocque@chromium.org

[commit-bot: I haz the power, 2014-05-14 22:42:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rlarocque@chromium.org/283143002/1

[commit-bot: I haz the power, 2014-05-14 22:42:37]

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

[commit-bot: I haz the power, 2014-05-14 22:42:37]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[tim (not reviewing), 2014-05-15 20:17:16]

On 2014/05/14 22:42:37, I haz the power (commit-bot) wrote:
> No LGTM from a valid reviewer yet. Only full committers are accepted.
> Even if an LGTM may have been provided, it was from a non-committer or
> a provisional committer, _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.

rubber stamp lgtm

though I don't know if I trust that error message... maniscalco@ is a committer!

[rlarocque, 2014-05-15 20:21:59]

The CQ bit was checked by rlarocque@chromium.org

[commit-bot: I haz the power, 2014-05-15 20:22:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rlarocque@chromium.org/283143002/1

[commit-bot: I haz the power, 2014-05-15 21:49:11]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-15 22:47:22]

Change committed as 270837