Commit c6955546 authored by Wenzhao Zang's avatar Wenzhao Zang Committed by Commit Bot

cros: Allow demo mode screensaver to always stay on top

The screensaver in demo mode should be full-screen and keep staying on
top, otherwise user experience would be really bad. If these two things
can't be achieved together, I think we should give up using Chrome App
for screensaver.

The demo mode screensaver app implements its own mechanism to close the
window when there's user interaction. If some security issues arise,
it'll be the demo mode team's responsibility to fix it.

TBR=rdevlin.cronin@chromium.org

Bug: 870851
Change-Id: I9773ec9dc99dd6b01c5202bcb5678cd0a59a544b
Reviewed-on: https://chromium-review.googlesource.com/1214602
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: default avatarSergey Volk <servolk@chromium.org>
Reviewed-by: default avatarMichael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592493}
parent 0186b248
......@@ -10,9 +10,9 @@
#include "base/files/file_util.h"
#include "base/task/post_task.h"
#include "chrome/browser/chromeos/extensions/install_limiter_factory.h"
#include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_source.h"
#include "extensions/browser/extensions_browser_client.h"
#include "extensions/browser/notification_types.h"
namespace {
......@@ -57,7 +57,7 @@ bool InstallLimiter::ShouldDeferInstall(int64_t app_size,
const std::string& app_id) {
constexpr int64_t kBigAppSizeThreshold = 1048576; // 1MB in bytes
return app_size > kBigAppSizeThreshold &&
!chromeos::DemoSession::IsScreensaverInDemoMode(app_id);
!ExtensionsBrowserClient::Get()->IsScreensaverInDemoMode(app_id);
}
InstallLimiter::InstallLimiter() : disabled_for_test_(false) {
......
......@@ -7,6 +7,9 @@
#include "base/macros.h"
#include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
#include "chrome/browser/chromeos/settings/stub_install_attributes.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/fake_image_loader_client.h"
#include "components/session_manager/core/session_manager.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "extensions/common/constants.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -26,9 +29,28 @@ class InstallLimiterTest : public testing::Test {
InstallLimiterTest() = default;
~InstallLimiterTest() override = default;
void SetUp() override {
auto image_loader_client =
std::make_unique<chromeos::FakeImageLoaderClient>();
image_loader_client_ = image_loader_client.get();
chromeos::DBusThreadManager::GetSetterForTesting()->SetImageLoaderClient(
std::move(image_loader_client));
session_manager_ = std::make_unique<session_manager::SessionManager>();
}
void TearDown() override {
chromeos::DemoSession::ShutDownIfInitialized();
chromeos::DemoSession::ResetDemoConfigForTesting();
image_loader_client_ = nullptr;
chromeos::DBusThreadManager::Shutdown();
}
private:
content::TestBrowserThreadBundle thread_bundle_;
chromeos::ScopedStubInstallAttributes test_install_attributes_;
std::unique_ptr<session_manager::SessionManager> session_manager_;
// Points to the image loader client passed to the test DBusTestManager.
chromeos::FakeImageLoaderClient* image_loader_client_ = nullptr;
DISALLOW_COPY_AND_ASSIGN(InstallLimiterTest);
};
......@@ -48,6 +70,7 @@ TEST_F(InstallLimiterTest, ShouldDeferInstall) {
// for the screensaver should be deferred.
chromeos::DemoSession::SetDemoConfigForTesting(
chromeos::DemoSession::DemoModeConfig::kOnline);
chromeos::DemoSession::StartIfInDemoMode();
EXPECT_FALSE(InstallLimiter::ShouldDeferInstall(
kLargeExtensionSize, extension_misc::kScreensaverAppId));
EXPECT_TRUE(InstallLimiter::ShouldDeferInstall(kLargeExtensionSize,
......
......@@ -66,10 +66,6 @@ constexpr char kHighlightsAppPath[] = "chrome_apps/highlights";
// contains sample photos.
constexpr char kPhotosPath[] = "media/photos";
constexpr char kDefaultHighlightsAppId[] = "lpmakjfjcconjeehbidjclhdlpjmfjjj";
constexpr char kEveHighlightsAppId[] = "iggildboghmjpbjcpmobahnkmoefkike";
bool IsDemoModeOfflineEnrolled() {
DCHECK(DemoSession::IsDeviceInDemoMode());
return DemoSession::GetDemoConfig() == DemoSession::DemoModeConfig::kOffline;
......@@ -113,8 +109,8 @@ std::string GetBoardName() {
std::string GetHighlightsAppId() {
if (GetBoardName() == "eve")
return kEveHighlightsAppId;
return kDefaultHighlightsAppId;
return extension_misc::kHighlightsAlt1AppId;
return extension_misc::kHighlightsAppId;
}
} // namespace
......@@ -245,11 +241,6 @@ DemoSession* DemoSession::Get() {
return g_demo_session;
}
// static
bool DemoSession::IsScreensaverInDemoMode(const std::string& app_id) {
return app_id == extension_misc::kScreensaverAppId && IsDeviceInDemoMode();
}
void DemoSession::EnsureOfflineResourcesLoaded(
base::OnceClosure load_callback) {
if (offline_resources_loaded_) {
......
......@@ -89,10 +89,6 @@ class DemoSession : public session_manager::SessionManagerObserver,
// StartIfInDemoMode() or PreloadOfflineResourcesIfInDemoMode()).
static DemoSession* Get();
// Returns whether |app_id| matches the screensaver app and the device is in
// demo mode.
static bool IsScreensaverInDemoMode(const std::string& app_id);
// Ensures that the load of offline demo session resources is requested.
// |load_callback| will be run once the offline resource load finishes.
void EnsureOfflineResourcesLoaded(base::OnceClosure load_callback);
......
......@@ -305,6 +305,14 @@ bool ChromeExtensionsBrowserClient::IsInDemoMode() {
#endif
}
bool ChromeExtensionsBrowserClient::IsScreensaverInDemoMode(
const std::string& app_id) {
#if defined(OS_CHROMEOS)
return app_id == extension_misc::kScreensaverAppId && IsInDemoMode();
#endif
return false;
}
bool ChromeExtensionsBrowserClient::IsRunningInForcedAppMode() {
return chrome::IsRunningInForcedAppMode();
}
......
......@@ -101,6 +101,7 @@ class ChromeExtensionsBrowserClient : public ExtensionsBrowserClient {
bool DidVersionUpdate(content::BrowserContext* context) override;
void PermitExternalProtocolHandler() override;
bool IsInDemoMode() override;
bool IsScreensaverInDemoMode(const std::string& app_id) override;
bool IsRunningInForcedAppMode() override;
bool IsAppModeForcedForApp(const ExtensionId& extension_id) override;
bool IsLoggedInAsPublicAccount() override;
......
......@@ -178,6 +178,11 @@ bool CastExtensionsBrowserClient::IsInDemoMode() {
return false;
}
bool CastExtensionsBrowserClient::IsScreensaverInDemoMode(
const std::string& app_id) {
return false;
}
bool CastExtensionsBrowserClient::IsRunningInForcedAppMode() {
return false;
}
......
......@@ -84,6 +84,7 @@ class CastExtensionsBrowserClient : public ExtensionsBrowserClient {
bool DidVersionUpdate(content::BrowserContext* context) override;
void PermitExternalProtocolHandler() override;
bool IsInDemoMode() override;
bool IsScreensaverInDemoMode(const std::string& app_id) override;
bool IsRunningInForcedAppMode() override;
bool IsAppModeForcedForApp(const ExtensionId& id) override;
bool IsLoggedInAsPublicAccount() override;
......
......@@ -282,8 +282,11 @@ void AppWindow::Init(const GURL& url,
// Windows cannot be always-on-top in fullscreen mode for security reasons.
cached_always_on_top_ = new_params.always_on_top;
if (new_params.state == ui::SHOW_STATE_FULLSCREEN)
if (new_params.state == ui::SHOW_STATE_FULLSCREEN &&
!ExtensionsBrowserClient::Get()->IsScreensaverInDemoMode(
extension_id())) {
new_params.always_on_top = false;
}
requested_alpha_enabled_ = new_params.alpha_enabled;
is_ime_window_ = params.is_ime_window;
......@@ -731,8 +734,12 @@ void AppWindow::SetAlwaysOnTop(bool always_on_top) {
// As a security measure, do not allow fullscreen windows or windows that
// overlap the taskbar to be on top. The property will be applied when the
// window exits fullscreen and moves away from the taskbar.
if (!IsFullscreen() && !IntersectsWithTaskbar())
if ((!IsFullscreen() ||
ExtensionsBrowserClient::Get()->IsScreensaverInDemoMode(
extension_id())) &&
!IntersectsWithTaskbar()) {
native_app_window_->SetAlwaysOnTop(always_on_top);
}
OnNativeWindowChanged();
}
......
......@@ -227,6 +227,10 @@ class ExtensionsBrowserClient {
// Return true if the device is enrolled in Demo Mode.
virtual bool IsInDemoMode() = 0;
// Return true if |app_id| matches the screensaver and the device is enrolled
// in Demo Mode.
virtual bool IsScreensaverInDemoMode(const std::string& app_id) = 0;
// Return true if the system is run in forced app mode.
virtual bool IsRunningInForcedAppMode() = 0;
......
......@@ -193,6 +193,11 @@ bool TestExtensionsBrowserClient::IsInDemoMode() {
return false;
}
bool TestExtensionsBrowserClient::IsScreensaverInDemoMode(
const std::string& app_id) {
return false;
}
bool TestExtensionsBrowserClient::IsRunningInForcedAppMode() { return false; }
bool TestExtensionsBrowserClient::IsAppModeForcedForApp(
......
......@@ -118,6 +118,7 @@ class TestExtensionsBrowserClient : public ExtensionsBrowserClient {
bool DidVersionUpdate(content::BrowserContext* context) override;
void PermitExternalProtocolHandler() override;
bool IsInDemoMode() override;
bool IsScreensaverInDemoMode(const std::string& app_id) override;
bool IsRunningInForcedAppMode() override;
bool IsAppModeForcedForApp(const ExtensionId& extension_id) override;
bool IsLoggedInAsPublicAccount() override;
......
......@@ -197,6 +197,11 @@ bool ShellExtensionsBrowserClient::IsInDemoMode() {
return false;
}
bool ShellExtensionsBrowserClient::IsScreensaverInDemoMode(
const std::string& app_id) {
return false;
}
bool ShellExtensionsBrowserClient::IsRunningInForcedAppMode() {
return false;
}
......
......@@ -89,6 +89,7 @@ class ShellExtensionsBrowserClient : public ExtensionsBrowserClient {
bool DidVersionUpdate(content::BrowserContext* context) override;
void PermitExternalProtocolHandler() override;
bool IsInDemoMode() override;
bool IsScreensaverInDemoMode(const std::string& app_id) override;
bool IsRunningInForcedAppMode() override;
bool IsAppModeForcedForApp(const ExtensionId& extension_id) override;
bool IsLoggedInAsPublicAccount() override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment