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

chrome/browser/ui/sync/one_click_signin_sync_observer.cc

Index: chrome/browser/ui/sync/one_click_signin_sync_observer.cc
diff --git a/chrome/browser/ui/sync/one_click_signin_sync_observer.cc b/chrome/browser/ui/sync/one_click_signin_sync_observer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..108c17752031d43a530073a38ef11e17dff9e0ab
--- /dev/null
+++ b/chrome/browser/ui/sync/one_click_signin_sync_observer.cc
@@ -0,0 +1,101 @@
+// Copyright 2014 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_sync_observer.h"
+
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/signin_promo.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_delegate.h"
+
+namespace {
+
+void CloseTab(content::WebContents* tab) {
+  content::WebContentsDelegate* tab_delegate = tab->GetDelegate();
+  if (tab_delegate)
+    tab_delegate->CloseContents(tab);
+}
+
+}  // namespace
+
+
+OneClickSigninSyncObserver::OneClickSigninSyncObserver(
+    content::WebContents* web_contents,
+    const GURL& continue_url)
+    : content::WebContentsObserver(web_contents),
+      continue_url_(continue_url) {
+  DCHECK(!continue_url_.is_empty());
+
+  ProfileSyncService* sync_service = GetSyncService(web_contents);
+  if (sync_service) {
+    sync_service->AddObserver(this);
+  } else {
+    LoadContinueUrl();
+    delete this;

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

From my reading of the c++ standard, I believe calling delete in the ctor is
legal in this particular case because the class has a trivial dtor.  However,
this seem quite brittle since the dtor could become non-trivial later.

I'd rather see a public Initialize() method that contains lines 32-38, and the
owner of OneClickSigninSyncObserver should call it after constructing the
object.

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

I agree, this seemed a little iffy to me.  I'd prefer to instead post a task to
delete the object, though, so that clients of the class don't need to be
affected by this implementation detail.  WDYT?

[Roger Tawa OOO till Jul 10th, 2014/03/31 20:10:38]

sgtm

[guohui, 2014/03/31 20:12:27]

delayed task +1

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

Done.

+  }
+}
+
+OneClickSigninSyncObserver::~OneClickSigninSyncObserver() {}
+
+void OneClickSigninSyncObserver::WebContentsDestroyed(
+    content::WebContents* web_contents) {
+  ProfileSyncService* sync_service = GetSyncService(web_contents);
+  if (sync_service)
+    sync_service->RemoveObserver(this);
+
+  delete this;
+}
+
+void OneClickSigninSyncObserver::OnStateChanged() {
+  ProfileSyncService* sync_service = GetSyncService(web_contents());
+
+  // 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 the |continue_url_|
+  // contains the |auto_close| parameter. Otherwise, wait for sync setup to
+  // complete and then navigate to the |continue_url_|.
+  if (signin::IsAutoCloseEnabledInURL(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(web_contents())));
+  } else {
+    if (sync_service->FirstSetupInProgress()) {
+      // Sync setup has not completed yet. Wait for it to complete.
+      return;
+    }
+
+    if (sync_service->sync_initialized() &&
+        signin::GetSourceForPromoURL(continue_url_)
+            != signin::SOURCE_SETTINGS) {
+      // TODO(isherman): Redirecting after Sync is set up still has some bugs:
+      // http://crbug.com/355885
+      //   Having multiple settings pages open can cause issues.
+      // http://crbug.com/357901
+      //   Selecting anything other than "Sync Everything" when configuring Sync
+      //   prevents the redirect.
+      LoadContinueUrl();
+    }
+  }
+
+  sync_service->RemoveObserver(this);
+  delete this;
+}
+
+void OneClickSigninSyncObserver::LoadContinueUrl() {
+  web_contents()->GetController().LoadURL(
+      continue_url_,
+      content::Referrer(),
+      content::PAGE_TRANSITION_AUTO_TOPLEVEL,
+      std::string());
+}
+
+ProfileSyncService* OneClickSigninSyncObserver::GetSyncService(
+    content::WebContents* web_contents) {
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  return ProfileSyncServiceFactory::GetForProfile(profile);
+}