Skip to content

Commit

Permalink
[ChromeSigninIntercept] React to Chrome Signin bubble buttons
Browse files Browse the repository at this point in the history
On accept: Signin in to chrome with the account and close the bubble.
On decline: Close the bubble.

Metrics to be recorded in a followup CL.

Bug: b:301431278
Low-Coverage-Reason: OTHER One file affected indirectly that is not part of the main changes.
Change-Id: I0afb6f9de3dcc855caf486b56f1c559e4b090777
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4965921
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Ryan Sultanem <rsult@google.com>
Cr-Commit-Position: refs/heads/main@{#1215447}
  • Loading branch information
Ryan Sultanem authored and Chromium LUCI CQ committed Oct 26, 2023
1 parent a9db8ef commit 0344235
Show file tree
Hide file tree
Showing 14 changed files with 274 additions and 164 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ export class ChromeSigninAppElement extends ChromeSigninAppElementBase {
}

private onAccept_() {
// TODO(b/301431278): change to accept when the reaction is implemented.
this.diceWebSigninInterceptBrowserProxy_.cancel();
this.diceWebSigninInterceptBrowserProxy_.accept();
}
}

Expand Down
30 changes: 24 additions & 6 deletions chrome/browser/signin/dice_web_signin_interceptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "base/i18n/case_conversion.h"
#include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_functions.h"
#include "base/notreached.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "base/task/single_thread_task_runner.h"
Expand Down Expand Up @@ -717,7 +718,7 @@ void DiceWebSigninInterceptor::OnInterceptionReadyToBeProcessed(
break;
case WebSigninInterceptor::SigninInterceptionType::kChromeSignin:
callback = base::BindOnce(&DiceWebSigninInterceptor::OnChromeSigninChoice,
base::Unretained(this));
base::Unretained(this), info);
break;
}
ShowSigninInterceptionBubble(bubble_parameters, std::move(callback));
Expand Down Expand Up @@ -771,13 +772,30 @@ void DiceWebSigninInterceptor::OnProfileCreationChoice(
}

void DiceWebSigninInterceptor::OnChromeSigninChoice(
const AccountInfo& account_info,
SigninInterceptionResult result) {
if (result == SigninInterceptionResult::kDeclined) {
Reset();
return;
}
// In all cases we want to close the bubble after the choice is taken.
Reset();

// TODO(b/301431278): Implement the rest of the cases.
switch (result) {
case SigninInterceptionResult::kIgnored:
// Can happen if the browser isclosed while the bubble is still opened.
case SigninInterceptionResult::kNotDisplayed:
// Can happen if the web contents is destroyed between the time the bubble
// was requested to be displayed and actually being displayed.
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.";
case SigninInterceptionResult::kAccepted:
identity_manager_->GetPrimaryAccountMutator()->SetPrimaryAccount(
account_info.account_id, signin::ConsentLevel::kSignin,
signin_metrics::AccessPoint::
ACCESS_POINT_CHROME_SIGNIN_INTERCEPT_BUBBLE);
}
}

void DiceWebSigninInterceptor::OnProfileSwitchChoice(
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/signin/dice_web_signin_interceptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ class DiceWebSigninInterceptor : public KeyedService,
SigninInterceptionResult switch_profile);
// Called after the user chose whether they want to sign in to chrome or not
// via the Chrome Signin Bubble.
void OnChromeSigninChoice(SigninInterceptionResult result);
void OnChromeSigninChoice(const AccountInfo& account_info,
SigninInterceptionResult result);

// Called when the new profile is created or loaded from disk.
// `profile_color` is set as theme color for the profile ; it should be
Expand Down

0 comments on commit 0344235

Please sign in to comment.