Skip to content

Commit

Permalink
[ReArchPPV] Move utils out of ProfileCreationSignedInFlowController
Browse files Browse the repository at this point in the history
This is done in preparation of extracting the SAML-related steps out
of the regular post sign-in step.

The extracted utilities cover obtaining the profile name, which can
be also used during the FRE, and transitioning temporary profiles into
fully-fledged oncs once the creation is confirmed.

Bug: 1358843
Change-Id: I13e3f04deaf83bc5579649f2e03776a182d26605
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3928737
Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1054105}
  • Loading branch information
Nicolas Dossou-Gbete authored and Chromium LUCI CQ committed Oct 3, 2022
1 parent 4466776 commit c00b988
Show file tree
Hide file tree
Showing 6 changed files with 266 additions and 148 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/ui/BUILD.gn
Expand Up @@ -3557,6 +3557,8 @@ static_library("ui") {
"views/profiles/profile_management_flow_controller.h",
"views/profiles/profile_management_step_controller.cc",
"views/profiles/profile_management_step_controller.h",
"views/profiles/profile_management_utils.cc",
"views/profiles/profile_management_utils.h",
"views/profiles/profile_menu_view.cc",
"views/profiles/profile_menu_view.h",
"views/profiles/profile_picker_flow_controller.cc",
Expand Down
Expand Up @@ -4,13 +4,9 @@

#include "chrome/browser/ui/views/profiles/profile_creation_signed_in_flow_controller.h"

#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "base/trace_event/trace_event.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_attributes_entry.h"
#include "chrome/browser/profiles/profile_attributes_storage.h"
#include "chrome/browser/profiles/profile_window.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/browser/signin/identity_manager_factory.h"
Expand All @@ -22,17 +18,9 @@
#include "chrome/browser/ui/views/profiles/avatar_toolbar_button.h"
#include "chrome/browser/ui/views/profiles/profile_customization_bubble_sync_controller.h"
#include "chrome/browser/ui/views/profiles/profile_customization_bubble_view.h"
#include "chrome/common/pref_names.h"
#include "components/signin/public/identity_manager/account_info.h"

namespace {

constexpr base::TimeDelta kDefaultExtendedAccountInfoTimeout =
base::Seconds(10);

absl::optional<base::TimeDelta> g_extended_account_info_timeout_for_testing =
absl::nullopt;

// Shows the customization bubble if possible. The bubble won't be shown if the
// color is enforced by policy or downloaded through Sync or the default theme
// should be used. An IPH is shown after the bubble, or right away if the bubble
Expand Down Expand Up @@ -127,19 +115,8 @@ void ProfileCreationSignedInFlowController::Init() {
// Listen for extended account info getting fetched.
signin::IdentityManager* identity_manager =
IdentityManagerFactory::GetForProfile(profile());
identity_manager_observation_.Observe(identity_manager);

// Set up a timeout for extended account info (which cancels any existing
// timeout closure).
const CoreAccountInfo& account_info =
identity_manager->GetPrimaryAccountInfo(signin::ConsentLevel::kSignin);
extended_account_info_timeout_closure_.Reset(base::BindOnce(
&ProfileCreationSignedInFlowController::OnExtendedAccountInfoTimeout,
weak_ptr_factory_.GetWeakPtr(), account_info));
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, extended_account_info_timeout_closure_.callback(),
g_extended_account_info_timeout_for_testing.value_or(
kDefaultExtendedAccountInfoTimeout));
profile_name_resolver_ =
std::make_unique<ProfileNameResolver>(identity_manager);
}

void ProfileCreationSignedInFlowController::Cancel() {
Expand All @@ -163,42 +140,16 @@ void ProfileCreationSignedInFlowController::FinishAndOpenBrowser(
return;
is_finished_ = true;

if (name_for_signed_in_profile_.empty()) {
on_profile_name_available_ = base::BindOnce(
&ProfileCreationSignedInFlowController::FinishAndOpenBrowserImpl,
weak_ptr_factory_.GetWeakPtr(), std::move(callback));
return;
if (profile_name_resolver_->resolved_profile_name().empty()) {
// Delay finishing the flow until we have obtained a profile name.
profile_name_resolver_->set_on_profile_name_resolved_callback(
base::BindOnce(
&ProfileCreationSignedInFlowController::FinishAndOpenBrowserImpl,
// Unretained ok: `this` outlives `profile_name_resolver_`.
base::Unretained(this), std::move(callback)));
} else {
FinishAndOpenBrowserImpl(std::move(callback));
}

FinishAndOpenBrowserImpl(std::move(callback));
}

void ProfileCreationSignedInFlowController::OnExtendedAccountInfoUpdated(
const AccountInfo& account_info) {
if (!account_info.IsValid())
return;
name_for_signed_in_profile_ =
profiles::GetDefaultNameForNewSignedInProfile(account_info);
OnProfileNameAvailable();
// Extended info arrived on time, no need for the timeout callback any more.
extended_account_info_timeout_closure_.Cancel();
}

void ProfileCreationSignedInFlowController::OnExtendedAccountInfoTimeout(
const CoreAccountInfo& account) {
name_for_signed_in_profile_ =
profiles::GetDefaultNameForNewSignedInProfileWithIncompleteInfo(account);
OnProfileNameAvailable();
}

void ProfileCreationSignedInFlowController::OnProfileNameAvailable() {
// Stop listening to further changes.
DCHECK(identity_manager_observation_.IsObservingSource(
IdentityManagerFactory::GetForProfile(profile())));
identity_manager_observation_.Reset();

if (on_profile_name_available_)
std::move(on_profile_name_available_).Run();
}

void ProfileCreationSignedInFlowController::FinishAndOpenBrowserImpl(
Expand All @@ -207,26 +158,13 @@ void ProfileCreationSignedInFlowController::FinishAndOpenBrowserImpl(
"browser",
"ProfileCreationSignedInFlowController::FinishAndOpenBrowserImpl",
"profile_path", profile()->GetPath().AsUTF8Unsafe());
DCHECK(!name_for_signed_in_profile_.empty());

ProfileAttributesEntry* entry =
g_browser_process->profile_manager()
->GetProfileAttributesStorage()
.GetProfileAttributesWithPath(profile()->GetPath());
if (!entry) {
NOTREACHED();
return;
}
std::u16string name_for_signed_in_profile =
profile_name_resolver_->resolved_profile_name();
profile_name_resolver_.reset();
DCHECK(!name_for_signed_in_profile.empty());

FinalizeNewProfileSetup(profile(), name_for_signed_in_profile);

entry->SetIsOmitted(false);
if (!profile()->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles)) {
// Unmark this profile ephemeral so that it isn't deleted upon next startup.
// Profiles should never be made non-ephemeral if ephemeral mode is forced
// by policy.
entry->SetIsEphemeral(false);
}
entry->SetLocalProfileName(name_for_signed_in_profile_,
/*is_default_name=*/false);
ProfileMetrics::LogProfileAddNewUser(
ProfileMetrics::ADD_NEW_PROFILE_PICKER_SIGNED_IN);

Expand All @@ -253,9 +191,11 @@ void ProfileCreationSignedInFlowController::FinishAndOpenBrowserImpl(
}
}

// Skip the FRE for this profile as it's replaced by profile creation flow.
profile()->GetPrefs()->SetBoolean(prefs::kHasSeenWelcomePage, true);
ExitPickerAndRunInNewBrowser(std::move(callback));
}

void ProfileCreationSignedInFlowController::ExitPickerAndRunInNewBrowser(
ProfilePicker::BrowserOpenedCallback callback) {
profiles::OpenBrowserWindowForProfile(
base::BindOnce(&ProfileCreationSignedInFlowController::OnBrowserOpened,
weak_ptr_factory_.GetWeakPtr(), std::move(callback)),
Expand Down Expand Up @@ -284,11 +224,15 @@ void ProfileCreationSignedInFlowController::OnSignInContentsFreedUp() {
DCHECK(!is_finished_);
is_finished_ = true;

DCHECK(name_for_signed_in_profile_.empty());
name_for_signed_in_profile_ =
profiles::GetDefaultNameForNewEnterpriseProfile();
DCHECK(!profile_name_resolver_);
contents()->SetDelegate(nullptr);
FinishAndOpenBrowserImpl(

FinalizeNewProfileSetup(profile(),
profiles::GetDefaultNameForNewEnterpriseProfile());
ProfileMetrics::LogProfileAddNewUser(
ProfileMetrics::ADD_NEW_PROFILE_PICKER_SIGNED_IN);

ExitPickerAndRunInNewBrowser(
base::BindOnce(&ContinueSAMLSignin, ReleaseContents()));
}

Expand All @@ -311,24 +255,3 @@ void ProfileCreationSignedInFlowController::OnBrowserOpened(
CHECK(browser);
std::move(finish_flow_callback).Run(browser);
}

namespace testing {

ScopedCreatedProfileInfoFetchTimeoutOverride::
ScopedCreatedProfileInfoFetchTimeoutOverride(base::TimeDelta timeout) {
if (g_extended_account_info_timeout_for_testing.has_value()) {
overriden_timeout_ = g_extended_account_info_timeout_for_testing.value();
}
g_extended_account_info_timeout_for_testing = timeout;
}

ScopedCreatedProfileInfoFetchTimeoutOverride::
~ScopedCreatedProfileInfoFetchTimeoutOverride() {
if (overriden_timeout_.has_value()) {
g_extended_account_info_timeout_for_testing = overriden_timeout_.value();
} else {
g_extended_account_info_timeout_for_testing.reset();
}
}

} // namespace testing
Expand Up @@ -5,21 +5,16 @@
#ifndef CHROME_BROWSER_UI_VIEWS_PROFILES_PROFILE_CREATION_SIGNED_IN_FLOW_CONTROLLER_H_
#define CHROME_BROWSER_UI_VIEWS_PROFILES_PROFILE_CREATION_SIGNED_IN_FLOW_CONTROLLER_H_

#include "base/cancelable_callback.h"
#include "base/memory/weak_ptr.h"
#include "base/time/time.h"
#include "chrome/browser/ui/views/profiles/profile_management_utils.h"
#include "chrome/browser/ui/views/profiles/profile_picker_signed_in_flow_controller.h"
#include "components/signin/public/identity_manager/identity_manager.h"

struct AccountInfo;
struct CoreAccountInfo;
class Profile;

// Class responsible for the part of the profile creation flow where the user is
// signed in (most importantly offering sync).
class ProfileCreationSignedInFlowController
: public ProfilePickerSignedInFlowController,
public signin::IdentityManager::Observer {
: public ProfilePickerSignedInFlowController {
public:
ProfileCreationSignedInFlowController(
ProfilePickerWebContentsHost* host,
Expand All @@ -40,20 +35,19 @@ class ProfileCreationSignedInFlowController
ProfilePicker::BrowserOpenedCallback callback) override;

private:
// IdentityManager::Observer:
void OnExtendedAccountInfoUpdated(const AccountInfo& account_info) override;

// Helper functions to deal with the lack of extended account info.
void OnExtendedAccountInfoTimeout(const CoreAccountInfo& account);
void OnProfileNameAvailable();

// Finishes the non-SAML flow, registering customisation-related callbacks if
// no `callback` is povided.
void FinishAndOpenBrowserImpl(ProfilePicker::BrowserOpenedCallback callback);

// Finishes the flow by finalizing the profile and continuing the SAML
// sign-in in a browser window.
// Finishes the SAML flow by continuing the sign-in in a browser window.
void FinishAndOpenBrowserForSAML();
void OnSignInContentsFreedUp();

// Shared helper. Opens a new browser window, closes the picker and runs
// `callback` in the opened window.
void ExitPickerAndRunInNewBrowser(
ProfilePicker::BrowserOpenedCallback callback);

// Internal callback to finish the last steps of the signed-in creation
// flow.
void OnBrowserOpened(
Expand All @@ -68,33 +62,10 @@ class ProfileCreationSignedInFlowController
// `profile` browser window at the end of the sign-in flow).
bool is_finished_ = false;

std::u16string name_for_signed_in_profile_;
base::OnceClosure on_profile_name_available_;

base::CancelableOnceClosure extended_account_info_timeout_closure_;

base::ScopedObservation<signin::IdentityManager,
signin::IdentityManager::Observer>
identity_manager_observation_{this};
std::unique_ptr<ProfileNameResolver> profile_name_resolver_;

base::WeakPtrFactory<ProfileCreationSignedInFlowController> weak_ptr_factory_{
this};
};

namespace testing {

// Overrides the timeout we give to the extended account info fetch after a
// profile creation, before we use fallback values instead.
class ScopedCreatedProfileInfoFetchTimeoutOverride {
public:
explicit ScopedCreatedProfileInfoFetchTimeoutOverride(
base::TimeDelta timeout);
~ScopedCreatedProfileInfoFetchTimeoutOverride();

private:
absl::optional<base::TimeDelta> overriden_timeout_;
};

} // namespace testing

#endif // CHROME_BROWSER_UI_VIEWS_PROFILES_PROFILE_CREATION_SIGNED_IN_FLOW_CONTROLLER_H_
101 changes: 101 additions & 0 deletions chrome/browser/ui/views/profiles/profile_management_utils.cc
@@ -0,0 +1,101 @@
// Copyright 2022 The Chromium Authors
// 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_management_utils.h"

#include "base/auto_reset.h"
#include "base/functional/bind.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile_attributes_entry.h"
#include "chrome/browser/profiles/profile_attributes_storage.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/common/pref_names.h"
#include "components/prefs/pref_service.h"

namespace {

constexpr base::TimeDelta kDefaultExtendedAccountInfoTimeout =
base::Seconds(10);

absl::optional<base::TimeDelta> g_extended_account_info_timeout_for_testing =
absl::nullopt;

} // namespace

// -- Helper functions ---------------------------------------------------------

void FinalizeNewProfileSetup(Profile* profile,
const std::u16string& profile_name) {
ProfileAttributesEntry* entry =
g_browser_process->profile_manager()
->GetProfileAttributesStorage()
.GetProfileAttributesWithPath(profile->GetPath());
CHECK(entry);

entry->SetIsOmitted(false);
if (!profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles)) {
// Unmark this profile ephemeral so that it isn't deleted upon next startup.
// Profiles should never be made non-ephemeral if ephemeral mode is forced
// by policy.
entry->SetIsEphemeral(false);
}
entry->SetLocalProfileName(profile_name,
/*is_default_name=*/false);

// Skip the welcome page for this profile as we already showed a profile setup
// experience.
profile->GetPrefs()->SetBoolean(prefs::kHasSeenWelcomePage, true);
}

// -- ProfileNameResolver ------------------------------------------------------

// static
ProfileNameResolver::ScopedInfoFetchTimeoutOverride
ProfileNameResolver::CreateScopedInfoFetchTimeoutOverrideForTesting(
base::TimeDelta timeout) {
return base::AutoReset<absl::optional<base::TimeDelta>>(
&g_extended_account_info_timeout_for_testing, timeout);
}

ProfileNameResolver::ProfileNameResolver(
signin::IdentityManager* identity_manager) {
// Listen for extended account info getting fetched.
identity_manager_observation_.Observe(identity_manager);

// Set up a timeout for extended account info (which cancels any existing
// timeout closure).
const CoreAccountInfo& account_info =
identity_manager->GetPrimaryAccountInfo(signin::ConsentLevel::kSignin);
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(
&ProfileNameResolver::OnProfileNameResolved,
weak_ptr_factory_.GetWeakPtr(),
profiles::GetDefaultNameForNewSignedInProfileWithIncompleteInfo(
account_info)),
g_extended_account_info_timeout_for_testing.value_or(
kDefaultExtendedAccountInfoTimeout));
}

ProfileNameResolver::~ProfileNameResolver() = default;

void ProfileNameResolver::OnExtendedAccountInfoUpdated(
const AccountInfo& account_info) {
if (!account_info.IsValid())
return;

OnProfileNameResolved(
profiles::GetDefaultNameForNewSignedInProfile(account_info));
}

void ProfileNameResolver::OnProfileNameResolved(
const std::u16string& profile_name) {
// Stop listening to further changes.
identity_manager_observation_.Reset();

resolved_profile_name_ = profile_name;
if (on_profile_name_resolved_callback_)
std::move(on_profile_name_resolved_callback_).Run();
}

0 comments on commit c00b988

Please sign in to comment.