Skip to content

Commit

Permalink
[DiceWebSigninIntercept] Cleanup Guest option
Browse files Browse the repository at this point in the history
The guest option was added as part of the Guest as Ephemeral profile.
The project is now cancelled; removing dead code, which was not tested.

Bug: b:307909028, 1225171
Change-Id: I1a98946da5ad37c0c58e9cbb4fdf41bf66386bb1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4975130
Commit-Queue: Ryan Sultanem <rsult@google.com>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216217}
  • Loading branch information
Ryan Sultanem authored and Chromium LUCI CQ committed Oct 27, 2023
1 parent 2fc962e commit b3f3c75
Show file tree
Hide file tree
Showing 25 changed files with 56 additions and 213 deletions.
3 changes: 0 additions & 3 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -12093,9 +12093,6 @@ This can include information about installed software, files, your browser, and
<message name="IDS_SIGNIN_DICE_WEB_INTERCEPT_BUBBLE_CONFIRM_SWITCH_BUTTON_LABEL_V2" desc="Label of the profile switch button in the web signin interception bubble">
Continue
</message>
<message name="IDS_SIGNIN_DICE_WEB_INTERCEPT_BUBBLE_GUEST_LINK" desc="Text of the link to offer switching to a Guest profile">
Not your device? Use <ph name="BEGIN_LINK">&lt;a is="action-link" target="_blank"&gt;</ph>Guest mode<ph name="END_LINK">&lt;/a&gt;</ph>.
</message>
<message name="IDS_SIGNIN_DICE_WEB_INTERCEPT_BUBBLE_CHROME_SIGNIN_ACCEPT_TEXT" desc="Label of the Accept button of the Chrome Signin Bubble intercept that is shown when a signed out user signs in on the web with it's first account. It also uses the given name of the user.">
Continue as <ph name="NAME">$1<ex>Bob</ex></ph>
</message>
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion chrome/browser/profiles/off_the_record_profile_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ profile_metrics::BrowserProfileType ComputeOffTheRecordProfileType(

case profile_metrics::BrowserProfileType::kIncognito:
case profile_metrics::BrowserProfileType::kOtherOffTheRecordProfile:
case profile_metrics::BrowserProfileType::kDeprecatedEphemeralGuest:
NOTREACHED();
}
return profile_metrics::BrowserProfileType::kOtherOffTheRecordProfile;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,7 @@
font-size: 12px;
}

#interceptDialog [slot=footer] {
background: var(--paper-grey-50);
border-top: none;
padding: 0;
}

@media (prefers-color-scheme: dark) {
#interceptDialog [slot=footer] {
background: rgb(50, 54, 57); /* Custom color from Namrata. */
}

#headerV2 {
/* Colors used in svg image file for split header border and box. */
--split-header-image-border-color-fill: var(--google-grey-700);
Expand All @@ -221,16 +211,6 @@
color: var(--google-grey-100);
}
}

.divider {
border-top: var(--cr-separator-line);
}

#footer-description {
color: var(--cr-secondary-text-color);
padding: 16px;
text-align: center;
}
</style>

<div role="dialog" id="interceptDialog" aria-labelledby="title"
Expand Down Expand Up @@ -307,13 +287,4 @@
</cr-button>
</div>
</div>
<template is="dom-if" if="[[interceptionParameters_.showGuestOption]]"
restamp>
<div slot="footer">
<div class="divider"></div>
<div id="footer-description"
inner-h-t-m-l="[[guestLink_]]">
</div>
</div>
</template>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import './strings.m.js';

import {CrButtonElement} from 'chrome://resources/cr_elements/cr_button/cr_button.js';
import {WebUiListenerMixin} from 'chrome://resources/cr_elements/web_ui_listener_mixin.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
import {sanitizeInnerHtml} from 'chrome://resources/js/parse_html_subset.js';
import {afterNextRender, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

Expand Down Expand Up @@ -49,20 +48,11 @@ export class DiceWebSigninInterceptAppElement extends
type: Boolean,
value: false,
},

guestLink_: {
type: String,
value() {
return sanitizeInnerHtml(
loadTimeData.getString('guestLink'), {attrs: ['is']});
},
},
};
}

private interceptionParameters_: InterceptionParameters;
private acceptButtonClicked_: boolean;
private guestLink_: TrustedHTML;
private diceWebSigninInterceptBrowserProxy_:
DiceWebSigninInterceptBrowserProxy =
DiceWebSigninInterceptBrowserProxyImpl.getInstance();
Expand All @@ -84,12 +74,6 @@ export class DiceWebSigninInterceptAppElement extends
this.shadowRoot!.querySelector<HTMLElement>(
'#interceptDialog')!.offsetHeight;
this.diceWebSigninInterceptBrowserProxy_.initializedWithHeight(height);
// |showGuestOption| is constant during the lifetime of this bubble,
// therefore it's safe to set the listener only during initialization.
if (this.interceptionParameters_.showGuestOption) {
this.shadowRoot!.querySelector('#footer-description a')!
.addEventListener('click', () => this.onGuest_());
}
});
}

Expand All @@ -102,14 +86,6 @@ export class DiceWebSigninInterceptAppElement extends
this.diceWebSigninInterceptBrowserProxy_.cancel();
}

private onGuest_() {
if (this.acceptButtonClicked_) {
return;
}
this.acceptButtonClicked_ = true;
this.diceWebSigninInterceptBrowserProxy_.guest();
}

/** Called when the interception parameters are updated. */
private handleParametersChanged_(parameters: InterceptionParameters) {
this.interceptionParameters_ = parameters;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export interface InterceptionParameters {
primaryProfileColor: string;
interceptedAccount: AccountInfo;
primaryAccount: AccountInfo;
showGuestOption: boolean;
useV2Design: boolean;
showManagedDisclaimer: boolean;
}
Expand All @@ -45,9 +44,6 @@ export interface DiceWebSigninInterceptBrowserProxy {
// Called when the user cancels the interception.
cancel(): void;

// Called when user selects Guest mode.
guest(): void;

// Called when the page is loaded.
pageLoaded(): Promise<InterceptionParameters>;

Expand All @@ -70,10 +66,6 @@ export class DiceWebSigninInterceptBrowserProxyImpl implements
chrome.send('cancel');
}

guest() {
chrome.send('guest');
}

pageLoaded() {
return sendWithPromise('pageLoaded');
}
Expand Down
46 changes: 12 additions & 34 deletions chrome/browser/signin/dice_signed_in_profile_creator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,45 +99,23 @@ DiceSignedInProfileCreator::DiceSignedInProfileCreator(
CoreAccountId account_id,
const std::u16string& local_profile_name,
absl::optional<size_t> icon_index,
bool use_guest_profile,
base::OnceCallback<void(Profile*)> callback)
: source_profile_(source_profile),
account_id_(account_id),
callback_(std::move(callback)) {
auto initialized_callback =
base::BindOnce(&DiceSignedInProfileCreator::OnNewProfileInitialized,
weak_pointer_factory_.GetWeakPtr());

// Passing the sign-in token to an ephemeral Guest profile is part of the
// experiment to surface a Guest mode link in the DiceWebSigninIntercept
// and is only used to sign in to the web through account consistency and
// does NOT enable sync or any other browser level functionality.
// TODO(https://crbug.com/1225171): Revise the comment after Guest mode plans
// are finalized.
if (use_guest_profile) {
// TODO(https://crbug.com/1225171): Re-enabled if ephemeral based Guest mode
// is added. Remove the code otherwise.
NOTREACHED();

// Make sure the callback is not called synchronously.
base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&ProfileManager::CreateProfileAsync,
base::Unretained(g_browser_process->profile_manager()),
ProfileManager::GetGuestProfilePath(),
std::move(initialized_callback), base::DoNothing()));
} else {
ProfileAttributesStorage& storage =
g_browser_process->profile_manager()->GetProfileAttributesStorage();
if (!icon_index.has_value())
icon_index = storage.ChooseAvatarIconIndexForNewProfile();
std::u16string name = local_profile_name.empty()
? storage.ChooseNameForNewProfile(*icon_index)
: local_profile_name;
ProfileManager::CreateMultiProfileAsync(name, *icon_index,
/*is_hidden=*/false,
std::move(initialized_callback));
ProfileAttributesStorage& storage =
g_browser_process->profile_manager()->GetProfileAttributesStorage();
if (!icon_index.has_value()) {
icon_index = storage.ChooseAvatarIconIndexForNewProfile();
}
std::u16string name = local_profile_name.empty()
? storage.ChooseNameForNewProfile(*icon_index)
: local_profile_name;
ProfileManager::CreateMultiProfileAsync(
name, *icon_index,
/*is_hidden=*/false,
base::BindOnce(&DiceSignedInProfileCreator::OnNewProfileInitialized,
weak_pointer_factory_.GetWeakPtr()));
}

DiceSignedInProfileCreator::DiceSignedInProfileCreator(
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/signin/dice_signed_in_profile_creator.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class DiceSignedInProfileCreator {
CoreAccountId account_id,
const std::u16string& local_profile_name,
absl::optional<size_t> icon_index,
bool use_guest_profile,
base::OnceCallback<void(Profile*)> callback);

// Uses this version when the profile already exists at `target_profile_path`
Expand Down
18 changes: 4 additions & 14 deletions chrome/browser/signin/dice_signed_in_profile_creator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ std::unique_ptr<TestingProfile> BuildTestingProfile(const base::FilePath& path,
IdentityTestEnvironmentProfileAdaptor adaptor(profile.get());
adaptor.identity_test_env()->ResetToAccountsNotYetLoadedFromDiskState();
}
if (profile->GetPath() == ProfileManager::GetGuestProfilePath())
profile->SetGuestSession(true);

return profile;
}

Expand Down Expand Up @@ -165,8 +164,6 @@ class DiceSignedInProfileCreatorTest
profile_added_closure_ = std::move(closure);
}

bool use_guest_profile() const { return use_guest_profile_; }

void DeleteProfiles() {
identity_test_env_profile_adaptor_.reset();

Expand Down Expand Up @@ -277,7 +274,6 @@ class DiceSignedInProfileCreatorTest
base::OnceClosure profile_added_closure_;
bool creator_callback_called_ = false;
base::test::ScopedFeatureList scoped_feature_list_;
bool use_guest_profile_ = false;
};

TEST_P(DiceSignedInProfileCreatorTest, CreateWithTokensLoaded) {
Expand All @@ -290,7 +286,6 @@ TEST_P(DiceSignedInProfileCreatorTest, CreateWithTokensLoaded) {
std::unique_ptr<DiceSignedInProfileCreator> creator =
std::make_unique<DiceSignedInProfileCreator>(
profile(), account_info.account_id, kProfileTestName, kTestIcon,
use_guest_profile(),
base::BindOnce(&DiceSignedInProfileCreatorTest::OnProfileCreated,
base::Unretained(this), loop.QuitClosure()));
loop.Run();
Expand All @@ -309,18 +304,16 @@ TEST_P(DiceSignedInProfileCreatorTest, CreateWithTokensLoaded) {
->HasAccountWithRefreshToken(account_info.account_id));

// Check profile type
ASSERT_EQ(use_guest_profile(), signed_in_profile()->IsGuestSession());
ASSERT_FALSE(signed_in_profile()->IsGuestSession());

// Check the profile name and icon.
ProfileAttributesStorage& storage =
profile_manager()->GetProfileAttributesStorage();
ProfileAttributesEntry* entry =
storage.GetProfileAttributesWithPath(signed_in_profile()->GetPath());
ASSERT_TRUE(entry);
if (!use_guest_profile()) {
EXPECT_EQ(kProfileTestName, entry->GetLocalProfileName());
EXPECT_EQ(kTestIcon, entry->GetAvatarIconIndex());
}
EXPECT_EQ(kProfileTestName, entry->GetLocalProfileName());
EXPECT_EQ(kTestIcon, entry->GetAvatarIconIndex());
VerifyCookiesMoved();
}

Expand All @@ -336,7 +329,6 @@ TEST_P(DiceSignedInProfileCreatorTest, CreateWithTokensNotLoaded) {
std::unique_ptr<DiceSignedInProfileCreator> creator =
std::make_unique<DiceSignedInProfileCreator>(
profile(), account_info.account_id, std::u16string(), absl::nullopt,
use_guest_profile(),
base::BindOnce(&DiceSignedInProfileCreatorTest::OnProfileCreated,
base::Unretained(this), creator_loop.QuitClosure()));
profile_added_loop.Run();
Expand Down Expand Up @@ -374,7 +366,6 @@ TEST_P(DiceSignedInProfileCreatorTest, DeleteWhileCreating) {
std::unique_ptr<DiceSignedInProfileCreator> creator =
std::make_unique<DiceSignedInProfileCreator>(
profile(), account_info.account_id, std::u16string(), absl::nullopt,
use_guest_profile(),
base::BindOnce(&DiceSignedInProfileCreatorTest::OnProfileCreated,
base::Unretained(this), base::OnceClosure()));
EXPECT_FALSE(creator_callback_called());
Expand All @@ -395,7 +386,6 @@ TEST_P(DiceSignedInProfileCreatorTest, DeleteProfile) {
std::unique_ptr<DiceSignedInProfileCreator> creator =
std::make_unique<DiceSignedInProfileCreator>(
profile(), account_info.account_id, std::u16string(), absl::nullopt,
use_guest_profile(),
base::BindOnce(&DiceSignedInProfileCreatorTest::OnProfileCreated,
base::Unretained(this), creator_loop.QuitClosure()));
profile_added_loop.Run();
Expand Down
8 changes: 2 additions & 6 deletions chrome/browser/signin/dice_web_signin_interceptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -692,8 +692,7 @@ void DiceWebSigninInterceptor::OnInterceptionReadyToBeProcessed(
WebSigninInterceptor::Delegate::BubbleParameters bubble_parameters(
*interception_type, info, GetPrimaryAccountInfo(identity_manager_),
GetAutogeneratedThemeColors(profile_color).frame_color,
/*show_guest_option=*/false, show_link_data_option,
show_managed_disclaimer);
show_link_data_option, show_managed_disclaimer);

base::OnceCallback<void(SigninInterceptionResult)> callback;
switch (*interception_type) {
Expand Down Expand Up @@ -748,8 +747,7 @@ void DiceWebSigninInterceptor::OnProfileCreationChoice(
const AccountInfo& account_info,
SkColor profile_color,
SigninInterceptionResult create) {
if (create != SigninInterceptionResult::kAccepted &&
create != SigninInterceptionResult::kAcceptedWithGuest) {
if (create != SigninInterceptionResult::kAccepted) {
if (create == SigninInterceptionResult::kDeclined)
RecordProfileCreationDeclined(account_info.email);
Reset();
Expand All @@ -766,7 +764,6 @@ void DiceWebSigninInterceptor::OnProfileCreationChoice(
std::make_unique<DiceSignedInProfileCreator>(
profile_, account_id_, profile_name,
profiles::GetPlaceholderAvatarIndex(),
create == SigninInterceptionResult::kAcceptedWithGuest,
base::BindOnce(&DiceWebSigninInterceptor::OnNewSignedInProfileCreated,
base::Unretained(this), profile_color));
}
Expand All @@ -786,7 +783,6 @@ void DiceWebSigninInterceptor::OnChromeSigninChoice(
case SigninInterceptionResult::kDeclined:
// The user declined the bubble.
break;
case SigninInterceptionResult::kAcceptedWithGuest:
case SigninInterceptionResult::kAcceptedWithExistingProfile:
NOTREACHED_NORETURN()
<< "Those results are not expected within the Chrome Signin Bubble.";
Expand Down

0 comments on commit b3f3c75

Please sign in to comment.