Skip to content

fix(screen-time):pause tracking while session is locked - #3897

Closed
AdityaKr015 wants to merge 3 commits into
noctalia-dev:mainfrom
AdityaKr015:fix/screentime-session-on-locked
Closed

fix(screen-time):pause tracking while session is locked#3897
AdityaKr015 wants to merge 3 commits into
noctalia-dev:mainfrom
AdityaKr015:fix/screentime-session-on-locked

Conversation

@AdityaKr015

Copy link
Copy Markdown
Contributor

Summary

This PR fixes the screen time accumulating usage while the session is locked.

Motivation

Just to keep screen time not counting usage when session is locked or suspended.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring
  • Build / packaging

Related Issue

Closes #3882

Testing

  • Ran just format, just build, just test all were clean and passed.
  • Then killed noctalia pkill noctalia then started build debug ./build-debug/noctalia
  • I already had screen time setting on, for testing i opened VLC played a video then stop after 2 mins, then lock and suspend. After 5 mins login, then screen time of vlc was still 2 min, didn't increase on suspend.

Manual Coverage

  • Tested on Niri
  • Tested on Hyprland
  • Tested on Sway
  • Tested on another compositor:
  • Tested with different bar positions and density settings
  • Tested at different interface scaling values
  • Tested with multiple monitors

Checklist

  • This PR is ready for review.
  • I read and followed the relevant guidance in CONTRIBUTING.md.
  • I ran just format with clang-format v22+ installed.
  • I ran the relevant build or test commands.
  • I self-reviewed the changes.
  • I checked for new warnings or errors.
  • I will update end-user documentation after merge, or this PR does not change user-facing configuration or behavior.
  • I added or updated assets/translations/en.json, or this PR adds no new user-facing strings.
  • I did not edit non-English translation files unless this PR is explicitly for translation tooling, an import/export sync, or a maintainer-requested locale change.
  • I used the existing canonical names for config keys, IPC names, paths, and identifiers.

@ItsLemmy

Copy link
Copy Markdown
Collaborator

[P3] PR 3897 can restart the timer while locked

ScreenTimeService::setEnabled() / restartTicking()
(https://github.com/noctalia-dev/noctalia/pull/3897/files) starts m_tickTimer without checking
m_sessionLocked.

Trigger:

  1. Session locks; setSessionLocked(true) clears active accounting state and stops the timer.
  2. A config reload enables screen time while still locked.
  3. setEnabled(true) calls restartTicking().
  4. onFocusChange() correctly returns because the session is locked, but restartTicking() still
    starts the five-second timer.
  5. A repeated setSessionLocked(true) cannot repair this because its idempotence guard returns
    immediately.

No usage is currently misattributed because the active app key remains empty, but the service is
no longer paused and wakes every five seconds while locked. Guard restartTicking() with
m_sessionLocked; the unlock path already restarts it after clearing that state.

@ItsLemmy
ItsLemmy marked this pull request as draft August 12, 2026 13:33
@AdityaKr015

Copy link
Copy Markdown
Contributor Author

Done, tell me if there anything else is wrong too

@AdityaKr015
AdityaKr015 marked this pull request as ready for review August 15, 2026 04:55
@ItsLemmy

Copy link
Copy Markdown
Collaborator

[P2] Preserve the suspend pause when the lock clears

application_ui.cpp unlock callback /src/app/application_ui.cpp#L333-L340 unconditionally calls:

m_screenTimeService.setTrackingPaused(false);

m_trackingPaused now represents two independent reasons: session lock and system suspend. This transition is incorrect:

  1. PrepareForSleep(true) pauses tracking.
  2. Lock-before-suspend requests the session lock.
  3. The compositor aborts/finishes the lock before engagement—the following comment explicitly documents this path.
  4. The unlock callback clears m_trackingPaused and restarts accounting while logind still reports the system as preparing to sleep.
  5. Any remaining inhibitor delay before process freeze is credited to the active application, and the service enters suspend in the unpaused state.

The wake callback cannot restore the lost invariant: because the session is unlocked, it calls setTrackingPaused(false) again, which is a no-op.

Track the causes independently—e.g. sessionLocked and preparingForSleep—and derive pause state as sessionLocked || preparingForSleep. Alternatively, gate the unlock resume on an explicit sleep-preparation flag.

Previous finding

The earlier timer-restart finding is fixed correctly: restartTicking() now returns while paused, including when configuration enables screen time during a lock.

Non-blocking correction

In PrepareForSleep(false):

if (!m_lockScreen.isSessionLocked()) {
 // Still locked after wake; the unlock hook resumes tracking.

The comment is inverted. This branch runs when the session is not locked.

@ItsLemmy
ItsLemmy marked this pull request as draft August 15, 2026 14:06
@AdityaKr015

Copy link
Copy Markdown
Contributor Author

Done. pushed a fix. Sorry for the wrong comment, used m_preparingForSleep for better logic. CI test is passed so it should be good now.

@AdityaKr015
AdityaKr015 marked this pull request as ready for review August 15, 2026 15:52
@AdityaKr015

AdityaKr015 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Closing it. Will push for suspend since it is missing.

@AdityaKr015
AdityaKr015 deleted the fix/screentime-session-on-locked branch August 15, 2026 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Screen time keeps counting while the session is locked

2 participants