Delay loading the NTP after sign in until MergeSession has been performed in

chrome/browser/signin/signin_tracker.cc

Index: chrome/browser/signin/signin_tracker.cc
diff --git a/chrome/browser/signin/signin_tracker.cc b/chrome/browser/signin/signin_tracker.cc
index 65c8154aa1229d2bbcbb913c8a6fb406ceb2e3f3..be3367786bab9294abc33e14e0d786740ba74a2b 100644
--- a/chrome/browser/signin/signin_tracker.cc
+++ b/chrome/browser/signin/signin_tracker.cc
@@ -6,10 +6,13 @@
 
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/account_reconcilor_factory.h"
+#include "chrome/browser/signin/google_auto_login_helper.h"
 #include "chrome/browser/signin/profile_oauth2_token_service.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/common/profile_management_switches.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "google_apis/gaia/gaia_constants.h"
@@ -23,6 +26,14 @@ SigninTracker::SigninTracker(Profile* profile, Observer* observer)
 SigninTracker::~SigninTracker() {
   ProfileOAuth2TokenServiceFactory::GetForProfile(profile_)->
       RemoveObserver(this);
+
+  if (merge_session_helper_)
+    merge_session_helper_->RemoveObserver(this);
+
+  if (switches::IsEnableInlineSignin() && switches::IsNewProfileManagement()) {
+      AccountReconcilorFactory::GetForProfile(profile_)->
+          RemoveMergeSessionObserver(this);
+  }
 }
 
 void SigninTracker::Initialize() {
@@ -52,9 +63,25 @@ void SigninTracker::Observe(int type,
 void SigninTracker::OnRefreshTokenAvailable(const std::string& account_id) {
   // TODO: when OAuth2TokenService handles multi-login, this should check
   // that |account_id| is the primary account before signalling success.
+  if (switches::IsEnableInlineSignin()) {
+    if (switches::IsNewProfileManagement()) {
+      AccountReconcilorFactory::GetForProfile(profile_)->
+          AddMergeSessionObserver(this);
+    } else {
+      merge_session_helper_.reset(new GoogleAutoLoginHelper(profile_, this));

[guohui, 2013/12/20 16:11:09]

It looks wierd to me to create a GoogleAutoLoginHelper and to start merge
session in SigninTracker. My understanding of SigninTracker is that it is an
observer helper for signin, any class that is interested in monitoring signin
events could create their own instance of SigninTracker and get notification
when certain signin events are triggered, however SIgninTracker itself should
not make any change to the signin state. Otherwise multiple SIgninTracker may
intervene with each other, resulting in multiple instances of
GogleAutoLoginHelper, which is exactly what this CL aims to fix.

I think it is better to ensure there is always at most one instance of
GoogleAutoLoginHelper. A few random ideas:

1. make GoogleAutoLoginHelper a PSK, in both mirror and non-mirror world
SIgninTrackers listens directly to GoogleAutoLoginHelper. May be an overkill.
2. attach GoogleAutoLoginHelper to an existing PSK. You already attach
GoogleAutoLoginHelper to AccountReconciler in the mirror world, maybe you can do
the same for non-mirror and attach GoogleAutoLoginHelper to another PKS, such as
SigninManager or PO2TS.
3. enables part of the functionality, such as MergeSession, of AccountReconciler
in non-mirror world. Not sure if this is possible at all, just list it here for
completeness.

[Roger Tawa OOO till Jul 10th, 2013/12/20 20:02:01]

Agreed, does not make sense to do this in the tracker.  A new PKS is overkill,
so implemented suggestion 2.

+      merge_session_helper_->LogIn(account_id);
+    }
+  }
+
   observer_->SigninSuccess();
 }
 
 void SigninTracker::OnRefreshTokenRevoked(const std::string& account_id) {
   NOTREACHED();
 }
+
+void SigninTracker::MergeSessionCompleted(
+    const std::string& account_id,
+    const GoogleServiceAuthError& error) {
+  observer_->MergeSessionComplete(error);
+}