Reland "[Profile creation] Show sync confirmation and finish the flow"

This is a reland of b8368dd1

Original change's description:
> [Profile creation] Show sync confirmation and finish the flow
>
> This CL proceeds with the happy path of the signed-in profile creation
> flow:
>  1) After a successful sign-in, the current WebUI for sync confirmation
>     gets displayed;
>  2) After choosing whether to sync, the flow is finished by opening a
>     browser window for the new profile.
>
> Error handling and restyling the confirmation page is left for follow-up
> CLs.
>
> Bug: 1126913
> Change-Id: I17b43fe4ec0c0e0f2f9725b1df8e8e59d2c35f3d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2421592
> Commit-Queue: Jan Krcal <jkrcal@chromium.org>
> Reviewed-by: David Roger <droger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#811140}

Bug: 1126913
Change-Id: I4b3b6d974214c4be16f207111007887b433b04dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437373
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Auto-Submit: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814160}

chrome/browser/profiles/profiles_state.cc

@@ -284,6 +284,20 @@ bool ArePublicSessionRestrictionsEnabled() {
 #endif
   return false;
 }
+
+#if !defined(OS_CHROMEOS)
+base::string16 GetDefaultNameForNewSignedInProfile(
+    const AccountInfo& account_info) {
+  DCHECK(account_info.IsValid());
+  bool is_consumer = account_info.hosted_domain.empty() ||
+                     account_info.hosted_domain == kNoHostedDomainFound;
+  if (is_consumer)
+    return base::UTF8ToUTF16(account_info.given_name);
+  return l10n_util::GetStringUTF16(
+      IDS_SIGNIN_DICE_WEB_INTERCEPT_ENTERPRISE_PROFILE_NAME);
+}
+#endif  // !defined(OS_CHROMEOS)
+
 #endif  // !defined(OS_ANDROID)
 
 }  // namespace profiles

chrome/browser/profiles/profiles_state.h

@@ -16,6 +16,7 @@
 #include "chrome/browser/profiles/avatar_menu.h"
 #endif
 
+struct AccountInfo;
 class Browser;
 class PrefRegistrySimple;
 class Profile;
@@ -108,6 +109,14 @@ bool IsPublicSession();
 
 // Returns whether public session restrictions are enabled.
 bool ArePublicSessionRestrictionsEnabled();
+
+#if !defined(OS_CHROMEOS)
+// Returns the default name for a new signed-in profile, based on
+// `account_info`.
+base::string16 GetDefaultNameForNewSignedInProfile(
+    const AccountInfo& account_info);
+#endif  // !defined(OS_CHROMEOS)
+
 #endif  // !defined(OS_ANDROID)
 
 }  // namespace profiles

chrome/browser/signin/dice_web_signin_interceptor.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_metrics.h"
+#include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/signin/dice_intercepted_session_startup_helper.h"
 #include "chrome/browser/signin/dice_signed_in_profile_creator.h"
 #include "chrome/browser/signin/dice_web_signin_interceptor_factory.h"
@@ -30,7 +31,6 @@
 #include "chrome/browser/ui/signin/profile_colors_util.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/themes/autogenerated_theme_util.h"
-#include "chrome/grit/generated_resources.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -333,12 +333,7 @@ void DiceWebSigninInterceptor::OnProfileCreationChoice(SkColor profile_color,
           ->FindExtendedAccountInfoForAccountWithRefreshTokenByAccountId(
               account_id_);
   if (account_info) {
-    bool is_managed = !account_info->hosted_domain.empty() &&
-                      account_info->hosted_domain != kNoHostedDomainFound;
-    profile_name =
-        is_managed ? l10n_util::GetStringUTF16(
-                         IDS_SIGNIN_DICE_WEB_INTERCEPT_ENTERPRISE_PROFILE_NAME)
-                   : base::UTF8ToUTF16(account_info->given_name);
+    profile_name = profiles::GetDefaultNameForNewSignedInProfile(*account_info);
   }
 
   DCHECK(!dice_signed_in_profile_creator_);

chrome/browser/ui/BUILD.gn

@@ -2798,6 +2798,8 @@ static_library("ui") {
       "views/profiles/badged_profile_photo.h",
       "views/profiles/profile_picker_view.cc",
       "views/profiles/profile_picker_view.h",
+      "views/profiles/profile_picker_view_sync_delegate.cc",
+      "views/profiles/profile_picker_view_sync_delegate.h",
       "views/profiles/user_manager_view.cc",
       "views/profiles/user_manager_view.h",
       "webui/app_launcher_page_ui.cc",

chrome/browser/ui/profile_picker.h

@@ -7,6 +7,11 @@
 
 #include "third_party/skia/include/core/SkColor.h"
 
+namespace views {
+class View;
+class WebView;
+}  // namespace views
+
 class ProfilePicker {
  public:
   // An entry point that triggers the profile picker window to open.
@@ -26,18 +31,28 @@ class ProfilePicker {
 
   // Starts the sign-in flow. The layout of the window gets updated for the
   // sign-in flow. At the same time, the new profile is created (with
-  // |profile_color|) and the sign-in page is rendered using the new profile.
-  // If the creation of the new profile fails, |switch_failure_callback| gets
+  // `profile_color`) and the sign-in page is rendered using the new profile.
+  // If the creation of the new profile fails, `switch_failure_callback` gets
   // called.
   static void SwitchToSignIn(SkColor profile_color,
                              base::OnceClosure switch_failure_callback);
 
+  // Finishes the sign-in flow by moving to the sync confirmation screen. It
+  // uses the same new profile created by `SwitchToSignIn()`.
+  static void SwitchToSyncConfirmation();
+
   // Hides the profile picker.
   static void Hide();
 
   // Returns whether the profile picker is currently open.
   static bool IsOpen();
 
+  // Returns the global profile picker view for testing.
+  static views::View* GetViewForTesting();
+
+  // Returns the web view (embedded in the picker) for testing.
+  static views::WebView* GetWebViewForTesting();
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ProfilePicker);
 };

chrome/browser/ui/views/profiles/profile_picker_view.h

@@ -9,10 +9,14 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/profile_picker.h"
 #include "components/keep_alive_registry/scoped_keep_alive.h"
+#include "components/signin/public/identity_manager/identity_manager.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "ui/views/controls/webview/webview.h"
 #include "ui/views/window/dialog_delegate.h"
 
+struct AccountInfo;
+class Browser;
+
 namespace content {
 struct ContextMenuParams;
 class RenderFrameHost;
@@ -20,7 +24,11 @@ class RenderFrameHost;
 
 // Dialog widget that contains the Desktop Profile picker webui.
 class ProfilePickerView : public views::DialogDelegateView,
-                          public content::WebContentsDelegate {
+                          public content::WebContentsDelegate,
+                          public signin::IdentityManager::Observer {
+ public:
+  using BrowserOpenedCallback = base::OnceCallback<void(Browser*)>;
+
  private:
   friend class ProfilePicker;
 
@@ -54,6 +62,8 @@ class ProfilePickerView : public views::DialogDelegateView,
   void OnProfileForSigninCreated(SkColor profile_color,
                                  Profile* new_profile,
                                  Profile::CreateStatus status);
+  // Switches the layout to the sync confirmation screen.
+  void SwitchToSyncConfirmation();
 
   // views::DialogDelegateView:
   gfx::Size CalculatePreferredSize() const override;
@@ -66,12 +76,37 @@ class ProfilePickerView : public views::DialogDelegateView,
   bool HandleContextMenu(content::RenderFrameHost* render_frame_host,
                          const content::ContextMenuParams& params) override;
 
+  // IdentityManager::Observer:
+  void OnRefreshTokenUpdatedForAccount(
+      const CoreAccountInfo& account_info) override;
+  void OnExtendedAccountInfoUpdated(const AccountInfo& account_info) override;
+
+  // Finishes the creation flow by marking `profile` as fully created, opening a
+  // browser window for `profile` and calling `callback`.
+  void FinishSignedInCreationFlow(Profile* profile,
+                                  BrowserOpenedCallback callback);
+  void FinishSignedInCreationFlowImpl(Profile* profile,
+                                      BrowserOpenedCallback callback);
+
+  // Internal callback to finish the last steps of the signed-in creation flow.
+  void OnBrowserOpened(BrowserOpenedCallback finish_flow_callback,
+                       Profile* profile,
+                       Profile::CreateStatus profile_create_status);
+
   ScopedKeepAlive keep_alive_;
-  views::WebView* web_view_ = nullptr;
   InitState initialized_ = InitState::kNotInitialized;
 
+  // The current WebView object, owned by the view hierarchy.
+  views::WebView* web_view_ = nullptr;
+  Profile* profile_being_created_ = nullptr;
+
+  AccountInfo account_info_;
+  base::OnceClosure on_account_info_available_;
+
   // Not null iff switching to sign-in is in progress.
   base::OnceClosure switch_failure_callback_;
+  ScopedObserver<signin::IdentityManager, signin::IdentityManager::Observer>
+      identity_manager_observer_{this};
 
   // Creation time of the picker, to measure performance on startup. Only set
   // when the picker is shown on startup.

chrome/browser/ui/views/profiles/profile_picker_view_sync_delegate.cc

+// Copyright 2020 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/views/profiles/profile_picker_view_sync_delegate.h"
+
+#include "base/logging.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
+#include "chrome/browser/ui/chrome_pages.h"
+#include "chrome/browser/ui/profile_picker.h"
+#include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
+#include "chrome/common/webui_url_constants.h"
+
+namespace {
+
+void OpenSettingsInBrowser(Browser* browser) {
+  chrome::ShowSettingsSubPage(browser, chrome::kSyncSetupSubPage);
+}
+
+}  // namespace
+
+ProfilePickerViewSyncDelegate::ProfilePickerViewSyncDelegate(
+    Profile* profile,
+    OpenBrowserCallback open_browser_callback)
+    : profile_(profile),
+      open_browser_callback_(std::move(open_browser_callback)) {}
+
+ProfilePickerViewSyncDelegate::~ProfilePickerViewSyncDelegate() = default;
+
+void ProfilePickerViewSyncDelegate::ShowLoginError(
+    const std::string& email,
+    const std::string& error_message) {
+  // TODO(crbug.com/1126913): Handle the error cases.
+  NOTIMPLEMENTED();
+}
+
+void ProfilePickerViewSyncDelegate::ShowMergeSyncDataConfirmation(
+    const std::string& previous_email,
+    const std::string& new_email,
+    DiceTurnSyncOnHelper::SigninChoiceCallback callback) {
+  // A brand new profile cannot have a conflict in sync accounts.
+  NOTREACHED();
+}
+
+void ProfilePickerViewSyncDelegate::ShowEnterpriseAccountConfirmation(
+    const std::string& email,
+    DiceTurnSyncOnHelper::SigninChoiceCallback callback) {
+  // TODO(crbug.com/1126913): Handle the error cases.
+  NOTIMPLEMENTED();
+}
+
+void ProfilePickerViewSyncDelegate::ShowSyncConfirmation(
+    base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
+        callback) {
+  DCHECK(callback);
+  sync_confirmation_callback_ = std::move(callback);
+  scoped_login_ui_service_observer_.Add(
+      LoginUIServiceFactory::GetForProfile(profile_));
+  ProfilePicker::SwitchToSyncConfirmation();
+}
+
+void ProfilePickerViewSyncDelegate::ShowSyncSettings() {
+  // Open the browser and when it's done, open settings in the browser.
+  std::move(open_browser_callback_)
+      .Run(profile_, base::BindOnce(&OpenSettingsInBrowser));
+}
+
+void ProfilePickerViewSyncDelegate::SwitchToProfile(Profile* new_profile) {
+  // TODO(crbug.com/1126913): Handle this flow.
+  NOTIMPLEMENTED();
+}
+
+void ProfilePickerViewSyncDelegate::OnSyncConfirmationUIClosed(
+    LoginUIService::SyncConfirmationUIClosedResult result) {
+  // No need to listen to further confirmations any more.
+  scoped_login_ui_service_observer_.Remove(
+      LoginUIServiceFactory::GetForProfile(profile_));
+
+  DCHECK(sync_confirmation_callback_);
+  std::move(sync_confirmation_callback_).Run(result);
+}

chrome/browser/ui/views/profiles/profile_picker_view_sync_delegate.h

+// Copyright 2020 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_PROFILES_PROFILE_PICKER_VIEW_SYNC_DELEGATE_H_
+#define CHROME_BROWSER_UI_VIEWS_PROFILES_PROFILE_PICKER_VIEW_SYNC_DELEGATE_H_
+
+#include "base/scoped_observer.h"
+#include "chrome/browser/ui/views/profiles/profile_picker_view.h"
+#include "chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.h"
+#include "chrome/browser/ui/webui/signin/login_ui_service.h"
+
+class Profile;
+
+// Handles the sync consent screen for creating a signed-in profile from the
+// profile picker.
+class ProfilePickerViewSyncDelegate : public DiceTurnSyncOnHelper::Delegate,
+                                      public LoginUIService::Observer {
+ public:
+  using OpenBrowserCallback =
+      base::OnceCallback<void(Profile*,
+                              ProfilePickerView::BrowserOpenedCallback)>;
+
+  ProfilePickerViewSyncDelegate(Profile* profile,
+                                OpenBrowserCallback open_browser_callback);
+  ~ProfilePickerViewSyncDelegate() override;
+
+ private:
+  // DiceTurnSyncOnHelper::Delegate:
+  void ShowLoginError(const std::string& email,
+                      const std::string& error_message) override;
+  void ShowMergeSyncDataConfirmation(
+      const std::string& previous_email,
+      const std::string& new_email,
+      DiceTurnSyncOnHelper::SigninChoiceCallback callback) override;
+  void ShowEnterpriseAccountConfirmation(
+      const std::string& email,
+      DiceTurnSyncOnHelper::SigninChoiceCallback callback) override;
+  void ShowSyncConfirmation(
+      base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
+          callback) override;
+  void ShowSyncSettings() override;
+  void SwitchToProfile(Profile* new_profile) override;
+
+  // LoginUIService::Observer:
+  void OnSyncConfirmationUIClosed(
+      LoginUIService::SyncConfirmationUIClosedResult result) override;
+
+  Profile* profile_;
+  OpenBrowserCallback open_browser_callback_;
+  base::OnceCallback<void(LoginUIService::SyncConfirmationUIClosedResult)>
+      sync_confirmation_callback_;
+  ScopedObserver<LoginUIService, LoginUIService::Observer>
+      scoped_login_ui_service_observer_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(ProfilePickerViewSyncDelegate);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_PROFILES_PROFILE_PICKER_VIEW_SYNC_DELEGATE_H_

chrome/browser/ui/webui/signin/sync_confirmation_handler.cc

@@ -37,14 +37,14 @@ const int kProfileImageSize = 128;
 }  // namespace
 
 SyncConfirmationHandler::SyncConfirmationHandler(
-    Browser* browser,
-    const std::unordered_map<std::string, int>& string_to_grd_id_map)
-    : profile_(browser->profile()),
-      browser_(browser),
+    Profile* profile,
+    const std::unordered_map<std::string, int>& string_to_grd_id_map,
+    Browser* browser)
+    : profile_(profile),
       string_to_grd_id_map_(string_to_grd_id_map),
+      browser_(browser),
       identity_manager_(IdentityManagerFactory::GetForProfile(profile_)) {
   DCHECK(profile_);
-  DCHECK(browser_);
   BrowserList::AddObserver(this);
 }
 
@@ -219,9 +219,6 @@ void SyncConfirmationHandler::HandleInitializedWithSize(
     const base::ListValue* args) {
   AllowJavascript();
 
-  if (!browser_)
-    return;
-
   base::Optional<AccountInfo> primary_account_info =
       identity_manager_->FindExtendedAccountInfoForAccountWithRefreshToken(
           identity_manager_->GetPrimaryAccountInfo(ConsentLevel::kNotRequired));
@@ -238,5 +235,6 @@ void SyncConfirmationHandler::HandleInitializedWithSize(
     SetUserImageURL(primary_account_info->picture_url);
   }
 
-  signin::SetInitializedModalHeight(browser_, web_ui(), args);
+  if (browser_)
+    signin::SetInitializedModalHeight(browser_, web_ui(), args);
 }

chrome/browser/ui/webui/signin/sync_confirmation_handler.h

@@ -29,12 +29,14 @@ class SyncConfirmationHandler : public content::WebUIMessageHandler,
                                 public signin::IdentityManager::Observer,
                                 public BrowserListObserver {
  public:
-  // Creates a SyncConfirmationHandler for the |browser|. All strings in the
+  // Creates a SyncConfirmationHandler for the |profile|. All strings in the
   // corresponding Web UI should be represented in |string_to_grd_id_map| and
-  // mapped to their GRD IDs.
+  // mapped to their GRD IDs. If |browser| is provided, its signin view
+  // controller will be notified of the rendered size of the web page.
   SyncConfirmationHandler(
-      Browser* browser,
-      const std::unordered_map<std::string, int>& string_to_grd_id_map);
+      Profile* profile,
+      const std::unordered_map<std::string, int>& string_to_grd_id_map,
+      Browser* browser = nullptr);
   ~SyncConfirmationHandler() override;
 
   // content::WebUIMessageHandler:
@@ -95,9 +97,6 @@ class SyncConfirmationHandler : public content::WebUIMessageHandler,
  private:
   Profile* profile_;
 
-  // Weak reference to the browser that showed the sync confirmation dialog.
-  Browser* browser_;
-
   // Records whether the user clicked on Undo, Ok, or Settings.
   bool did_user_explicitly_interact_ = false;
 
@@ -105,6 +104,10 @@ class SyncConfirmationHandler : public content::WebUIMessageHandler,
   // and their respective GRD IDs.
   std::unordered_map<std::string, int> string_to_grd_id_map_;
 
+  // Weak reference to the browser that showed the sync confirmation dialog (if
+  // such a dialog exists).
+  Browser* browser_;
+
   signin::IdentityManager* identity_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncConfirmationHandler);

chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc

@@ -45,7 +45,9 @@ class TestingSyncConfirmationHandler : public SyncConfirmationHandler {
       Browser* browser,
       content::WebUI* web_ui,
       std::unordered_map<std::string, int> string_to_grd_id_map)
-      : SyncConfirmationHandler(browser, string_to_grd_id_map) {
+      : SyncConfirmationHandler(browser->profile(),
+                                string_to_grd_id_map,
+                                browser) {
     set_web_ui(web_ui);
   }
 

chrome/browser/ui/webui/signin/sync_confirmation_ui.cc

@@ -12,6 +12,7 @@
 #include "chrome/browser/signin/account_consistency_mode_manager.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/webui/signin/sync_confirmation_handler.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/browser_resources.h"
@@ -120,7 +121,12 @@ SyncConfirmationUI::~SyncConfirmationUI() {}
 
 void SyncConfirmationUI::InitializeMessageHandlerWithBrowser(Browser* browser) {
   web_ui()->AddMessageHandler(std::make_unique<SyncConfirmationHandler>(
-      browser, js_localized_string_to_ids_map_));
+      browser->profile(), js_localized_string_to_ids_map_, browser));
+}
+
+void SyncConfirmationUI::InitializeMessageHandlerWithProfile(Profile* profile) {
+  web_ui()->AddMessageHandler(std::make_unique<SyncConfirmationHandler>(
+      profile, js_localized_string_to_ids_map_));
 }
 
 void SyncConfirmationUI::AddStringResource(content::WebUIDataSource* source,

chrome/browser/ui/webui/signin/sync_confirmation_ui.h

@@ -12,6 +12,9 @@
 #include "base/macros.h"
 #include "chrome/browser/ui/webui/signin/signin_web_dialog_ui.h"
 
+class Browser;
+class Profile;
+
 namespace content {
 class WebUIDataSource;
 }
@@ -32,6 +35,10 @@ class SyncConfirmationUI : public SigninWebDialogUI {
   // SigninWebDialogUI:
   void InitializeMessageHandlerWithBrowser(Browser* browser) override;
 
+  // Initializes the message handler when there's no browser for `profile`
+  // available (such as in the profile creation flow).
+  void InitializeMessageHandlerWithProfile(Profile* profile);
+
  private:
   // Adds a string resource with the given GRD |ids| to the WebUI data |source|
   // named as |name|. Also stores a reverse mapping from the localized version

chrome/test/BUILD.gn

@@ -3077,6 +3077,11 @@ if (!is_android) {
           [ "../browser/site_isolation/spellcheck_per_process_browsertest.cc" ]
     }
 
+    if (is_win || is_mac || (is_desktop_linux && !chromeos_is_browser_only)) {
+      sources +=
+          [ "../browser/ui/views/profiles/profile_picker_view_browsertest.cc" ]
+    }
+
     sources += metric_integration_sources
   }
 }