docs: trim the #228 comments to the load-bearing facts
The rollover scheduler and receiver retold the bug story that already lives in the commit messages and the issue. Keep what a reader of the code needs — why the alarm exists rather than DATE_CHANGED, why it is inexact, and why the day boundary is the real start of day — and drop the rest.
This commit is contained in:
11
app/proguard-rules.pro
vendored
11
app/proguard-rules.pro
vendored
@@ -51,11 +51,8 @@
|
|||||||
# obfuscated name and orphan the stored mapping.
|
# obfuscated name and orphan the stored mapping.
|
||||||
-keep class * extends androidx.glance.appwidget.GlanceAppWidget
|
-keep class * extends androidx.glance.appwidget.GlanceAppWidget
|
||||||
|
|
||||||
# Belt and braces one level up: MonthWidgetReceiver and AgendaWidgetReceiver are
|
# Belt and braces one level up: the two receivers are nearly as alike, and the
|
||||||
# nearly as alike (same supertype, same overrides, only a differing property
|
# provider map is keyed off the receiver component too. Redundant today (AGP's
|
||||||
# initializer), and Glance's provider map is keyed off the receiver component
|
# manifest-derived rules cover them), but #89 cost a release to diagnose and the
|
||||||
# too. AGP's manifest-derived keep rules already cover them, and the rule above
|
# guarantee should not rest on a component staying in the manifest.
|
||||||
# keeps the two widgets distinct enough that the receivers' constructors differ —
|
|
||||||
# so this is redundant today. It is here because #89 cost a release to diagnose
|
|
||||||
# and the guarantee should not rest on a component staying in the manifest.
|
|
||||||
-keep class * extends androidx.glance.appwidget.GlanceAppWidgetReceiver
|
-keep class * extends androidx.glance.appwidget.GlanceAppWidgetReceiver
|
||||||
|
|||||||
@@ -330,13 +330,12 @@
|
|||||||
</receiver>
|
</receiver>
|
||||||
|
|
||||||
<!-- Keeps both widgets fresh: the calendar provider broadcasts
|
<!-- Keeps both widgets fresh: the calendar provider broadcasts
|
||||||
PROVIDER_CHANGED on any data change (our writes and external sync),
|
PROVIDER_CHANGED on any data change (our writes and external sync).
|
||||||
and the day boundary arrives as the app's own ROLLOVER alarm (#228),
|
The day boundary arrives as the app's own ROLLOVER alarm (#228), by
|
||||||
delivered by an explicit PendingIntent so it needs no filter here.
|
explicit PendingIntent, so it needs no filter here; DATE_CHANGED is
|
||||||
DATE_CHANGED is kept as a free extra only — it is not an exempted
|
a free extra only, since Android 8+ withholds it from manifest
|
||||||
implicit broadcast, so a manifest-declared receiver is not given it
|
receivers. The four below re-arm that alarm: TIME_SET /
|
||||||
on Android 8+. TIME_SET / TIMEZONE_CHANGED move the day boundary,
|
TIMEZONE_CHANGED move the boundary, boot / package-replace wipe it.
|
||||||
and boot / package-replace wipe the alarm, so all four re-arm it.
|
|
||||||
Exported: the system broadcasts arrive from outside the app. -->
|
Exported: the system broadcasts arrive from outside the app. -->
|
||||||
<receiver
|
<receiver
|
||||||
android:name=".widget.WidgetUpdateReceiver"
|
android:name=".widget.WidgetUpdateReceiver"
|
||||||
|
|||||||
@@ -50,11 +50,9 @@ class CalendulaApp : Application() {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Re-arm the widgets' midnight rollover from whatever is actually placed
|
* Re-arm the widgets' midnight rollover from whatever is actually placed
|
||||||
* (#228). Idempotent, and it covers the cases no broadcast reaches — an
|
* (#228). Idempotent, and it covers what no broadcast reaches — an alarm
|
||||||
* install upgrading into the fix, or an alarm dropped by a force-stop, is
|
* dropped by a force-stop is armed again the next time the app is opened.
|
||||||
* armed again the next time the app is opened. Off the main thread because
|
* Off the main thread: a handful of binder calls on every process start.
|
||||||
* it makes a handful of binder calls and every process start runs it,
|
|
||||||
* including ones a worker or a receiver triggered.
|
|
||||||
*/
|
*/
|
||||||
private fun reconcileWidgetRollover() {
|
private fun reconcileWidgetRollover() {
|
||||||
CoroutineScope(SupervisorJob() + Dispatchers.Default).launch {
|
CoroutineScope(SupervisorJob() + Dispatchers.Default).launch {
|
||||||
|
|||||||
@@ -23,26 +23,21 @@ import kotlin.time.Instant
|
|||||||
* Holds the app's own wake-up for the next local midnight, so the home-screen
|
* Holds the app's own wake-up for the next local midnight, so the home-screen
|
||||||
* widgets roll "today" over on the day boundary (#228).
|
* widgets roll "today" over on the day boundary (#228).
|
||||||
*
|
*
|
||||||
* The widgets used to lean on `ACTION_DATE_CHANGED`, but that broadcast is not
|
* The widgets used to lean on `ACTION_DATE_CHANGED`, which is not an exempted
|
||||||
* on the implicit-broadcast exemption list, so a manifest-declared receiver has
|
* implicit broadcast — a manifest-declared receiver has not been given it since
|
||||||
* never been given it since Android 8 — leaving only `updatePeriodMillis`, which
|
* Android 8, leaving only the throttled `updatePeriodMillis`.
|
||||||
* the system defers in doze and OEM skins throttle harder still. The result was
|
|
||||||
* yesterday staying highlighted (and the agenda's past-event dimming staying
|
|
||||||
* anchored to yesterday) until something else forced a redraw.
|
|
||||||
*
|
*
|
||||||
* Exactly one alarm exists at a time and every firing re-arms the next one, the
|
* Exactly one alarm exists at a time and every firing re-arms the next, the same
|
||||||
* same shape as [de.jeanlucmakiola.calendula.data.reminders.ReminderAlarmScheduler].
|
* shape as [de.jeanlucmakiola.calendula.data.reminders.ReminderAlarmScheduler].
|
||||||
* It is deliberately **inexact**: `setAndAllowWhileIdle` needs no permission and
|
* Deliberately **inexact**: `setAndAllowWhileIdle` needs no permission and
|
||||||
* survives doze (which plain `set` does not), and a rollover that lands a few
|
* survives doze (plain `set` does not), a rollover a few minutes late is
|
||||||
* minutes late is invisible on a sleeping screen. Exact alarms stay reserved for
|
* invisible on a sleeping screen, and exact alarms stay reserved for snooze.
|
||||||
* reminder snooze.
|
|
||||||
*/
|
*/
|
||||||
object WidgetRolloverScheduler {
|
object WidgetRolloverScheduler {
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Fire just *after* midnight, never exactly on it. An alarm delivered a few
|
* Fire just *after* midnight: an alarm delivered a few milliseconds early
|
||||||
* milliseconds early would still read the old date and re-arm for an instant
|
* would still read the old date and re-arm for an instant later.
|
||||||
* later; the offset makes "the day has changed" unambiguous.
|
|
||||||
*/
|
*/
|
||||||
internal val ROLLOVER_SLACK = 5.seconds
|
internal val ROLLOVER_SLACK = 5.seconds
|
||||||
|
|
||||||
@@ -68,16 +63,14 @@ object WidgetRolloverScheduler {
|
|||||||
/**
|
/**
|
||||||
* The instant just after the next local midnight following [now] in [zone].
|
* The instant just after the next local midnight following [now] in [zone].
|
||||||
*
|
*
|
||||||
* Uses the *actual* start of the day rather than 00:00, so it stays correct
|
* The *actual* start of day, not 00:00, so it holds where a DST jump means
|
||||||
* where a DST jump means midnight never happens (Havana springs from 00:00 to
|
* midnight never happens (Havana) and where a date-line move skips a whole
|
||||||
* 01:00) and where a whole local date is skipped by a date-line move (Apia
|
* local date (Apia, December 2011) — the loop walks on to the next real day.
|
||||||
* had no 30 December 2011) — the loop then walks on to the next real day.
|
|
||||||
*
|
*
|
||||||
* The mirror case, a zone that rewinds *across* midnight so the day starts
|
* The mirror case — a zone rewinding *across* midnight, so the day starts
|
||||||
* twice, resolves to the earlier start; the widget would then run an hour
|
* twice — resolves to the earlier start and runs an hour ahead of the clock.
|
||||||
* ahead of the clock. No entry in the current tz database does that (Brazil,
|
* No live tz entry does that (Brazil dropped DST in 2019) and
|
||||||
* which used to, dropped DST in 2019), and `updatePeriodMillis` covers it,
|
* `updatePeriodMillis` covers it, so it isn't worth state to detect.
|
||||||
* so it is not worth carrying state to detect.
|
|
||||||
*/
|
*/
|
||||||
fun nextRolloverAt(now: Instant, zone: TimeZone): Instant {
|
fun nextRolloverAt(now: Instant, zone: TimeZone): Instant {
|
||||||
val date = now.toLocalDateTime(zone).date
|
val date = now.toLocalDateTime(zone).date
|
||||||
|
|||||||
@@ -19,38 +19,31 @@ import kotlinx.coroutines.launch
|
|||||||
* - [ACTION_ROLLOVER], the app's own alarm from [WidgetRolloverScheduler] —
|
* - [ACTION_ROLLOVER], the app's own alarm from [WidgetRolloverScheduler] —
|
||||||
* the day boundary, so "today" highlighting and the agenda's past-event
|
* the day boundary, so "today" highlighting and the agenda's past-event
|
||||||
* dimming move on (#228).
|
* dimming move on (#228).
|
||||||
* - `TIME_SET` / `TIMEZONE_CHANGED` — a clock or zone change moves the day
|
* - `TIME_SET` / `TIMEZONE_CHANGED` — the day boundary moved, so redraw *and*
|
||||||
* boundary relative to the armed alarm, so both redraw *and* re-arm.
|
* re-arm.
|
||||||
* - `BOOT_COMPLETED` / `MY_PACKAGE_REPLACED` — both wipe pending alarms. The
|
* - `BOOT_COMPLETED` / `MY_PACKAGE_REPLACED` — both wipe pending alarms; the
|
||||||
* package-replaced one is also what arms existing installs that upgrade into
|
* latter is also what arms installs upgrading into the fix.
|
||||||
* the fix without re-adding their widget.
|
|
||||||
*
|
*
|
||||||
* `DATE_CHANGED` is still in the manifest filter as a free extra, but nothing
|
* `DATE_CHANGED` is a free extra in the filter that nothing depends on — see
|
||||||
* depends on it: it is not an exempted implicit broadcast, so a manifest-declared
|
* [WidgetRolloverScheduler]. The backstops are `updatePeriodMillis` in the
|
||||||
* receiver has not actually been given it since Android 8. The widgets also carry
|
* provider XML and the month widget's refresh button.
|
||||||
* an `updatePeriodMillis` backstop in their provider XML, and the month widget's
|
|
||||||
* refresh button forces an immediate redraw.
|
|
||||||
*
|
*
|
||||||
* Exported for the system broadcasts; an extra redraw triggered by another app
|
* Exported for the system broadcasts; an extra redraw from another app is
|
||||||
* is harmless.
|
* harmless.
|
||||||
*/
|
*/
|
||||||
class WidgetUpdateReceiver : BroadcastReceiver() {
|
class WidgetUpdateReceiver : BroadcastReceiver() {
|
||||||
override fun onReceive(context: Context, intent: Intent) {
|
override fun onReceive(context: Context, intent: Intent) {
|
||||||
// The receiver has to stay exported for the system broadcasts, so an
|
// Exported, so anything can reach it with an explicit intent. Nothing
|
||||||
// explicit intent can reach it with anything in it. Nothing here reads
|
// here crosses a trust boundary, but narrowing to the actions we asked
|
||||||
// the intent's data and nothing crosses a trust boundary, but narrowing
|
// for keeps a stray broadcast from costing two wide provider reads.
|
||||||
// to the actions we actually asked for keeps a stray broadcast from
|
|
||||||
// costing two wide provider reads.
|
|
||||||
if (intent.action !in HANDLED_ACTIONS) return
|
if (intent.action !in HANDLED_ACTIONS) return
|
||||||
val appContext = context.applicationContext
|
val appContext = context.applicationContext
|
||||||
// Re-arm first: whatever happens to the redraw, the next day boundary is
|
// Re-arm first, so the next day boundary is covered whatever the redraw
|
||||||
// covered. Boot and package-replace dropped the alarm outright; a
|
// does. Every handled action either dropped, consumed or invalidated it.
|
||||||
// rollover just consumed it; a clock change invalidated it.
|
|
||||||
WidgetRolloverScheduler.sync(appContext)
|
WidgetRolloverScheduler.sync(appContext)
|
||||||
// Boot and package-replace only cost us the alarm. The host sends
|
// The host sends APPWIDGET_UPDATE after both of these anyway, so
|
||||||
// APPWIDGET_UPDATE after both anyway, so redrawing here would just repeat
|
// redrawing here would only repeat the work in a cold process, at the
|
||||||
// two wide provider reads and two RemoteViews serialisations in a cold
|
// moment the device is most contended.
|
||||||
// process, at the moment the device is most contended.
|
|
||||||
if (intent.action in REARM_ONLY_ACTIONS) return
|
if (intent.action in REARM_ONLY_ACTIONS) return
|
||||||
val pending = goAsync()
|
val pending = goAsync()
|
||||||
// Calendar data may have changed (sync / our own write) — drop the cached
|
// Calendar data may have changed (sync / our own write) — drop the cached
|
||||||
|
|||||||
@@ -21,9 +21,8 @@ class AgendaWidgetReceiver : GlanceAppWidgetReceiver() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Last agenda widget removed. [WidgetRolloverScheduler.sync] only cancels the
|
* Last agenda widget removed. [WidgetRolloverScheduler.sync] cancels only if
|
||||||
* alarm if no month widget is left either, so removing one kind never stops
|
* no month widget is left either.
|
||||||
* the other from rolling over.
|
|
||||||
*/
|
*/
|
||||||
override fun onDisabled(context: Context) {
|
override fun onDisabled(context: Context) {
|
||||||
super.onDisabled(context)
|
super.onDisabled(context)
|
||||||
|
|||||||
@@ -153,13 +153,11 @@ class ShiftMonthAction : ActionCallback {
|
|||||||
updateAppWidgetState(context, glanceId) { prefs ->
|
updateAppWidgetState(context, glanceId) { prefs ->
|
||||||
val cur = prefs[MONTH_INDEX_KEY] ?: currentMonthIndex(systemZone())
|
val cur = prefs[MONTH_INDEX_KEY] ?: currentMonthIndex(systemZone())
|
||||||
val next = cur + delta
|
val next = cur + delta
|
||||||
// Landing back on the current month clears the key rather than
|
// Landing back on the current month clears the key, so the widget
|
||||||
// storing today's index, so the widget goes back to *following* the
|
// goes back to *following* the date rather than being pinned to
|
||||||
// date instead of being pinned to the month that happened to be
|
// whichever month was current at the tap. Paging out and back is the
|
||||||
// current when it was tapped. Paging forward and back is the very
|
// workaround #228's reporter used, and pinning it there would have
|
||||||
// workaround #228's reporter used to force a redraw; storing the
|
// stuck them on that month once it stopped being the current one.
|
||||||
// index there would have left them stuck on that month for good once
|
|
||||||
// it stopped being the current one.
|
|
||||||
if (next == currentMonthIndex(systemZone())) {
|
if (next == currentMonthIndex(systemZone())) {
|
||||||
prefs.remove(MONTH_INDEX_KEY)
|
prefs.remove(MONTH_INDEX_KEY)
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -20,9 +20,8 @@ class MonthWidgetReceiver : GlanceAppWidgetReceiver() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Last month widget removed. [WidgetRolloverScheduler.sync] only cancels the
|
* Last month widget removed. [WidgetRolloverScheduler.sync] cancels only if
|
||||||
* alarm if no agenda widget is left either, so removing one kind never stops
|
* no agenda widget is left either.
|
||||||
* the other from rolling over.
|
|
||||||
*/
|
*/
|
||||||
override fun onDisabled(context: Context) {
|
override fun onDisabled(context: Context) {
|
||||||
super.onDisabled(context)
|
super.onDisabled(context)
|
||||||
@@ -31,11 +30,10 @@ class MonthWidgetReceiver : GlanceAppWidgetReceiver() {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* The `updatePeriodMillis` backstop is the one wake-up the *system* still
|
* The `updatePeriodMillis` backstop is the one wake-up the *system* still
|
||||||
* owns, so it doubles as the rollover alarm's self-heal: anything that drops
|
* owns, so it doubles as the alarm's self-heal: anything that drops a
|
||||||
* a pending alarm without a broadcast — a force-stop, a battery-restricted
|
* pending alarm without a broadcast (force-stop, battery restriction, an OEM
|
||||||
* transition, an OEM freeze — is repaired here rather than waiting for the
|
* freeze) is repaired here rather than on the next app open. Re-arming
|
||||||
* app to be opened. Re-arming closer to midnight also narrows the inexact
|
* closer to midnight also narrows the inexact delivery window.
|
||||||
* alarm's delivery window, which scales with how far out it was set.
|
|
||||||
*/
|
*/
|
||||||
override fun onUpdate(
|
override fun onUpdate(
|
||||||
context: Context,
|
context: Context,
|
||||||
|
|||||||
@@ -44,8 +44,8 @@ class WidgetRolloverSchedulerTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `at midnight exactly the target is the next day, never the current instant`() {
|
fun `at midnight exactly the target is the next day, never the current instant`() {
|
||||||
// The alarm has just fired and is re-arming: it must move a whole day on,
|
// Re-arming after a firing must move a whole day on, or the widget wakes
|
||||||
// otherwise the widget would wake itself in a tight loop.
|
// itself in a tight loop.
|
||||||
val now = at("2026-08-28T00:00:00", berlin)
|
val now = at("2026-08-28T00:00:00", berlin)
|
||||||
val next = WidgetRolloverScheduler.nextRolloverAt(now, berlin)
|
val next = WidgetRolloverScheduler.nextRolloverAt(now, berlin)
|
||||||
assertThat(next).isEqualTo(at("2026-08-29T00:00:05", berlin))
|
assertThat(next).isEqualTo(at("2026-08-29T00:00:05", berlin))
|
||||||
@@ -63,8 +63,8 @@ class WidgetRolloverSchedulerTest {
|
|||||||
@Test
|
@Test
|
||||||
fun `the result is always in the future for every minute of a day`() {
|
fun `the result is always in the future for every minute of a day`() {
|
||||||
val zone = berlin
|
val zone = berlin
|
||||||
// Spans Berlin's 2024 spring-forward, the case most likely to produce a
|
// Spans Berlin's 2024 spring-forward: the likeliest source of a target
|
||||||
// target in the past and so an alarm that fires immediately, forever.
|
// in the past, i.e. an alarm that fires immediately, forever.
|
||||||
var probe = LocalDateTime.parse("2024-03-29T00:00:00").toInstant(zone)
|
var probe = LocalDateTime.parse("2024-03-29T00:00:00").toInstant(zone)
|
||||||
val end = LocalDateTime.parse("2024-04-01T00:00:00").toInstant(zone)
|
val end = LocalDateTime.parse("2024-04-01T00:00:00").toInstant(zone)
|
||||||
while (probe < end) {
|
while (probe < end) {
|
||||||
@@ -87,9 +87,8 @@ class WidgetRolloverSchedulerTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `fall back does not overshoot into the repeated hour`() {
|
fun `fall back does not overshoot into the repeated hour`() {
|
||||||
// Berlin repeated 02:00-03:00 on 27 October 2024: midnight itself is
|
// Berlin repeated 02:00-03:00 on 27 October 2024: a 25h day, so
|
||||||
// unambiguous, but the day is 25h long, so "now + 24h" would land at
|
// "now + 24h" would land at 23:00 on the 26th and never roll over.
|
||||||
// 23:00 on the 26th and never roll the date over at all.
|
|
||||||
val now = at("2024-10-26T12:00:00", berlin)
|
val now = at("2024-10-26T12:00:00", berlin)
|
||||||
val next = WidgetRolloverScheduler.nextRolloverAt(now, berlin)
|
val next = WidgetRolloverScheduler.nextRolloverAt(now, berlin)
|
||||||
assertThat(next).isEqualTo(at("2024-10-27T00:00:05", berlin))
|
assertThat(next).isEqualTo(at("2024-10-27T00:00:05", berlin))
|
||||||
@@ -99,9 +98,8 @@ class WidgetRolloverSchedulerTest {
|
|||||||
@Test
|
@Test
|
||||||
fun `a zone that repeats midnight takes the first start of day`() {
|
fun `a zone that repeats midnight takes the first start of day`() {
|
||||||
// Sao Paulo used to end DST by moving 00:00 back to 23:00, so the day
|
// Sao Paulo used to end DST by moving 00:00 back to 23:00, so the day
|
||||||
// began twice. Pinned deliberately: the widget then runs an hour ahead
|
// began twice. Pinned as the accepted trade: the widget runs an hour
|
||||||
// of the clock until the next redraw, which is the accepted trade
|
// ahead until the next redraw. No live zone does this since 2019.
|
||||||
// (Brazil dropped DST in 2019, so no live zone does this).
|
|
||||||
val saoPaulo = TimeZone.of("America/Sao_Paulo")
|
val saoPaulo = TimeZone.of("America/Sao_Paulo")
|
||||||
val next = WidgetRolloverScheduler.nextRolloverAt(
|
val next = WidgetRolloverScheduler.nextRolloverAt(
|
||||||
at("2018-02-16T12:00:00", saoPaulo), saoPaulo,
|
at("2018-02-16T12:00:00", saoPaulo), saoPaulo,
|
||||||
@@ -132,8 +130,8 @@ class WidgetRolloverSchedulerTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `the same instant rolls over at different times in different zones`() {
|
fun `the same instant rolls over at different times in different zones`() {
|
||||||
// Flying east and getting TIMEZONE_CHANGED must re-arm to the new local
|
// TIMEZONE_CHANGED must re-arm to the new local midnight: the arithmetic
|
||||||
// midnight — the arithmetic follows the zone, not a cached offset.
|
// follows the zone, not a cached offset.
|
||||||
val instant = at("2026-08-27T12:00:00", berlin)
|
val instant = at("2026-08-27T12:00:00", berlin)
|
||||||
val tokyo = TimeZone.of("Asia/Tokyo")
|
val tokyo = TimeZone.of("Asia/Tokyo")
|
||||||
val berlinNext = WidgetRolloverScheduler.nextRolloverAt(instant, berlin)
|
val berlinNext = WidgetRolloverScheduler.nextRolloverAt(instant, berlin)
|
||||||
|
|||||||
Reference in New Issue
Block a user