[Signin] Factor out a helper class to load a continue URL once Sync is configured.

chrome/browser/ui/sync/one_click_signin_helper.cc

Index: chrome/browser/ui/sync/one_click_signin_helper.cc
diff --git a/chrome/browser/ui/sync/one_click_signin_helper.cc b/chrome/browser/ui/sync/one_click_signin_helper.cc
index 899a0313a66f1d2c612ead4bfa82f4e82c7458ba..0bd961a23fb81e32e398bd4fe7ba2c47d509cfa1 100644
--- a/chrome/browser/ui/sync/one_click_signin_helper.cc
+++ b/chrome/browser/ui/sync/one_click_signin_helper.cc
@@ -50,6 +50,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/sync/one_click_signin_histogram.h"
+#include "chrome/browser/ui/sync/one_click_signin_sync_observer.h"
 #include "chrome/browser/ui/sync/one_click_signin_sync_starter.h"
 #include "chrome/browser/ui/sync/signin_histogram.h"
 #include "chrome/browser/ui/tab_modal_confirm_dialog.h"
@@ -484,12 +485,6 @@ void CurrentHistoryCleaner::WebContentsDestroyed(
   delete this;  // Failure.
 }
 
-void CloseTab(content::WebContents* tab) {
-  content::WebContentsDelegate* tab_delegate = tab->GetDelegate();
-  if (tab_delegate)
-    tab_delegate->CloseContents(tab);
-}
-
 }  // namespace
 
 
@@ -700,11 +695,7 @@ OneClickSigninHelper::OneClickSigninHelper(content::WebContents* web_contents,
   }
 }
 
-OneClickSigninHelper::~OneClickSigninHelper() {
-  // WebContentsDestroyed() should always be called before the object is
-  // deleted.
-  DCHECK(!web_contents());

[Roger Tawa OOO till Jul 10th, 2014/03/31 18:24:11]

Why is this DCHECK no longer valid?

[Ilya Sherman, 2014/03/31 19:34:06]

It's valid, but no longer relevant, since this class no longer listens for
WebContentsDestroyed().  I can't move it over into the new class because there,
the destructor can be called *from* WebContentsDestroyed().

-}
+OneClickSigninHelper::~OneClickSigninHelper() {}
 
 // static
 void OneClickSigninHelper::LogHistogramValue(
@@ -1248,6 +1239,7 @@ void OneClickSigninHelper::CleanTransientState() {
   continue_url_ = GURL();
   untrusted_navigations_since_signin_visit_ = 0;
   untrusted_confirmation_required_ = false;
+  original_continue_url_ = GURL();

[Roger Tawa OOO till Jul 10th, 2014/03/31 18:24:11]

Because this member is used in
OneClickSigninHelper::SyncSetupCompletedCallback(), it should not be cleared
here.

[Ilya Sherman, 2014/04/01 02:00:35]

Good catch.  Is it ok to never reset the continue URL, or will something break
if I do that?

[Roger Tawa OOO till Jul 10th, 2014/04/01 15:21:08]

Its OK to not reset here, it would be reinitialized should the user reattempt a
sign-in.  This is the current behaviour in any case.

   error_message_.clear();
 
   // Post to IO thread to clear pending email.
@@ -1564,10 +1556,8 @@ void OneClickSigninHelper::DidStopLoading(
       if (original_source == signin::SOURCE_SETTINGS ||
           (original_source == signin::SOURCE_WEBSTORE_INSTALL &&
            source_ == signin::SOURCE_SETTINGS)) {
-        ProfileSyncService* sync_service =
-            ProfileSyncServiceFactory::GetForProfile(profile);
-        if (sync_service)
-          sync_service->AddObserver(this);
+        // The observer deletes itself once it's done.
+        new OneClickSigninSyncObserver(contents, original_continue_url_);
       }
       break;
     }
@@ -1583,60 +1573,6 @@ void OneClickSigninHelper::DidStopLoading(
   CleanTransientState();
 }
 
-// It is guaranteed that this method is called before the object is deleted.
-void OneClickSigninHelper::WebContentsDestroyed(
-    content::WebContents* contents) {
-  Profile* profile =
-      Profile::FromBrowserContext(contents->GetBrowserContext());
-  ProfileSyncService* sync_service =
-      ProfileSyncServiceFactory::GetForProfile(profile);
-  if (sync_service)
-    sync_service->RemoveObserver(this);
-}
-
-void OneClickSigninHelper::OnStateChanged() {
-  // We only add observer for ProfileSyncService when original_continue_url_ is
-  // not empty.
-  DCHECK(!original_continue_url_.is_empty());
-
-  content::WebContents* contents = web_contents();
-  Profile* profile =
-      Profile::FromBrowserContext(contents->GetBrowserContext());
-  ProfileSyncService* sync_service =
-      ProfileSyncServiceFactory::GetForProfile(profile);
-
-  // At this point, the sign in process is complete, and control has been handed
-  // back to the sync engine. Close the gaia sign in tab if
-  // |original_continue_url_| contains the |auto_close| parameter. Otherwise,
-  // wait for sync setup to complete and then navigate to
-  // |original_continue_url_|.
-  if (signin::IsAutoCloseEnabledInURL(original_continue_url_)) {
-    // Close the gaia sign in tab via a task to make sure we aren't in the
-    // middle of any webui handler code.
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&CloseTab, base::Unretained(contents)));
-  } else {
-    // Sync setup not completed yet.
-    if (sync_service->FirstSetupInProgress())
-      return;
-
-    if (sync_service->sync_initialized() &&
-        signin::GetSourceForPromoURL(original_continue_url_)
-            != signin::SOURCE_SETTINGS) {
-      contents->GetController().LoadURL(original_continue_url_,
-                                        content::Referrer(),
-                                        content::PAGE_TRANSITION_AUTO_TOPLEVEL,
-                                        std::string());
-    }
-  }
-
-  // Clears |original_continue_url_| here instead of in CleanTransientState,
-  // because it is used in OnStateChanged which occurs later.
-  original_continue_url_ = GURL();
-  sync_service->RemoveObserver(this);
-}
-
 OneClickSigninSyncStarter::Callback
     OneClickSigninHelper::CreateSyncStarterCallback() {
   // The callback will only be invoked if this object is still alive when sync