[Sync] Add support for associating a new Synced Bookmarks node.

Previously we would result in an unrecoverable error when the server sends
us a newly created Synced Bookmarks node. We now properly detect and associate
it, allowing us to also associate new bookmarks created under the Synced
Bookmarks folder.

In addition, we soften our conditions for unrecoverable errors. A bookmark
that cannot be added is ignored, instead of triggering an error. There are
very few of these, and are likely triggered by a crash during initial
association. 

BUG=108978
TEST=sync_integration_tests


Review URL: http://codereview.chromium.org/9570055

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127101 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 93 insertions, 14 deletions

diff --git a/net/tools/testserver/chromiumsync.py b/net/tools/testserver/chromiumsync.py
index f8d7a5a..752d6a4 100755
--- a/net/tools/testserver/chromiumsync.py
+++ b/net/tools/testserver/chromiumsync.py
@@ -264,13 +264,18 @@ class PermanentItem(object):
     sync_type: A value from ALL_TYPES, giving the datatype of this permanent
       item.  This controls which types of client GetUpdates requests will
       cause the permanent item to be created and returned.
+    create_by_default: Whether the permanent item is created at startup or not.
+      This value is set to True in the default case. Non-default permanent items
+      are those that are created only when a client explicitly tells the server
+      to do so.
   """
 
-  def __init__(self, tag, name, parent_tag, sync_type):
+  def __init__(self, tag, name, parent_tag, sync_type, create_by_default=True):
     self.tag = tag
     self.name = name
     self.parent_tag = parent_tag
     self.sync_type = sync_type
+    self.create_by_default = create_by_default
 
 
 class MigrationHistory(object):
@@ -409,6 +414,9 @@ class SyncDataModel(object):
                     parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK),
       PermanentItem('other_bookmarks', name='Other Bookmarks',
                     parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK),
+      PermanentItem('synced_bookmarks', name='Synced Bookmarks',
+                    parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK,
+                    create_by_default=False),
       PermanentItem('google_chrome_preferences', name='Preferences',
                     parent_tag='google_chrome', sync_type=PREFERENCE),
       PermanentItem('google_chrome_autofill', name='Autofill',
@@ -595,15 +603,15 @@ class SyncDataModel(object):
     self._WritePosition(entry, self._ServerTagToId(spec.parent_tag))
     self._SaveEntry(entry)
 
-  def _CreatePermanentItems(self, requested_types):
-    """Ensure creation of all permanent items for a given set of sync types.
+  def _CreateDefaultPermanentItems(self, requested_types):
+    """Ensure creation of all default permanent items for a given set of types.
 
     Args:
       requested_types: A list of sync data types from ALL_TYPES.
-        Permanent items of only these types will be created.
+        All default permanent items of only these types will be created.
     """
     for spec in self._PERMANENT_ITEM_SPECS:
-      if spec.sync_type in requested_types:
+      if spec.sync_type in requested_types and spec.create_by_default:
         self._CreatePermanentItem(spec)
 
   def ResetStoreBirthday(self):
@@ -634,7 +642,7 @@ class SyncDataModel(object):
     if not sieve.HasAnyTimestamp():
       return (0, [], 0)
     min_timestamp = sieve.GetMinTimestamp()
-    self._CreatePermanentItems(sieve.GetFirstTimeTypes())
+    self._CreateDefaultPermanentItems(sieve.GetFirstTimeTypes())
     change_log = sorted(self._entries.values(),
                         key=operator.attrgetter('version'))
     new_changes = [x for x in change_log if x.version > min_timestamp]
@@ -925,6 +933,18 @@ class SyncDataModel(object):
     nigori_new.specifics.nigori.sync_tabs = True
     self._SaveEntry(nigori_new)
 
+  def TriggerCreateSyncedBookmarks(self):
+    """Create the Synced Bookmarks folder under the Bookmarks permanent item.
+
+    Clients will then receive the Synced Bookmarks folder on future
+    GetUpdates, and new bookmarks can be added within the Synced Bookmarks
+    folder.
+    """
+
+    synced_bookmarks_spec, = [spec for spec in self._PERMANENT_ITEM_SPECS
+                              if spec.tag == "synced_bookmarks"]
+    self._CreatePermanentItem(synced_bookmarks_spec)
+
   def SetInducedError(self, error, error_frequency,
                       sync_count_before_errors):
     self.induced_error = error
@@ -1074,6 +1094,13 @@ class TestServer(object):
         200,
         '<html><title>Sync Tabs</title><H1>Sync Tabs</H1></html>')
 
+  def HandleCreateSyncedBookmarks(self):
+    """Create the Synced Bookmarks folder under Bookmarks."""
+    self.account.TriggerCreateSyncedBookmarks()
+    return (
+        200,
+        '<html><title>Synced Bookmarks</title><H1>Synced Bookmarks</H1></html>')
+
   def HandleCommand(self, query, raw_request):
     """Decode and handle a sync command from a raw input of bytes.
 
diff --git a/net/tools/testserver/chromiumsync_test.py b/net/tools/testserver/chromiumsync_test.py
index 799d5d6..2485b0f 100755
--- a/net/tools/testserver/chromiumsync_test.py
+++ b/net/tools/testserver/chromiumsync_test.py
@@ -17,6 +17,8 @@ import theme_specifics_pb2
 class SyncDataModelTest(unittest.TestCase):
   def setUp(self):
     self.model = chromiumsync.SyncDataModel()
+    # The Synced Bookmarks folder is not created by default
+    self._expect_synced_bookmarks_folder = False
 
   def AddToModel(self, proto):
     self.model._entries[proto.id_string] = proto
@@ -67,13 +69,16 @@ class SyncDataModelTest(unittest.TestCase):
     self.assertEqual(2, self.model._entries[proto.id_string].version)
 
   def testCreatePermanentItems(self):
-    self.model._CreatePermanentItems(chromiumsync.ALL_TYPES)
+    self.model._CreateDefaultPermanentItems(chromiumsync.ALL_TYPES)
     self.assertEqual(len(chromiumsync.ALL_TYPES) + 2,
                      len(self.model._entries))
 
   def ExpectedPermanentItemCount(self, sync_type):
     if sync_type == chromiumsync.BOOKMARK:
-      return 4
+      if self._expect_synced_bookmarks_folder:
+        return 5
+      else:
+        return 4
     elif sync_type == chromiumsync.TOP_LEVEL:
       return 1
     else:
@@ -107,8 +112,13 @@ class SyncDataModelTest(unittest.TestCase):
       # Doing a wider GetUpdates from timestamp zero shouldn't recreate either.
       new_version, changes, remaining = (
           self.GetChangesFromTimestamp(all_types, 0))
-      self.assertEqual(len(chromiumsync.SyncDataModel._PERMANENT_ITEM_SPECS),
-                       new_version)
+      if self._expect_synced_bookmarks_folder:
+        self.assertEqual(len(chromiumsync.SyncDataModel._PERMANENT_ITEM_SPECS),
+                         new_version)
+      else:
+        self.assertEqual(
+            len(chromiumsync.SyncDataModel._PERMANENT_ITEM_SPECS) -1,
+            new_version)
       self.assertEqual(new_version, len(changes))
       self.assertEqual(0, remaining)
       version, changes, remaining = (
@@ -561,6 +571,35 @@ class SyncDataModelTest(unittest.TestCase):
     sieve = chromiumsync.UpdateSieve(msg, migrator)
     sieve.CheckMigrationState()
 
+  def testCreateSyncedBookmaks(self):
+    version1, changes, remaining = (
+        self.GetChangesFromTimestamp([chromiumsync.BOOKMARK], 0))
+    id_string = self.model._MakeCurrentId(chromiumsync.BOOKMARK,
+                                          '<server tag>synced_bookmarks')
+    self.assertFalse(self.model._ItemExists(id_string))
+    self._expect_synced_bookmarks_folder = True
+    self.model.TriggerCreateSyncedBookmarks()
+    self.assertTrue(self.model._ItemExists(id_string))
+
+    # Check that the version changed when the folder was created and the only
+    # change was the folder creation.
+    version2, changes, remaining = (
+        self.GetChangesFromTimestamp([chromiumsync.BOOKMARK], version1))
+    self.assertEqual(len(changes), 1)
+    self.assertEqual(changes[0].id_string, id_string)
+    self.assertNotEqual(version1, version2)
+    self.assertEqual(
+        self.ExpectedPermanentItemCount(chromiumsync.BOOKMARK),
+        version2)
+
+    # Ensure getting from timestamp 0 includes the folder.
+    version, changes, remaining = (
+        self.GetChangesFromTimestamp([chromiumsync.BOOKMARK], 0))
+    self.assertEqual(
+        self.ExpectedPermanentItemCount(chromiumsync.BOOKMARK),
+        len(changes))
+    self.assertEqual(version2, version)
+
 
 if __name__ == '__main__':
   unittest.main()
diff --git a/net/tools/testserver/testserver.py b/net/tools/testserver/testserver.py
index 28405ff..11a7ad8 100755
--- a/net/tools/testserver/testserver.py
+++ b/net/tools/testserver/testserver.py
@@ -1649,16 +1649,17 @@ class SyncPageHandler(BasePageHandler):
   """Handler for the main HTTP sync server."""
 
   def __init__(self, request, client_address, sync_http_server):
-    get_handlers = [self.ChromiumSyncMigrationOpHandler,
-                    self.ChromiumSyncTimeHandler,
+    get_handlers = [self.ChromiumSyncTimeHandler,
+                    self.ChromiumSyncMigrationOpHandler,
+                    self.ChromiumSyncCredHandler,
                     self.ChromiumSyncDisableNotificationsOpHandler,
                     self.ChromiumSyncEnableNotificationsOpHandler,
                     self.ChromiumSyncSendNotificationOpHandler,
                     self.ChromiumSyncBirthdayErrorOpHandler,
                     self.ChromiumSyncTransientErrorOpHandler,
-                    self.ChromiumSyncSyncTabsOpHandler,
                     self.ChromiumSyncErrorOpHandler,
-                    self.ChromiumSyncCredHandler]
+                    self.ChromiumSyncSyncTabsOpHandler,
+                    self.ChromiumSyncCreateSyncedBookmarksOpHandler]
 
     post_handlers = [self.ChromiumSyncCommandHandler,
                      self.ChromiumSyncTimeHandler]
@@ -1858,6 +1859,18 @@ class SyncPageHandler(BasePageHandler):
     self.wfile.write(raw_reply)
     return True;
 
+  def ChromiumSyncCreateSyncedBookmarksOpHandler(self):
+    test_name = "/chromiumsync/createsyncedbookmarks"
+    if not self._ShouldHandleRequest(test_name):
+      return False
+    result, raw_reply = self.server._sync_handler.HandleCreateSyncedBookmarks()
+    self.send_response(result)
+    self.send_header('Content-Type', 'text/html')
+    self.send_header('Content-Length', len(raw_reply))
+    self.end_headers()
+    self.wfile.write(raw_reply)
+    return True;
+
 
 def MakeDataDir():
   if options.data_dir: