[sync] [chromeos] Fix setup flow when sync data is cleared via dashboard and sync is set up again

chrome/browser/ui/webui/sync_setup_handler_unittest.cc

Index: chrome/browser/ui/webui/sync_setup_handler_unittest.cc
diff --git a/chrome/browser/ui/webui/sync_setup_handler_unittest.cc b/chrome/browser/ui/webui/sync_setup_handler_unittest.cc
index 84d2bfcbc6314debd2fadd0a6072f46151af52c5..74ce58052a1c90a64a8808782e24cd7e750afc08 100644
--- a/chrome/browser/ui/webui/sync_setup_handler_unittest.cc
+++ b/chrome/browser/ui/webui/sync_setup_handler_unittest.cc
@@ -538,10 +538,18 @@ TEST_P(SyncSetupHandlerTest, DisplayConfigureWithBackendDisabledAndCancel) {
   // that won't finish for this test as we're simulating cancelling while the
   // spinner is showing.
   handler_->OpenSyncSetup(false);
-  EXPECT_EQ(handler_.get(),
+
+  // When the SigninTracker is initialized here, a signin failure is triggered
+  // due to sync_initialized() returning false, causing the current login UI to
+  // be dismissed.
+  EXPECT_EQ(NULL,
             LoginUIServiceFactory::GetForProfile(
                 profile_.get())->current_login_ui());
-  ASSERT_EQ(1U, web_ui_.call_data().size());
+
+  // We expect a call to SyncSetupOverlay.showSyncSetupPage. Some variations of
+  // this test also include a call to OptionsPage.closeOverlay, that we ignore.
+  ASSERT_LE(1U, web_ui_.call_data().size());

[James Hawkins, 2013/01/14 22:30:18]

EXPECT_LE

Here and the other ASSERTs you added.

[Raghu Simha, 2013/01/14 22:45:47]

Done.

+

[Roger Tawa OOO till Jul 10th, 2013/01/11 18:33:51]

The web-base sign in flow does not use the webui.  So it seems  it would be
better to do:

if (GetParam()) {
  ASSERT_EQ(1U, web_ui_.call_data().size());
} else {
  ASSERT_EQ(0U, web_ui_.call_data().size());
}

Similar for ASSERT_LE() below at line 605.

[Raghu Simha, 2013/01/11 18:53:04]

When I was debugging this test on the various buildbots, I noticed that the size
of call_data() at this spot was 2 for web ui (SyncSetupOverlay.showSyncSetupPage
and OptionsPage.closeOverlay) and 1 for client login
(SyncSetupOverlay.showSyncSetupPage). Does this not seem right to you?

   const TestWebUI::CallData& data = web_ui_.call_data()[0];
   EXPECT_EQ("SyncSetupOverlay.showSyncSetupPage", data.function_name);
   std::string page;
@@ -573,20 +581,29 @@ TEST_P(SyncSetupHandlerTest,
   SetDefaultExpectationsForConfigPage();
 
   handler_->OpenSyncSetup(false);
-  ASSERT_EQ(1U, web_ui_.call_data().size());
+
+  // We expect a call to SyncSetupOverlay.showSyncSetupPage. Some variations of
+  // this test also include a call to OptionsPage.closeOverlay, that we ignore.
+  ASSERT_LE(1U, web_ui_.call_data().size());
+
   const TestWebUI::CallData& data0 = web_ui_.call_data()[0];
   EXPECT_EQ("SyncSetupOverlay.showSyncSetupPage", data0.function_name);
   std::string page;
   ASSERT_TRUE(data0.arg1->GetAsString(&page));
   EXPECT_EQ(page, "spinner");
   handler_->SigninSuccess();
+
   // On signin success, the dialog will proceed from spinner to configure sync
-  // everything.
-  EXPECT_EQ(handler_.get(),
+  // everything. There is no login UI once signin is successful.
+  EXPECT_EQ(NULL,
             LoginUIServiceFactory::GetForProfile(
                 profile_.get())->current_login_ui());
-  ASSERT_EQ(2U, web_ui_.call_data().size());
-  const TestWebUI::CallData& data1 = web_ui_.call_data()[1];
+
+  // We expect a second call to SyncSetupOverlay.showSyncSetupPage. Some
+  // variations of this test also include a call to OptionsPage.closeOverlay,
+  // that we ignore.
+  ASSERT_LE(2U, web_ui_.call_data().size());

[Raghu Simha, 2013/01/11 18:53:04]

At this spot, the size of call_data() was 3 for web ui and 2 for client login
when I ran the test on linux. However, there was a different number of calls
when the tests ran with USE_AURA.

+  const TestWebUI::CallData& data1 = web_ui_.call_data().back();
   EXPECT_EQ("SyncSetupOverlay.showSyncSetupPage", data1.function_name);
   ASSERT_TRUE(data1.arg1->GetAsString(&page));
   EXPECT_EQ(page, "configure");