Enable Projector for managed user by default.
The feature is gated by Admin policy (disabled by default) after this CL. This CL also fixed a few issues with the flag enabled by default: - Avoid creating MarkerController in unit test. - Avoid creating ProjectorSession in unit test. - Check ProjectorClient before using as it could be nullptr in tests - Check before observing DriveFs Host. Can't use ScopedObservation in this case because we observe DriveFS host conditionally. Bug: b/209675088 Change-Id: I6dc6ab7ada2dd75db879f629be9431f41f909eab Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3360651 Reviewed-by: Xiqi Ruan <xiqiruan@chromium.org> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: Li Lin <llin@chromium.org> Cr-Commit-Position: refs/heads/main@{#961206}
This commit is contained in:
ash
constants
marker
projector
chrome/browser/ui/ash/projector
@@ -1052,7 +1052,7 @@ const base::Feature kProjector{"Projector", base::FEATURE_DISABLED_BY_DEFAULT};
|
|||||||
|
|
||||||
// Controls whether to enable Projector for managed users.
|
// Controls whether to enable Projector for managed users.
|
||||||
const base::Feature kProjectorManagedUser{"ProjectorManagedUser",
|
const base::Feature kProjectorManagedUser{"ProjectorManagedUser",
|
||||||
base::FEATURE_DISABLED_BY_DEFAULT};
|
base::FEATURE_ENABLED_BY_DEFAULT};
|
||||||
|
|
||||||
// Controls whether to enable Projector annotator or marker tools.
|
// Controls whether to enable Projector annotator or marker tools.
|
||||||
// The annotator tools are newer and based on the ink library.
|
// The annotator tools are newer and based on the ink library.
|
||||||
|
@@ -9,6 +9,7 @@
|
|||||||
#include "ash/public/cpp/stylus_utils.h"
|
#include "ash/public/cpp/stylus_utils.h"
|
||||||
#include "ash/shell.h"
|
#include "ash/shell.h"
|
||||||
#include "ash/test/ash_test_base.h"
|
#include "ash/test/ash_test_base.h"
|
||||||
|
#include "base/test/scoped_feature_list.h"
|
||||||
#include "ui/events/test/event_generator.h"
|
#include "ui/events/test/event_generator.h"
|
||||||
|
|
||||||
namespace ash {
|
namespace ash {
|
||||||
@@ -39,18 +40,21 @@ class MarkerControllerTest : public AshTestBase {
|
|||||||
|
|
||||||
// AshTestBase:
|
// AshTestBase:
|
||||||
void SetUp() override {
|
void SetUp() override {
|
||||||
|
scoped_feature_list_.InitWithFeatures(
|
||||||
|
/*enabled_features=*/{features::kProjector,
|
||||||
|
features::kProjectorManagedUser},
|
||||||
|
/*disabled_features=*/{});
|
||||||
AshTestBase::SetUp();
|
AshTestBase::SetUp();
|
||||||
observer_ = std::make_unique<TestMarkerObserver>();
|
observer_ = std::make_unique<TestMarkerObserver>();
|
||||||
controller_ = std::make_unique<MarkerController>();
|
controller_ = MarkerController::Get();
|
||||||
controller_test_api_ =
|
controller_test_api_ =
|
||||||
std::make_unique<MarkerControllerTestApi>(controller_.get());
|
std::make_unique<MarkerControllerTestApi>(controller_);
|
||||||
}
|
}
|
||||||
|
|
||||||
void TearDown() override {
|
void TearDown() override {
|
||||||
// This needs to be called first to remove the event handler before the
|
// This needs to be called first to remove the event handler before the
|
||||||
// shell instance gets torn down.
|
// shell instance gets torn down.
|
||||||
controller_test_api_.reset();
|
controller_test_api_.reset();
|
||||||
controller_.reset();
|
|
||||||
AshTestBase::TearDown();
|
AshTestBase::TearDown();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -166,7 +170,8 @@ class MarkerControllerTest : public AshTestBase {
|
|||||||
|
|
||||||
TestMarkerObserver* observer() { return observer_.get(); }
|
TestMarkerObserver* observer() { return observer_.get(); }
|
||||||
|
|
||||||
std::unique_ptr<MarkerController> controller_;
|
base::test::ScopedFeatureList scoped_feature_list_;
|
||||||
|
MarkerController* controller_;
|
||||||
std::unique_ptr<MarkerControllerTestApi> controller_test_api_;
|
std::unique_ptr<MarkerControllerTestApi> controller_test_api_;
|
||||||
std::unique_ptr<TestMarkerObserver> observer_;
|
std::unique_ptr<TestMarkerObserver> observer_;
|
||||||
};
|
};
|
||||||
|
@@ -4,11 +4,13 @@
|
|||||||
|
|
||||||
#include "ash/projector/model/projector_session_impl.h"
|
#include "ash/projector/model/projector_session_impl.h"
|
||||||
|
|
||||||
|
#include "ash/constants/ash_features.h"
|
||||||
#include "ash/projector/projector_metrics.h"
|
#include "ash/projector/projector_metrics.h"
|
||||||
#include "ash/public/cpp/projector/projector_session.h"
|
#include "ash/public/cpp/projector/projector_session.h"
|
||||||
#include "ash/test/ash_test_base.h"
|
#include "ash/test/ash_test_base.h"
|
||||||
#include "base/dcheck_is_on.h"
|
#include "base/dcheck_is_on.h"
|
||||||
#include "base/test/metrics/histogram_tester.h"
|
#include "base/test/metrics/histogram_tester.h"
|
||||||
|
#include "base/test/scoped_feature_list.h"
|
||||||
|
|
||||||
namespace ash {
|
namespace ash {
|
||||||
|
|
||||||
@@ -28,12 +30,17 @@ class ProjectorSessionImplTest : public AshTestBase {
|
|||||||
|
|
||||||
// AshTestBase:
|
// AshTestBase:
|
||||||
void SetUp() override {
|
void SetUp() override {
|
||||||
|
scoped_feature_list_.InitWithFeatures(
|
||||||
|
/*enabled_features=*/{features::kProjector,
|
||||||
|
features::kProjectorManagedUser},
|
||||||
|
/*disabled_features=*/{});
|
||||||
AshTestBase::SetUp();
|
AshTestBase::SetUp();
|
||||||
session_ = std::make_unique<ProjectorSessionImpl>();
|
session_ = static_cast<ProjectorSessionImpl*>(ProjectorSession::Get());
|
||||||
}
|
}
|
||||||
|
|
||||||
protected:
|
protected:
|
||||||
std::unique_ptr<ProjectorSessionImpl> session_;
|
base::test::ScopedFeatureList scoped_feature_list_;
|
||||||
|
ProjectorSessionImpl* session_;
|
||||||
};
|
};
|
||||||
|
|
||||||
TEST_F(ProjectorSessionImplTest, Start) {
|
TEST_F(ProjectorSessionImplTest, Start) {
|
||||||
|
@@ -331,7 +331,9 @@ bool ProjectorControllerImpl::IsAnnotatorEnabled() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void ProjectorControllerImpl::OnNewScreencastPreconditionChanged() {
|
void ProjectorControllerImpl::OnNewScreencastPreconditionChanged() {
|
||||||
client_->OnNewScreencastPreconditionChanged(GetNewScreencastPrecondition());
|
// `client_` could be not available in unit tests.
|
||||||
|
if (client_)
|
||||||
|
client_->OnNewScreencastPreconditionChanged(GetNewScreencastPrecondition());
|
||||||
}
|
}
|
||||||
|
|
||||||
void ProjectorControllerImpl::SetProjectorUiControllerForTest(
|
void ProjectorControllerImpl::SetProjectorUiControllerForTest(
|
||||||
|
@@ -168,7 +168,7 @@ PendingSreencastManager::~PendingSreencastManager() {
|
|||||||
if (session_manager) {
|
if (session_manager) {
|
||||||
session_manager->RemoveObserver(this);
|
session_manager->RemoveObserver(this);
|
||||||
auto* drivefs_host = GetDriveFsHostForActiveProfile();
|
auto* drivefs_host = GetDriveFsHostForActiveProfile();
|
||||||
if (drivefs_host)
|
if (observed_drive_fs_host_ && drivefs_host)
|
||||||
drivefs_host->RemoveObserver(this);
|
drivefs_host->RemoveObserver(this);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -218,14 +218,19 @@ void PendingSreencastManager::OnSyncingStatusUpdate(
|
|||||||
void PendingSreencastManager::OnError(const drivefs::mojom::DriveError& error) {
|
void PendingSreencastManager::OnError(const drivefs::mojom::DriveError& error) {
|
||||||
}
|
}
|
||||||
|
|
||||||
void PendingSreencastManager::OnUserSessionStarted(bool is_primary_user) {
|
void PendingSreencastManager::OnUserProfileLoaded(const AccountId& account_id) {
|
||||||
|
if (observed_drive_fs_host_)
|
||||||
|
return;
|
||||||
|
|
||||||
auto* profile = ProfileManager::GetActiveUserProfile();
|
auto* profile = ProfileManager::GetActiveUserProfile();
|
||||||
if (!IsProjectorAllowedForProfile(profile))
|
if (!IsProjectorAllowedForProfile(profile))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
auto* drivefs_host = GetDriveFsHostForActiveProfile();
|
auto* drivefs_host = GetDriveFsHostForActiveProfile();
|
||||||
if (drivefs_host)
|
if (drivefs_host) {
|
||||||
GetDriveFsHostForActiveProfile()->AddObserver(this);
|
GetDriveFsHostForActiveProfile()->AddObserver(this);
|
||||||
|
observed_drive_fs_host_ = true;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const ash::PendingScreencastSet&
|
const ash::PendingScreencastSet&
|
||||||
|
@@ -45,7 +45,7 @@ class PendingSreencastManager : public session_manager::SessionManagerObserver,
|
|||||||
void OnError(const drivefs::mojom::DriveError& error) override;
|
void OnError(const drivefs::mojom::DriveError& error) override;
|
||||||
|
|
||||||
// session_manager::SessionManagerObserver:
|
// session_manager::SessionManagerObserver:
|
||||||
void OnUserSessionStarted(bool is_primary_user) override;
|
void OnUserProfileLoaded(const AccountId& account_id) override;
|
||||||
|
|
||||||
// Returns a list of pending screencast from `pending_screencast_cache_`.
|
// Returns a list of pending screencast from `pending_screencast_cache_`.
|
||||||
const ash::PendingScreencastSet& GetPendingScreencasts() const;
|
const ash::PendingScreencastSet& GetPendingScreencasts() const;
|
||||||
@@ -64,6 +64,9 @@ class PendingSreencastManager : public session_manager::SessionManagerObserver,
|
|||||||
// A blocking task runner for file IO operations.
|
// A blocking task runner for file IO operations.
|
||||||
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
|
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
|
||||||
|
|
||||||
|
// True if DriveFS host has been observed.
|
||||||
|
bool observed_drive_fs_host_ = false;
|
||||||
|
|
||||||
base::WeakPtrFactory<PendingSreencastManager> weak_ptr_factory_{this};
|
base::WeakPtrFactory<PendingSreencastManager> weak_ptr_factory_{this};
|
||||||
};
|
};
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user