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

chrome/browser/ui/sync/one_click_signin_helper.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/sync/one_click_signin_helper.h"
 
 #include <algorithm>
 #include <functional>
 #include <utility>
 #include <vector>
@@ skipping 32 matching lines @@
 #include "chrome/browser/signin/signin_names_io_thread.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.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/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"
 #include "chrome/browser/ui/tab_modal_confirm_dialog_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/net/url_util.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/profile_management_switches.h"
 #include "chrome/common/url_constants.h"
@@ skipping 414 matching lines @@
       delete this;  // Success.
     }
   }
 }
 
 void CurrentHistoryCleaner::WebContentsDestroyed(
     content::WebContents* contents) {
   delete this;  // Failure.
 }
 
-void CloseTab(content::WebContents* tab) {
-  content::WebContentsDelegate* tab_delegate = tab->GetDelegate();
-  if (tab_delegate)
-    tab_delegate->CloseContents(tab);
-}
-
 }  // namespace
 
 
 // StartSyncArgs --------------------------------------------------------------
 
 OneClickSigninHelper::StartSyncArgs::StartSyncArgs()
     : profile(NULL),
       browser(NULL),
       auto_accept(AUTO_ACCEPT_NONE),
       web_contents(NULL),
@@ skipping 190 matching lines @@
       do_not_start_sync_for_testing_(false),
       weak_pointer_factory_(this) {
   // May be NULL during testing.
   if (password_manager) {
     password_manager->AddSubmissionCallback(
         base::Bind(&OneClickSigninHelper::PasswordSubmitted,
                    weak_pointer_factory_.GetWeakPtr()));
   }
 }
 
-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(
     signin::Source source, int action) {
   switch (source) {
     case signin::SOURCE_START_PAGE:
       UMA_HISTOGRAM_ENUMERATION("Signin.StartPageActions", action,
                                 one_click_signin::HISTOGRAM_MAX);
       break;
     case signin::SOURCE_NTP_LINK:
@@ skipping 523 matching lines @@
   VLOG(1) << "OneClickSigninHelper::CleanTransientState";
   showing_signin_ = false;
   email_.clear();
   password_.clear();
   auto_accept_ = AUTO_ACCEPT_NONE;
   source_ = signin::SOURCE_UNKNOWN;
   switched_to_advanced_ = false;
   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.
   if (!do_not_clear_pending_email_) {
     Profile* profile =
         Profile::FromBrowserContext(web_contents()->GetBrowserContext());
     content::BrowserThread::PostTask(
         content::BrowserThread::IO, FROM_HERE,
         base::Bind(&ClearPendingEmailOnIOThread,
                    base::Unretained(profile->GetResourceContext())));
@@ skipping 296 matching lines @@
         RedirectToNtpOrAppsPageIfNecessary(web_contents(), source_);
       }
 
       // Observe the sync service if the Webstore tab or the settings tab
       // requested a gaia sign in, so that when sign in and sync setup are
       // successful, we can redirect to the correct URL, or auto-close the gaia
       // sign in tab.
       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;
     }
     case AUTO_ACCEPT_REJECTED_FOR_PROFILE:
       AddEmailToOneClickRejectedList(profile, email_);
       LogOneClickHistogramValue(one_click_signin::HISTOGRAM_REJECTED);
       break;
     default:
       NOTREACHED() << "Invalid auto_accept=" << auto_accept_;
       break;
   }
 
   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
   // setup is completed. This is correct because this object is only deleted
   // when the web contents that potentially shows a blank page is deleted.
   return base::Bind(&OneClickSigninHelper::SyncSetupCompletedCallback,
                     weak_pointer_factory_.GetWeakPtr());
 }
 
 void OneClickSigninHelper::SyncSetupCompletedCallback(
     OneClickSigninSyncStarter::SyncSetupResult result) {
   if (result == OneClickSigninSyncStarter::SYNC_SETUP_FAILURE &&
       web_contents()) {
     GURL current_url = web_contents()->GetVisibleURL();
 
     // If the web contents is showing a blank page and not about to be closed,
     // redirect to the NTP or apps page.
     if (signin::IsContinueUrlForWebBasedSigninFlow(current_url) &&
         !signin::IsAutoCloseEnabledInURL(original_continue_url_)) {
       RedirectToNtpOrAppsPage(
           web_contents(),
           signin::GetSourceForPromoURL(original_continue_url_));
     }
   }
 }