Make sure OneClickSigninSyncStarter detects Browser object being closed

chrome/browser/ui/sync/one_click_signin_sync_starter.cc

Index: chrome/browser/ui/sync/one_click_signin_sync_starter.cc
diff --git a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
index dc1b99838f723cee569fc91ca919e0446c5ea41f..64c4cd2e2e448745735f14b38121ef104465235a 100644
--- a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
+++ b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
@@ -24,7 +24,9 @@
 #include "chrome/browser/sync/sync_prefs.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_navigator.h"
+#include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/webui/signin/login_ui_service.h"
@@ -50,6 +52,8 @@ OneClickSigninSyncStarter::OneClickSigninSyncStarter(
       confirmation_required_(confirmation_required),
       weak_pointer_factory_(this) {
   DCHECK(profile);
+  BrowserList::AddObserver(this);
+
   Initialize(profile, browser);
 
   // Start the signin process using the cookies in the cookie jar.
@@ -62,7 +66,13 @@ OneClickSigninSyncStarter::OneClickSigninSyncStarter(
   manager->StartSignInWithCredentials(session_index, email, password, callback);
 }
 
+void OneClickSigninSyncStarter::OnBrowserRemoved(Browser* browser) {
+  if (browser == browser_)
+    browser_ = NULL;
+}
+
 OneClickSigninSyncStarter::~OneClickSigninSyncStarter() {
+  BrowserList::RemoveObserver(this);
 }
 
 void OneClickSigninSyncStarter::Initialize(Profile* profile, Browser* browser) {
@@ -241,8 +251,8 @@ void OneClickSigninSyncStarter::CompleteSigninForNewProfile(
 
 void OneClickSigninSyncStarter::ConfirmAndSignin() {
   SigninManager* signin = SigninManagerFactory::GetForProfile(profile_);
-  // browser_ can be null for unit tests.
-  if (browser_ && confirmation_required_ == CONFIRM_UNTRUSTED_SIGNIN) {
+  if (confirmation_required_ == CONFIRM_UNTRUSTED_SIGNIN) {

[fdoray, 2013/05/30 16:40:46]

The only unit test that involves this method is
OneClickSigninHelperTest.SigninFromWebstoreWithConfigSyncfirst. In this unit
test, confirmation_required_ == NO_CONFIRMATION, so I don't think we need need
the special case that we had before.

[Roger Tawa OOO till Jul 10th, 2013/05/30 18:14:49]

OK.  Sounds like the OneClickSigninHelper unit tests should actually be browser
tests.  There is a bug to implement browser tests for that code
(crbug.com/142487), so this should be fine.

+    EnsureBrowser();
     // Display a confirmation dialog to the user.
     browser_->window()->ShowOneClickSigninBubble(
         BrowserWindow::ONE_CLICK_SIGNIN_BUBBLE_TYPE_SAML_MODAL_DIALOG,
@@ -337,13 +347,14 @@ void OneClickSigninSyncStarter::DisplayFinalConfirmationBubble(
 
 void OneClickSigninSyncStarter::EnsureBrowser() {
   if (!browser_) {
-    // The user just created a new profile so we need to figure out what
-    // browser to use to display settings. Grab the most recently active
-    // browser or else create a new one.
+    // The user just created a new profile or has closed the browser that
+    // we used previously. Grab the most recently active browser or else
+    // create a new one.
     browser_ = chrome::FindLastActiveWithProfile(profile_, desktop_type_);
     if (!browser_) {
       browser_ = new Browser(Browser::CreateParams(profile_,
-                                                   desktop_type_));
+                                                   desktop_type_));
+      chrome::AddBlankTabAt(browser_, -1, true);
     }
     browser_->window()->Show();
   }