Ensure owner id is present before show gaia signin
The check for isApprovalRequired requires the device owner ID to be
available, which it may not yet be during signin. Ensure the ID is
available, and add a comment to the isApprovalRequired function to
clarify this requirement.
Bug: 1143369
Change-Id: I315215f173c4a8a3ce0a37a52f4e3109dddae1eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3543829
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Commit-Queue: Galen Corey <galenemco@chromium.org>
Cr-Commit-Position: refs/heads/main@{#986077}
diff --git a/chrome/browser/ash/child_accounts/parent_access_code/parent_access_service.cc b/chrome/browser/ash/child_accounts/parent_access_code/parent_access_service.cc
index d532c4c7..26be0e8e 100644
--- a/chrome/browser/ash/child_accounts/parent_access_code/parent_access_service.cc
+++ b/chrome/browser/ash/child_accounts/parent_access_code/parent_access_service.cc
@@ -29,8 +29,6 @@
// Returns true when the device owner is a child.
bool IsDeviceOwnedByChild() {
- // TODO(crbug.com/1143369): Owner id might not be available early after
- // startup. Wait for it to be ready.
AccountId owner_account_id =
user_manager::UserManager::Get()->GetOwnerAccountId();
if (owner_account_id.empty()) {
diff --git a/chrome/browser/ash/child_accounts/parent_access_code/parent_access_service.h b/chrome/browser/ash/child_accounts/parent_access_code/parent_access_service.h
index cf7f88d7..5c5500f2 100644
--- a/chrome/browser/ash/child_accounts/parent_access_code/parent_access_service.h
+++ b/chrome/browser/ash/child_accounts/parent_access_code/parent_access_service.h
@@ -51,6 +51,8 @@
ParentAccessService& operator=(const ParentAccessService&) = delete;
// Checks if the provided |action| requires parental approval to be performed.
+ // Requires owner_account_id to be available in the UserManager, so if calling
+ // close to startup, ensure owner account is set before calling.
static bool IsApprovalRequired(SupervisedAction action);
// Checks if |access_code| is valid for the user identified by |account_id|.
diff --git a/chrome/browser/ui/ash/login_screen_client_impl.cc b/chrome/browser/ui/ash/login_screen_client_impl.cc
index e62c56a..5aa59a0 100644
--- a/chrome/browser/ui/ash/login_screen_client_impl.cc
+++ b/chrome/browser/ui/ash/login_screen_client_impl.cc
@@ -6,11 +6,13 @@
#include <utility>
+#include "ash/components/settings/cros_settings_provider.h"
#include "ash/public/cpp/child_accounts/parent_access_controller.h"
#include "ash/public/cpp/login_screen.h"
#include "ash/public/cpp/login_screen_model.h"
#include "base/bind.h"
#include "base/memory/scoped_refptr.h"
+#include "base/metrics/histogram_functions.h"
#include "chrome/browser/ash/child_accounts/parent_access_code/parent_access_service.h"
#include "chrome/browser/ash/login/existing_user_controller.h"
#include "chrome/browser/ash/login/hats_unlock_survey_trigger.h"
@@ -25,6 +27,7 @@
#include "chrome/browser/ash/login/ui/login_display_host.h"
#include "chrome/browser/ash/login/ui/login_display_host_webui.h"
#include "chrome/browser/ash/login/ui/user_adding_screen.h"
+#include "chrome/browser/ash/settings/cros_settings.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profile_metrics.h"
#include "chrome/browser/ui/ash/wallpaper_controller_client_impl.h"
@@ -37,7 +40,6 @@
#include "components/user_manager/remove_user_delegate.h"
#include "components/user_manager/user_names.h"
#include "ui/base/ime/ash/input_method_manager.h"
-
namespace {
using ash::SupervisedAction;
@@ -186,6 +188,34 @@
}
void LoginScreenClientImpl::ShowGaiaSignin(const AccountId& prefilled_account) {
+ if (time_show_gaia_signin_initiated_.is_null())
+ time_show_gaia_signin_initiated_ = base::TimeTicks::Now();
+ // Check trusted status as a workaround to ensure that device owner id is
+ // ready. Device owner ID is necessary for IsApprovalRequired checks.
+ const ash::CrosSettingsProvider::TrustedStatus status =
+ ash::CrosSettings::Get()->PrepareTrustedValues(
+ base::BindOnce(&LoginScreenClientImpl::ShowGaiaSignin,
+ weak_ptr_factory_.GetWeakPtr(), prefilled_account));
+ switch (status) {
+ case ash::CrosSettingsProvider::TRUSTED:
+ // Owner account ID is available. Record time spent waiting for owner
+ // account ID and continue showing Gaia Signin.
+ base::UmaHistogramTimes(
+ "Ash.Login.ShowGaiaSignin.WaitTime",
+ base::TimeTicks::Now() - time_show_gaia_signin_initiated_);
+ time_show_gaia_signin_initiated_ = base::TimeTicks();
+ break;
+ case ash::CrosSettingsProvider::TEMPORARILY_UNTRUSTED:
+ // Do nothing. This function will be called again when the values are
+ // ready.
+ return;
+ case ash::CrosSettingsProvider::PERMANENTLY_UNTRUSTED:
+ base::UmaHistogramBoolean("Ash.Login.ShowGaiaSignin.PermanentlyUntrusted",
+ true);
+ time_show_gaia_signin_initiated_ = base::TimeTicks();
+ return;
+ }
+
auto supervised_action = prefilled_account.empty()
? SupervisedAction::kAddUser
: SupervisedAction::kReauth;
diff --git a/chrome/browser/ui/ash/login_screen_client_impl.h b/chrome/browser/ui/ash/login_screen_client_impl.h
index 9e87c63..ac2401ff 100644
--- a/chrome/browser/ui/ash/login_screen_client_impl.h
+++ b/chrome/browser/ui/ash/login_screen_client_impl.h
@@ -160,6 +160,8 @@
base::ObserverList<LoginScreenShownObserver> login_screen_shown_observers_;
+ base::TimeTicks time_show_gaia_signin_initiated_;
+
base::WeakPtrFactory<LoginScreenClientImpl> weak_ptr_factory_{this};
};
diff --git a/tools/metrics/histograms/metadata/ash/histograms.xml b/tools/metrics/histograms/metadata/ash/histograms.xml
index f68e0a7..3086a42 100644
--- a/tools/metrics/histograms/metadata/ash/histograms.xml
+++ b/tools/metrics/histograms/metadata/ash/histograms.xml
@@ -2126,6 +2126,32 @@
</summary>
</histogram>
+<histogram name="Ash.Login.ShowGaiaSignin.PermanentlyUntrusted" enum="Boolean"
+ expires_after="2023-01-20">
+ <owner>agawronska@chromium.org</owner>
+ <owner>galenemco@google.com</owner>
+ <owner>cros-families-eng@google.com</owner>
+ <summary>
+ Records instances of a "PERMANENTLY_UNTRUSTED" result during the
+ check for trusted values before showing gaia signin. This is a fatal result
+ which will result in signin not being shown.
+ </summary>
+</histogram>
+
+<histogram name="Ash.Login.ShowGaiaSignin.WaitTime" units="ms"
+ expires_after="2023-01-20">
+ <owner>agawronska@chromium.org</owner>
+ <owner>galenemco@google.com</owner>
+ <owner>cros-families-eng@google.com</owner>
+ <summary>
+ Records the amount of time spent waiting for trusted values before showing
+ gaia signin. Waiting for trusted values ensures that the device owner id is
+ present before showing the signin, which is necessary for checking signin
+ permissions for child users. This check may introduce a delay for all users.
+ This metric allows monitoring that delay.
+ </summary>
+</histogram>
+
<histogram base="true" name="Ash.LoginAnimation.Duration" units="ms"
expires_after="2022-04-17">
<owner>oshima@chromium.org</owner>
