Skip to content

Commit

Permalink
Restrict GlanceablesV2TrustedTester feature
Browse files Browse the repository at this point in the history
When GlanceablesV2TrustedTester feature is enabled, glanceables should
be enabled only if enabled by policy.

BUG=b/297297434

Change-Id: Ic1343381a3c7bbe8745c58455ba79b1c93716c12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4809358
Reviewed-by: Ana Salazar <anasalazar@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1187974}
  • Loading branch information
Toni Barzic authored and Chromium LUCI CQ committed Aug 24, 2023
1 parent ab8878e commit 5c054ce
Show file tree
Hide file tree
Showing 17 changed files with 99 additions and 49 deletions.
7 changes: 5 additions & 2 deletions ash/constants/ash_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3288,8 +3288,11 @@ bool AreGlanceablesEnabled() {
}

bool AreGlanceablesV2Enabled() {
return base::FeatureList::IsEnabled(kGlanceablesV2) ||
base::FeatureList::IsEnabled(kGlanceablesV2TrustedTesters);
return base::FeatureList::IsEnabled(kGlanceablesV2);
}

bool AreGlanceablesV2EnabledForTrustedTesters() {
return base::FeatureList::IsEnabled(kGlanceablesV2TrustedTesters);
}

bool IsGlanceablesV2ClassroomTeacherViewEnabled() {
Expand Down
1 change: 1 addition & 0 deletions ash/constants/ash_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,7 @@ COMPONENT_EXPORT(ASH_CONSTANTS) bool IsGifRecordingEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsGifRenderingEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool AreGlanceablesEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool AreGlanceablesV2Enabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool AreGlanceablesV2EnabledForTrustedTesters();
COMPONENT_EXPORT(ASH_CONSTANTS)
bool IsGlanceablesV2ClassroomTeacherViewEnabled();
COMPONENT_EXPORT(ASH_CONSTANTS) bool IsHibernateEnabled();
Expand Down
3 changes: 1 addition & 2 deletions ash/glanceables/glanceables_v2_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ void GlanceablesV2Controller::OnActiveUserSessionChanged(
}

bool GlanceablesV2Controller::AreGlanceablesAvailable() const {
return features::AreGlanceablesV2Enabled() &&
(GetClassroomClient() != nullptr || GetTasksClient() != nullptr);
return GetClassroomClient() != nullptr || GetTasksClient() != nullptr;
}

void GlanceablesV2Controller::UpdateClientsRegistration(
Expand Down
3 changes: 2 additions & 1 deletion ash/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1658,7 +1658,8 @@ void Shell::Init(
glanceables_controller_.get()));
}

if (features::AreGlanceablesV2Enabled()) {
if (features::AreGlanceablesV2Enabled() ||
features::AreGlanceablesV2EnabledForTrustedTesters()) {
glanceables_v2_controller_ = std::make_unique<GlanceablesV2Controller>();
}

Expand Down
5 changes: 3 additions & 2 deletions ash/system/time/calendar_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,9 @@ void CalendarHeaderView::UpdateHeaders(const std::u16string& month,
BEGIN_METADATA(CalendarHeaderView, views::View)
END_METADATA

CalendarView::CalendarView(DetailedViewDelegate* delegate)
: GlanceableTrayChildBubble(delegate),
CalendarView::CalendarView(DetailedViewDelegate* delegate,
bool for_glanceables_container)
: GlanceableTrayChildBubble(delegate, for_glanceables_container),
calendar_view_controller_(std::make_unique<CalendarViewController>()),
scrolling_settled_timer_(
FROM_HERE,
Expand Down
5 changes: 4 additions & 1 deletion ash/system/time/calendar_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ class ASH_EXPORT CalendarView : public CalendarModel::Observer,
public:
METADATA_HEADER(CalendarView);

explicit CalendarView(DetailedViewDelegate* delegate);
// `for_glanceables_container` - Whether the calendar view is shown as a
// bubble in glanceables container, or a `UnifiedSystemTrayBubble` (which is
// the case if glanceables feature is not enabled).
CalendarView(DetailedViewDelegate* delegate, bool for_glanceables_container);
CalendarView(const CalendarView& other) = delete;
CalendarView& operator=(const CalendarView& other) = delete;
~CalendarView() override;
Expand Down
7 changes: 4 additions & 3 deletions ash/system/time/calendar_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ class CalendarViewTest : public AshTestBase {
AccountId user_account = AccountId::FromUserEmail(kTestUser);
GetSessionControllerClient()->SwitchActiveUser(user_account);

auto calendar_view = std::make_unique<CalendarView>(delegate_.get());
auto calendar_view = std::make_unique<CalendarView>(
delegate_.get(), /*for_glanceables_container=*/false);

calendar_view_ = widget_->SetContentsView(std::move(calendar_view));
}
Expand Down Expand Up @@ -1425,8 +1426,8 @@ class CalendarViewAnimationTest : public AshTestBase {
}

void CreateCalendarView() {
calendar_view_ = widget_->SetContentsView(
std::make_unique<CalendarView>(delegate_.get()));
calendar_view_ = widget_->SetContentsView(std::make_unique<CalendarView>(
delegate_.get(), /*for_glanceables_container=*/false));
}

// Gets date cell of a given CalendarMonthView and numerical `day`.
Expand Down
3 changes: 2 additions & 1 deletion ash/system/time/unified_calendar_view_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ UnifiedCalendarViewController::~UnifiedCalendarViewController() = default;
std::unique_ptr<views::View> UnifiedCalendarViewController::CreateView() {
DCHECK(!view_);
const base::Time start_time = base::Time::Now();
auto view = std::make_unique<CalendarView>(detailed_view_delegate_.get());
auto view = std::make_unique<CalendarView>(
detailed_view_delegate_.get(), /*for_glanceables_container=*/false);
base::UmaHistogramTimes("Ash.CalendarView.ConstructionTime",
base::Time::Now() - start_time);
view_ = view.get();
Expand Down
2 changes: 1 addition & 1 deletion ash/system/unified/classroom_bubble_base_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ constexpr char kClassroomHomePage[] = "https://classroom.google.com/u/0/h";
ClassroomBubbleBaseView::ClassroomBubbleBaseView(
DetailedViewDelegate* delegate,
std::unique_ptr<ui::ComboboxModel> combobox_model)
: GlanceableTrayChildBubble(delegate) {
: GlanceableTrayChildBubble(delegate, /*for_glanceables_container=*/true) {
auto* layout_manager =
SetLayoutManager(std::make_unique<views::FlexLayout>());
layout_manager
Expand Down
14 changes: 8 additions & 6 deletions ash/system/unified/date_tray.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,10 @@ void DateTray::ShowBubble() {
return;
}

if (ash::Shell::Get()
->glanceables_v2_controller()
->AreGlanceablesAvailable()) {
GlanceablesV2Controller* const glanceables_controller =
ash::Shell::Get()->glanceables_v2_controller();
if (glanceables_controller &&
glanceables_controller->AreGlanceablesAvailable()) {
ShowGlanceableBubble();
}
}
Expand Down Expand Up @@ -133,9 +134,10 @@ void DateTray::OnButtonPressed(const ui::Event& event) {
return;
}

if (ash::Shell::Get()
->glanceables_v2_controller()
->AreGlanceablesAvailable()) {
GlanceablesV2Controller* const glanceables_controller =
ash::Shell::Get()->glanceables_v2_controller();
if (glanceables_controller &&
glanceables_controller->AreGlanceablesAvailable()) {
// Hide the unified_system_tray_ bubble.
unified_system_tray_->CloseBubble();
// Open the glanceables bubble.
Expand Down
5 changes: 3 additions & 2 deletions ash/system/unified/glanceable_tray_bubble_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,9 @@ void GlanceableTrayBubbleView::InitializeContents() {
}

if (!calendar_view_) {
calendar_view_ = child_glanceable_container->AddChildView(
std::make_unique<CalendarView>(detailed_view_delegate_.get()));
calendar_view_ =
child_glanceable_container->AddChildView(std::make_unique<CalendarView>(
detailed_view_delegate_.get(), /*for_glanceables_container=*/true));
// TODO(b:277268122): Update with glanceable spec.
calendar_view_->SetPreferredSize(gfx::Size(kRevampedTrayMenuWidth, 400));
}
Expand Down
8 changes: 3 additions & 5 deletions ash/system/unified/glanceable_tray_child_bubble.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,10 @@ constexpr int kBubbleCornerRadius = 24;
} // namespace

GlanceableTrayChildBubble::GlanceableTrayChildBubble(
DetailedViewDelegate* delegate)
DetailedViewDelegate* delegate,
bool for_glanceables_container)
: TrayDetailedView(delegate) {
// `CalendarView` also extends from this view. If the glanceblae view flag is
// not enabled, the calendar view will be added to the
// `UnifiedSystemTrayBubble` which also has its own style settings.
if (features::AreGlanceablesV2Enabled()) {
if (for_glanceables_container) {
SetAccessibleRole(ax::mojom::Role::kGroup);

SetPaintToLayer();
Expand Down
8 changes: 7 additions & 1 deletion ash/system/unified/glanceable_tray_child_bubble.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ class ASH_EXPORT GlanceableTrayChildBubble : public TrayDetailedView {
public:
METADATA_HEADER(GlanceableTrayChildBubble);

explicit GlanceableTrayChildBubble(DetailedViewDelegate* delegate);
// `for_glanceables_container` - whether the bubble should be styled as a
// bubble in the glanceables container. `CalendarView` is a
// `GlanceablesTrayChildBubble` and if the glanceblae view flag is
// not enabled, the calendar view will be added to the
// `UnifiedSystemTrayBubble` which has its own styling.
GlanceableTrayChildBubble(DetailedViewDelegate* delegate,
bool for_glanceables_container);
GlanceableTrayChildBubble(const GlanceableTrayChildBubble&) = delete;
GlanceableTrayChildBubble& operator-(const GlanceableTrayChildBubble&) =
delete;
Expand Down
2 changes: 1 addition & 1 deletion ash/system/unified/tasks_bubble_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ constexpr char kTasksManagementPage[] =
namespace ash {

TasksBubbleView::TasksBubbleView(DetailedViewDelegate* delegate)
: GlanceableTrayChildBubble(delegate) {
: GlanceableTrayChildBubble(delegate, /*for_glanceables_container=*/true) {
if (Shell::Get()->glanceables_v2_controller()->GetTasksClient()) {
Shell::Get()->glanceables_v2_controller()->GetTasksClient()->GetTaskLists(
base::BindOnce(&TasksBubbleView::InitViews,
Expand Down
54 changes: 38 additions & 16 deletions chrome/browser/ui/ash/glanceables/glanceables_keyed_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,30 @@ GlanceablesKeyedService::GlanceablesKeyedService(Profile* profile)
pref_change_registrar_.Init(pref_service);
pref_change_registrar_.Add(
prefs::kGlanceablesEnabled,
base::BindRepeating(&GlanceablesKeyedService::UpdateRegistrationInAsh,
base::BindRepeating(&GlanceablesKeyedService::UpdateRegistration,
base::Unretained(this)));

CreateClients();
UpdateRegistration();
}

GlanceablesKeyedService::~GlanceablesKeyedService() = default;

void GlanceablesKeyedService::Shutdown() {
classroom_client_.reset();
tasks_client_.reset();
UpdateRegistrationInAsh();
ClearClients();
}

bool GlanceablesKeyedService::AreGlanceablesEnabled() const {
PrefService* const prefs = profile_->GetPrefs();
if (features::AreGlanceablesV2Enabled()) {
return prefs->GetBoolean(prefs::kGlanceablesEnabled);
}

if (features::AreGlanceablesV2EnabledForTrustedTesters()) {
return prefs->IsManagedPreference(prefs::kGlanceablesEnabled) &&
prefs->GetBoolean(prefs::kGlanceablesEnabled);
}

return false;
}

std::unique_ptr<google_apis::RequestSender>
Expand All @@ -84,7 +96,7 @@ GlanceablesKeyedService::CreateRequestSenderForClient(
/*custom_user_agent=*/std::string(), traffic_annotation_tag);
}

void GlanceablesKeyedService::CreateClients() {
void GlanceablesKeyedService::RegisterClients() {
const auto create_request_sender_callback = base::BindRepeating(
&GlanceablesKeyedService::CreateRequestSenderForClient,
base::Unretained(this));
Expand All @@ -94,10 +106,23 @@ void GlanceablesKeyedService::CreateClients() {
tasks_client_ = std::make_unique<GlanceablesTasksClientImpl>(
create_request_sender_callback);

UpdateRegistrationInAsh();
Shell::Get()->glanceables_v2_controller()->UpdateClientsRegistration(
account_id_, GlanceablesV2Controller::ClientsRegistration{
.classroom_client = classroom_client_.get(),
.tasks_client = tasks_client_.get()});
}

void GlanceablesKeyedService::ClearClients() {
classroom_client_.reset();
tasks_client_.reset();
if (Shell::HasInstance()) {
Shell::Get()->glanceables_v2_controller()->UpdateClientsRegistration(
account_id_, GlanceablesV2Controller::ClientsRegistration{
.classroom_client = nullptr, .tasks_client = nullptr});
}
}

void GlanceablesKeyedService::UpdateRegistrationInAsh() const {
void GlanceablesKeyedService::UpdateRegistration() {
if (!Shell::HasInstance()) {
return;
}
Expand All @@ -108,17 +133,14 @@ void GlanceablesKeyedService::UpdateRegistrationInAsh() const {

CHECK(prefs);

if (!prefs->GetBoolean(prefs::kGlanceablesEnabled)) {
Shell::Get()->glanceables_v2_controller()->UpdateClientsRegistration(
account_id_, GlanceablesV2Controller::ClientsRegistration{
.classroom_client = nullptr, .tasks_client = nullptr});
if (!AreGlanceablesEnabled()) {
ClearClients();
return;
}

Shell::Get()->glanceables_v2_controller()->UpdateClientsRegistration(
account_id_, GlanceablesV2Controller::ClientsRegistration{
.classroom_client = classroom_client_.get(),
.tasks_client = tasks_client_.get()});
if (!classroom_client_ || !tasks_client_) {
RegisterClients();
}
}

} // namespace ash
18 changes: 14 additions & 4 deletions chrome/browser/ui/ash/glanceables/glanceables_keyed_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ class GlanceablesKeyedService : public KeyedService {
void Shutdown() override;

private:
// Returns whether glanceables are enabled for the profile that owns the
// GlanceablesKeyedService.
bool AreGlanceablesEnabled() const;

// Helper method that creates a `google_apis::RequestSender` instance.
// `scopes` - OAuth 2 scopes needed for a client.
// `traffic_annotation_tag` - describes requests issued by a client (for more
Expand All @@ -62,11 +66,17 @@ class GlanceablesKeyedService : public KeyedService {
const std::vector<std::string>& scopes,
const net::NetworkTrafficAnnotationTag& traffic_annotation_tag) const;

// Creates clients needed to communicate with different Google services.
void CreateClients();
// Creates clients needed to communicate with different Google services, and
// registers them with glanceables v2 controller in ash.
void RegisterClients();

// Resets clients needed to communicate with different Google services, and
// clears any existing registrations with glanceables v2 controller in ash.
void ClearClients();

// Notifies `ash/` about created clients for `account_id_`.
void UpdateRegistrationInAsh() const;
// Creates or clear clients that communicated with Google services, and
// notifies `ash/` about created clients for `account_id_`.
void UpdateRegistration();

// The profile for which this keyed service was created.
const raw_ptr<Profile, ExperimentalAsh> profile_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ GlanceablesKeyedService* GlanceablesKeyedServiceFactory::GetService(
content::BrowserContext* context) {
return static_cast<GlanceablesKeyedService*>(
GetInstance()->GetServiceForBrowserContext(
context, /*create=*/features::AreGlanceablesV2Enabled()));
context, /*create=*/features::AreGlanceablesV2Enabled() ||
features::AreGlanceablesV2EnabledForTrustedTesters()));
}

std::unique_ptr<KeyedService>
Expand Down

0 comments on commit 5c054ce

Please sign in to comment.