Avoid SYNC_CYCLE_CONTINUATION sync cycles

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

Index: chrome/browser/sync/test/integration/enable_disable_test.cc
diff --git a/chrome/browser/sync/test/integration/enable_disable_test.cc b/chrome/browser/sync/test/integration/enable_disable_test.cc
index dafcbb6a6ea84be9b731de5349413bfb1092d7e1..cce472c1907e4ea90f6f97ff5a25c09a046aea95 100644
--- a/chrome/browser/sync/test/integration/enable_disable_test.cc
+++ b/chrome/browser/sync/test/integration/enable_disable_test.cc
@@ -40,11 +40,11 @@ bool DoesTopLevelNodeExist(sync_api::UserShare* user_share,
 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();
@@ -73,11 +73,11 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, EnableOneAtATime) {
 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();