Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --idle-timeout-when-locked for idle timeout when session is locked #3546

Merged
merged 5 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 66 additions & 4 deletions src/server/shell/basic_idle_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "basic_idle_handler.h"
#include "mir/fatal.h"
#include "mir/scene/idle_hub.h"
#include "mir/scene/session_lock.h"
#include "mir/graphics/renderable.h"
#include "mir/renderer/sw/pixel_source.h"
#include "mir/input/scene.h"
Expand Down Expand Up @@ -168,22 +169,72 @@ struct PowerModeSetter : ms::IdleStateObserver
MirPowerMode const power_mode;
msh::IdleHandlerObserver& observer;
};

struct TimeoutRestorer : ms::IdleStateObserver
{
explicit TimeoutRestorer(msh::BasicIdleHandler* idle_handler, mir::time::Duration timeout)
: idle_handler{idle_handler}, timeout{timeout}
{
}

void active() override
{
if (!is_active && !restored)
{
restored = true;
idle_handler->set_display_off_timeout(timeout);
}
is_active = true;
}

void idle() override
{
is_active = false;
}

private:
mir::shell::BasicIdleHandler* const idle_handler;
mir::time::Duration const timeout;

bool is_active{true};
bool restored{false};
};
}

class msh::BasicIdleHandler::SessionLockListener : public ms::SessionLockObserver
{
public:
explicit SessionLockListener(BasicIdleHandler* idle_handler) : idle_handler{idle_handler} {};

private:
void on_lock() override
{
idle_handler->set_display_off_timeout(time::Duration{});
};
void on_unlock() override {};

BasicIdleHandler* const idle_handler;
};

msh::BasicIdleHandler::BasicIdleHandler(
std::shared_ptr<ms::IdleHub> const& idle_hub,
std::shared_ptr<input::Scene> const& input_scene,
std::shared_ptr<graphics::GraphicBufferAllocator> const& allocator,
std::shared_ptr<msh::DisplayConfigurationController> const& display_config_controller)
std::shared_ptr<msh::DisplayConfigurationController> const& display_config_controller,
std::shared_ptr<ms::SessionLock> const& session_lock)
: idle_hub{idle_hub},
input_scene{input_scene},
allocator{allocator},
display_config_controller{display_config_controller}
display_config_controller{display_config_controller},
session_lock{session_lock},
session_lock_monitor{std::make_shared<SessionLockListener>(this)}
{
session_lock->register_interest(session_lock_monitor);
}

msh::BasicIdleHandler::~BasicIdleHandler()
{
session_lock->unregister_interest(*session_lock_monitor);
std::lock_guard lock{mutex};
clear_observers(lock);
}
Expand All @@ -195,14 +246,15 @@ void msh::BasicIdleHandler::set_display_off_timeout(std::optional<time::Duration
{
return;
}
auto const previous_off_timeout{current_off_timeout};
current_off_timeout = timeout;
clear_observers(lock);
if (timeout)
{
auto const off_timeout = timeout.value();
if (off_timeout <= time::Duration{})
if (off_timeout < time::Duration{})
{
fatal_error("BasicIdleHandler given invalid timeout %d, should be >0", off_timeout.count());
fatal_error("BasicIdleHandler given invalid timeout %d, should be >=0", off_timeout.count());
}
if (off_timeout >= dim_time_before_off * 2)
{
Expand All @@ -215,6 +267,16 @@ void msh::BasicIdleHandler::set_display_off_timeout(std::optional<time::Duration
display_config_controller, mir_power_mode_off, multiplexer);
observers.push_back(power_setter);
idle_hub->register_interest(power_setter, off_timeout);
if (off_timeout == time::Duration{} && previous_off_timeout)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is "just me", but I find that off_timeout == time::Duration{0} better expresses a comparison to zero. (I know comparing to "default constructed" has the same meaning to the compiler but it takes more thought to read)

{
// A zero timeout cannot be set through configuration options. However,
// it can be set internally to immediately turn off the display when
// the session is locked. In this case, the original timeout from the
// config options will be restored the next time Mir wakes from idle.
auto const timeout_restorer = std::make_shared<TimeoutRestorer>(this, previous_off_timeout.value());
observers.push_back(timeout_restorer);
idle_hub->register_interest(timeout_restorer, off_timeout);
}
}
}

Expand Down
8 changes: 7 additions & 1 deletion src/server/shell/basic_idle_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ namespace scene
{
class IdleHub;
class IdleStateObserver;
class SessionLock;
}
namespace shell
{
Expand All @@ -51,7 +52,8 @@ class BasicIdleHandler : public IdleHandler
std::shared_ptr<scene::IdleHub> const& idle_hub,
std::shared_ptr<input::Scene> const& input_scene,
std::shared_ptr<graphics::GraphicBufferAllocator> const& allocator,
std::shared_ptr<shell::DisplayConfigurationController> const& display_config_controller);
std::shared_ptr<shell::DisplayConfigurationController> const& display_config_controller,
std::shared_ptr<scene::SessionLock> const& session_lock);

~BasicIdleHandler();

Expand All @@ -70,12 +72,16 @@ class BasicIdleHandler : public IdleHandler
void unregister_interest(IdleHandlerObserver const&) override;

private:
class SessionLockListener;

void clear_observers(ProofOfMutexLock const&);

std::shared_ptr<scene::IdleHub> const idle_hub;
std::shared_ptr<input::Scene> const input_scene;
std::shared_ptr<graphics::GraphicBufferAllocator> const allocator;
std::shared_ptr<shell::DisplayConfigurationController> const display_config_controller;
std::shared_ptr<scene::SessionLock> const session_lock;
std::shared_ptr<SessionLockListener> const session_lock_monitor;

std::mutex mutex;
std::optional<time::Duration> current_off_timeout;
Expand Down
3 changes: 2 additions & 1 deletion src/server/shell/default_configuration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ auto mir::DefaultServerConfiguration::the_idle_handler() -> std::shared_ptr<msh:
the_idle_hub(),
the_input_scene(),
the_buffer_allocator(),
the_display_configuration_controller());
the_display_configuration_controller(),
the_session_lock());

auto options = the_options();
int const idle_timeout_seconds = options->get<int>(options::idle_timeout_opt);
Expand Down
8 changes: 6 additions & 2 deletions tests/unit-tests/shell/test_basic_idle_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "mir/test/doubles/mock_idle_hub.h"
#include "mir/test/doubles/stub_input_scene.h"
#include "mir/test/doubles/stub_buffer_allocator.h"
#include "mir/test/doubles/stub_session_lock.h"
#include "mir/shell/display_configuration_controller.h"
#include "mir/test/fake_shared.h"

Expand Down Expand Up @@ -52,11 +53,13 @@ struct BasicIdleHandler: Test
mtd::StubInputScene input_scene;
mtd::StubBufferAllocator allocator;
NiceMock<MockDisplayConfigurationController> display;
mtd::StubSessionLock session_lock;
msh::BasicIdleHandler handler{
mt::fake_shared(idle_hub),
mt::fake_shared(input_scene),
mt::fake_shared(allocator),
mt::fake_shared(display)};
mt::fake_shared(display),
mt::fake_shared(session_lock)};
std::map<mir::time::Duration, std::weak_ptr<ms::IdleStateObserver>> observers;

void SetUp()
Expand Down Expand Up @@ -95,7 +98,8 @@ TEST_F(BasicIdleHandler, does_not_register_observers_by_default)
mt::fake_shared(idle_hub),
mt::fake_shared(input_scene),
mt::fake_shared(allocator),
mt::fake_shared(display)};
mt::fake_shared(display),
mt::fake_shared(session_lock)};
}

TEST_F(BasicIdleHandler, turns_display_off_when_idle)
Expand Down
Loading