Avoid SYNC_CYCLE_CONTINUATION sync cycles

chrome/browser/sync/test/integration/enable_disable_test.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/test/integration/sync_test.h"
 
 #include "chrome/browser/sync/profile_sync_service_harness.h"
 #include "chrome/browser/sync/internal_api/read_node.h"
 #include "chrome/browser/sync/internal_api/read_transaction.h"
 #include "chrome/browser/sync/syncable/model_type.h"
@@ skipping 22 matching lines @@
 bool DoesTopLevelNodeExist(sync_api::UserShare* user_share,
                            syncable::ModelType type) {
     sync_api::ReadTransaction trans(FROM_HERE, user_share);
     sync_api::ReadNode node(&trans);
     return node.InitByTagLookup(syncable::ModelTypeToRootTag(type));
 }
 
 IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, EnableOneAtATime) {
   ASSERT_TRUE(SetupClients());
 
-  DisableNotifications();
-
   // Setup sync with no enabled types.
   ASSERT_TRUE(GetClient(0)->SetupSync(syncable::ModelTypeSet()));
 
+  DisableNotifications();

[akalin, 2012/01/20 22:41:51]

why was this necessary?  i.e. why does setting up sync for a single client need
notifications to be enabled?  that seems weird.

[rlarocque, 2012/01/20 23:04:56]

The problem is that SetupSync() doesn't complete until the profile can pass the
IsFullySynced() test.  In order for that to happen, the harness has to see a
snapshot with num_unsynced_handles == 0, or, in other words, a non-committing
sync cycle.  The SYNC_CYCLE_CONTINUATION used to ensure this would always
happen.  I'm removing that (otherwise useless) sync cycle, so we need something
else to trigger a useless sync cycle to keep the integration tests happy.  

It might also be possible to make this work by relaxing the constraint in
SetupSync() to be IsDataSynced() rather than IsFullySynced().  However, I think
I tried that a while a ago and found it interacted badly with other tests.  This
seems like the easiest way to make things work for now, without significantly
reducing the coverage of the integration tests.  

See also crbug.com/97780, which tracks the efforts to fix these hacks.

[akalin, 2012/01/20 23:07:24]

Okay, if this is a temporary change, add a TODO to move it back and refer to the
bug.

+
   const syncable::ModelTypeSet registered_types =
       GetClient(0)->service()->GetRegisteredDataTypes();
   sync_api::UserShare* user_share = GetClient(0)->service()->GetUserShare();
   for (syncable::ModelTypeSet::Iterator it = registered_types.First();
        it.Good(); it.Inc()) {
     ASSERT_TRUE(GetClient(0)->EnableSyncForDatatype(it.Get()));
 
     // AUTOFILL_PROFILE is lumped together with AUTOFILL.
     if (it.Get() == syncable::AUTOFILL_PROFILE) {
       continue;
     }
 
     ASSERT_TRUE(DoesTopLevelNodeExist(user_share, it.Get()))
         << syncable::ModelTypeToString(it.Get());
 
     // AUTOFILL_PROFILE is lumped together with AUTOFILL.
     if (it.Get() == syncable::AUTOFILL) {
       ASSERT_TRUE(DoesTopLevelNodeExist(user_share,
                                         syncable::AUTOFILL_PROFILE));
     }
   }
 
   EnableNotifications();
 }
 
 IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) {
   ASSERT_TRUE(SetupClients());
 
-  DisableNotifications();
-
   // Setup sync with no disabled types.
   ASSERT_TRUE(GetClient(0)->SetupSync());
 
+  DisableNotifications();

[akalin, 2012/01/20 22:41:51]

why was this necessary?

[rlarocque, 2012/01/20 23:04:56]

Same reasoning as above.

[akalin, 2012/01/20 23:07:24]

add todo.

+
   const syncable::ModelTypeSet registered_types =
       GetClient(0)->service()->GetRegisteredDataTypes();
 
   sync_api::UserShare* user_share = GetClient(0)->service()->GetUserShare();
 
   // Make sure all top-level nodes exist first.
   for (syncable::ModelTypeSet::Iterator it = registered_types.First();
        it.Good(); it.Inc()) {
     ASSERT_TRUE(DoesTopLevelNodeExist(user_share, it.Get()));
   }
@@ skipping 17 matching lines @@
     if (it.Get() == syncable::AUTOFILL) {
       ASSERT_FALSE(DoesTopLevelNodeExist(user_share,
                                          syncable::AUTOFILL_PROFILE));
     }
   }
 
   EnableNotifications();
 }
 
 }  // namespace