Add Views implementation of ProfileSigninConfirmationDialog.

chrome/browser/ui/sync/one_click_signin_sync_starter.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_sync_starter.h"
 
 #include "base/prefs/pref_service.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 
 #if defined(ENABLE_CONFIGURATION_POLICY)
 #include "chrome/browser/policy/cloud/user_policy_signin_service.h"
 #include "chrome/browser/policy/cloud/user_policy_signin_service_factory.h"
 #endif
 
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/sync_prefs.h"
 #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_navigator.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/chrome_pages.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/webui/signin/login_ui_service.h"
 #include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
 #include "chrome/browser/ui/webui/signin/profile_signin_confirmation_dialog.h"
 #include "chrome/common/url_constants.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 
 OneClickSigninSyncStarter::OneClickSigninSyncStarter(
@@ skipping 50 matching lines @@
 }
 
 void OneClickSigninSyncStarter::GaiaCredentialsValid() {
 }
 
 void OneClickSigninSyncStarter::ConfirmSignin(const std::string& oauth_token) {
   DCHECK(!oauth_token.empty());
   SigninManager* signin = SigninManagerFactory::GetForProfile(profile_);
   // If this is a new signin (no authenticated username yet) try loading
   // policy for this user now, before any signed in services are initialized.
   // This callback is only invoked for the web-based signin flow - for the old

[Peter Kasting, 2013/05/15 00:11:03]

Nit: This comment is now somewhat inaccurate (in that your new flow is not
web-based), right?

[Andrew T Wilson (Slow), 2013/05/16 08:00:30]

This comment has nothing to do with the confirmation dialog - it's talking about
the signin flow (GAIA web-form based vs old webui client-login based).

   // ClientLogin flow, policy will get loaded once the TokenService finishes
   // initializing (not ideal, but it's a reasonable fallback).
   if (signin->GetAuthenticatedUsername().empty()) {
 #if defined(ENABLE_CONFIGURATION_POLICY)
     policy::UserPolicySigninService* policy_service =
         policy::UserPolicySigninServiceFactory::GetForProfile(profile_);
     policy_service->RegisterPolicyClient(
         signin->GetUsernameForAuthInProgress(),
         oauth_token,
         base::Bind(&OneClickSigninSyncStarter::OnRegisteredForPolicy,
@@ skipping 22 matching lines @@
   }
 
   DCHECK(client->is_registered());
   DVLOG(1) << "Policy registration succeeded: dm_token=" << client->dm_token();
 
   // Stash away a copy of our CloudPolicyClient (should not already have one).
   DCHECK(!policy_client_);
   policy_client_.swap(client);
 
   // Allow user to create a new profile before continuing with sign-in.
-  ProfileSigninConfirmationDialog::ShowDialog(
+  Browser* browser =
+      chrome::FindBrowserWithProfile(profile_, chrome::GetActiveDesktop());
+  if (!browser) {
+    DLOG(WARNING) << "No browser found to display the confirmation dialog";

[Peter Kasting, 2013/05/15 00:11:03]

In most Chrome code we try to avoid logging because we usually don't have a good
way of getting the log data and doing something useful with it anyway.  Do you
really need these logs?

It would be clearer to either just handle these cases and return (if they can
actually ever happen) or DCHECK and don't handle them if they can't actually
occur.

[Andrew T Wilson (Slow), 2013/05/16 08:00:30]

Do we really have any restrictions on adding DLOGs()? This is often useful when
debugging failed tests, especially for exceptional conditions.

[Peter Kasting, 2013/05/16 21:57:39]

In the particular code here, it seems like either these conditions are
impossible -- in which case we should at most DCHECK() them, and not handle
them, which will be just as informative in a test failure -- or they're
reasonable for some particular scenario(s), in which case we really shouldn't be
logging them, we should just silently handle them.

Logging here would be more appropriate if you're in the process of trying to
make these conditions hold, but you think they currently don't, and you think
there are important cases that will break if using a DCHECK, but you have a
specific plan on how to locate the log output and use it to finish off the
fixes.

In general, there aren't hard-and-fast rules so much as a desire to have no
middle ground between "this can happen" and "this cannot happen".  Something
like DLOG(WARNING) is worrisome in the same sort of way that handling DCHECK
failures is worrisome -- it becomes unclear precisely what the intent of the
code or the magnitude of any problem is, and over time we tend toward console
spew about issues no one is sure are important or aware of how to deal with. 
For short-term changes, logs are welcome if they're useful, it's just not good
to have them persist for a significant length of time.

[dconnelly, 2013/05/17 13:01:55]

Done: removed the logging and kept the handling.  This situation can occur if
the user starts signin and then closes the browser before this is reached.

+    CancelSigninAndDelete();
+    return;
+  }
+  content::WebContents* web_contents =
+      browser->tab_strip_model()->GetActiveWebContents();
+  if (!web_contents) {
+    DLOG(WARNING) << "No web contents found to display the confirmation dialog";
+    CancelSigninAndDelete();
+    return;
+  }
+  chrome::ShowProfileSigninConfirmationDialog(
+      browser,
+      web_contents,
       profile_,
       signin->GetUsernameForAuthInProgress(),
       base::Bind(&OneClickSigninSyncStarter::CancelSigninAndDelete,
                  weak_pointer_factory_.GetWeakPtr()),
       base::Bind(&OneClickSigninSyncStarter::CreateNewSignedInProfile,
                  weak_pointer_factory_.GetWeakPtr()),
       base::Bind(&OneClickSigninSyncStarter::LoadPolicyWithCachedClient,
                  weak_pointer_factory_.GetWeakPtr()));
 }
 
@@ skipping 229 matching lines @@
 
 void OneClickSigninSyncStarter::ShowSyncSettingsPageOnSameTab() {
   std::string url = std::string(chrome::kChromeUISettingsURL) +
       chrome::kSyncSetupSubPage;
   chrome::NavigateParams params(
       browser_, GURL(url), content::PAGE_TRANSITION_AUTO_TOPLEVEL);
   params.disposition = CURRENT_TAB;
   params.window_action = chrome::NavigateParams::SHOW_WINDOW;
   chrome::Navigate(&params);
 }