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 df57a20dbb1f9e0c9f3e633ba09de9bf7ce83e85..0c49c49e422b4ed41bab7a87ccbd95d4c7b143a8 100644
--- a/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
+++ b/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
@@ -25,7 +25,9 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.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/tabs/tab_strip_model.h"
@@ -52,6 +54,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.
@@ -64,7 +68,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) {
@@ -263,8 +273,8 @@ void OneClickSigninSyncStarter::CompleteInitForNewProfile(
 
 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) {
+    EnsureBrowser();
     // Display a confirmation dialog to the user.
     browser_->window()->ShowOneClickSigninBubble(
         BrowserWindow::ONE_CLICK_SIGNIN_BUBBLE_TYPE_SAML_MODAL_DIALOG,
@@ -357,13 +367,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);

[Andrew T Wilson (Slow), 2013/06/05 08:43:51]

Why is this necessary? This code was working properly before in the case where
we create a new profile.

[(NOT FOR CODE REVIEWS), 2013/06/05 14:11:34]

Maybe something changed since the time you looked at this?  Francois showed me
that without this line, he would see a new window with only the "New tab" button
shown, no actual tab.

[fdoray, 2013/06/05 15:09:04]

When you create a new profile as part of the sign in process, a new browser with
a new tab page is created before EnsureBrowser is called.
chrome::FindLastActiveWithProfile returns this browser and the line I added is
not executed. That's why the code seemed to work before. As Roger said, if I
remove the call to chrome::AddBlankTabAt, you get a window with no tab when the
"if" block is actually executed.

     }
     browser_->window()->Show();
   }