Skip to content

Fix IAM display rendering issues on foldable Android devices (Samsung Galaxy Fold/Flip)#2597

Open
abdulraqeeb33 wants to merge 2 commits intomainfrom
cursor/iam-display-foldable-devices-d6f1
Open

Fix IAM display rendering issues on foldable Android devices (Samsung Galaxy Fold/Flip)#2597
abdulraqeeb33 wants to merge 2 commits intomainfrom
cursor/iam-display-foldable-devices-d6f1

Conversation

@abdulraqeeb33
Copy link
Copy Markdown
Contributor

@abdulraqeeb33 abdulraqeeb33 commented Mar 26, 2026

Description

One Line Summary

Fix In-App Message (IAM) display rendering issues on foldable Android devices, gated behind a remote feature flag.

Details

Motivation

Foldable Android devices like Samsung Galaxy Fold and Flip experience IAM rendering issues when users fold/unfold their devices. The IAM content may appear cut-off, mispositioned, or incorrectly sized because:

  1. Screen size changes without orientation change: When a foldable device transitions between its cover screen and main screen (or vice versa), the screen dimensions change dramatically without triggering an orientation change event.

  2. Deprecated APIs: The SDK was using deprecated defaultDisplay.getSize() APIs that don't properly handle multi-window and foldable device scenarios on Android 11+.

  3. Missing configuration detection: The SDK only listened for CONFIG_ORIENTATION changes, missing the CONFIG_SCREEN_SIZE changes that foldable devices trigger.

Feature Flag Implementation

All changes are gated behind a remote feature flag: SDK_050800_FOLDABLE_IAM_FIX

  • When OFF (default): Legacy behavior is used - no changes to existing functionality
  • When ON: New foldable-aware behavior is activated:
    • Uses WindowMetrics API for accurate window dimensions on API 30+
    • Detects screen size changes from fold/unfold events
    • Recalculates IAM dimensions when screen size changes

This allows for safe rollout and easy rollback if issues are discovered.

Scope

What is affected:

  • In-App Message display on foldable devices (Samsung Galaxy Fold, Flip, and similar devices)
  • Window dimension calculations on Android 11+ (API 30+)

What is NOT affected:

  • IAM behavior when feature flag is disabled (default)
  • IAM behavior on non-foldable devices when flag is enabled (no functional change expected)
  • Other SDK features (notifications, outcomes, sessions, etc.)

Technical Changes

  1. FoldableIAMFeature.kt (NEW):

    • Global feature switch object similar to ThreadingMode
    • isEnabled property controlled by FeatureManager
    • Provides centralized access to feature state from static utilities
  2. FeatureFlag.kt:

    • Added SDK_050800_FOLDABLE_IAM_FIX feature flag
    • Uses IMMEDIATE activation mode for instant toggling
  3. FeatureManager.kt:

    • Added side effect handler for the new feature flag
    • Updates FoldableIAMFeature.isEnabled when flag changes
  4. ViewUtils.kt:

    • getWindowHeight(): Uses WindowMetrics API (API 30+) when FF enabled
    • getWindowWidth(): Uses WindowMetrics API (API 30+) when FF enabled
    • getFullbleedWindowWidth(): Uses WindowMetrics API (API 30+) when FF enabled
    • Falls back to legacy behavior when FF disabled
  5. ApplicationService.kt:

    • Screen size change detection only runs when FF enabled
    • Detects fold/unfold via screenWidthDp/screenHeightDp changes
    • Triggers IAM view recreation on screen size change
  6. WebViewManager.kt:

    • Updated comment to clarify fold/unfold handling alongside rotation

Testing

Unit testing

  • Existing ApplicationServiceTests pass
  • Existing FeatureManagerTests pass
  • The feature flag integration follows the same pattern as SDK_050800_BACKGROUND_THREADING

Manual testing

Recommended testing scenarios:

  1. With flag OFF: Verify no change in behavior on any device
  2. With flag ON on foldable device: Display an IAM, then fold/unfold - IAM should resize correctly
  3. With flag ON on non-foldable device: Verify no regression
  4. Test all IAM positions (TOP_BANNER, BOTTOM_BANNER, CENTER_MODAL, FULL_SCREEN) during fold/unfold

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item
Open in Web Open in Cursor 

This commit addresses rendering issues with In-App Messages (IAM) on
foldable Android devices like Samsung Galaxy Fold/Flip.

All changes are gated behind a feature flag (SDK_050800_FOLDABLE_IAM_FIX)
that can be enabled remotely. When the flag is OFF, the SDK uses legacy
behavior. When ON, the new foldable-aware behavior is activated.

Root cause:
- Foldable devices change screen size without triggering orientation changes
- The SDK only listened for CONFIG_ORIENTATION changes, missing fold/unfold events
- ViewUtils used deprecated APIs that don't properly handle multi-window scenarios

Changes:
1. FoldableIAMFeature.kt (NEW):
   - Global feature switch for foldable device IAM improvements
   - Controlled by FeatureManager via remote config

2. FeatureFlag.kt:
   - Added SDK_050800_FOLDABLE_IAM_FIX feature flag
   - Uses IMMEDIATE activation mode for instant toggling

3. FeatureManager.kt:
   - Added side effect handler for the new feature flag
   - Updates FoldableIAMFeature.isEnabled when flag changes

4. ViewUtils.kt:
   - getWindowHeight(): Uses WindowMetrics API (API 30+) when FF enabled
   - getWindowWidth(): Uses WindowMetrics API (API 30+) when FF enabled
   - getFullbleedWindowWidth(): Uses WindowMetrics API (API 30+) when FF enabled
   - Falls back to legacy behavior when FF disabled

5. ApplicationService.kt:
   - Screen size change detection only runs when FF enabled
   - Detects fold/unfold via screenWidthDp/screenHeightDp changes
   - Triggers IAM view recreation on screen size change

6. WebViewManager.kt:
   - Updated comment to clarify fold/unfold handling

The fix ensures IAMs are properly resized and repositioned when users
fold/unfold their devices, preventing cut-off content and mispositioned
messages - but only when the feature flag is enabled.

Co-authored-by: abdul <abdul@onesignal.com>
@cursor cursor Bot force-pushed the cursor/iam-display-foldable-devices-d6f1 branch from 87d5ea4 to 3e55686 Compare April 2, 2026 20:25
Support-team side builds can exercise foldable IAM behavior without
backend remote config. Remove or clear overrides before general release.

Made-with: Cursor
@abdulraqeeb33 abdulraqeeb33 marked this pull request as ready for review April 27, 2026 18:25
Copilot AI review requested due to automatic review settings April 27, 2026 18:25
@abdulraqeeb33 abdulraqeeb33 requested a review from a team April 27, 2026 18:27
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a remotely gated (“SDK_050800_FOLDABLE_IAM_FIX”) fix for In-App Message sizing/positioning on foldable Android devices by improving window dimension calculations and reacting to fold/unfold-driven configuration changes.

Changes:

  • Introduces a new feature flag and a global switch (FoldableIAMFeature) updated via FeatureManager side effects.
  • Updates ViewUtils window sizing to use WindowMetrics on API 30+ when the flag is enabled.
  • Adds fold/unfold screen-size change detection in ApplicationService to trigger IAM view recreation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/display/impl/WebViewManager.kt Clarifies comment to include screen-size changes (fold/unfold) alongside rotation.
OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt Applies side effects for the new flag (updates FoldableIAMFeature) and adds a local override hook (currently forcing the flag ON).
OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureFlag.kt Adds the SDK_050800_FOLDABLE_IAM_FIX feature flag (IMMEDIATE).
OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/application/impl/ApplicationService.kt Detects screen-size dp changes (fold/unfold) and recreates IAM views when enabled.
OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/ViewUtils.kt Uses WindowMetrics for width/height computations on API 30+ when the flag is enabled.
OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/FoldableIAMFeature.kt Adds a global, volatile feature toggle with logging and an update hook.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +131 to 133
private val localFeatureOverrides: Set<String> =
setOf(FeatureFlag.SDK_050800_FOLDABLE_IAM_FIX.key)
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

localFeatureOverrides is currently hardcoded to setOf(FeatureFlag.SDK_050800_FOLDABLE_IAM_FIX.key), which forces the foldable IAM fix on for all builds and defeats the remote feature-flag gating described in the PR. This should be emptySet() by default (or only enabled via an explicit local/dev-only mechanism) so production behavior remains OFF unless the backend enables it.

Copilot uses AI. Check for mistakes.
Comment on lines +117 to +121
FeatureFlag.SDK_050800_FOLDABLE_IAM_FIX ->
FoldableIAMFeature.updateEnabled(
enabled = enabled,
source = "FeatureManager:${feature.activationMode}"
)
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

A new side effect was added for SDK_050800_FOLDABLE_IAM_FIX, but there’s no unit test asserting that FoldableIAMFeature.isEnabled is updated when the flag is present/absent in config (similar to how FeatureManagerTests asserts ThreadingMode.useBackgroundThreading). Adding a test here would prevent regressions and confirm the IMMEDIATE activation behavior works as intended.

Copilot uses AI. Check for mistakes.
Comment on lines +116 to 122
private fun hasScreenSizeChanged(newConfig: Configuration): Boolean {
if (lastScreenWidthDp == 0 && lastScreenHeightDp == 0) {
return false
}
return newConfig.screenWidthDp != lastScreenWidthDp ||
newConfig.screenHeightDp != lastScreenHeightDp
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

hasScreenSizeChanged returns false until lastScreenWidthDp/lastScreenHeightDp are set, but these are never initialized to the current configuration when the callbacks are registered. This means the first fold/unfold screen-size change after SDK start will be ignored. Initialize the cached dp values from context.resources.configuration (or similar) when creating/registering the ComponentCallbacks so the first change is detected.

Copilot uses AI. Check for mistakes.
Comment on lines 90 to +111
@@ -93,6 +97,28 @@ class ApplicationService() : IApplicationService, ActivityLifecycleCallbacks, On
) {
onOrientationChanged(newConfig.orientation, current!!)
}

// Handle foldable device screen size changes (fold/unfold events)
// Only enabled when FoldableIAMFeature is on
// Foldable devices trigger CONFIG_SCREEN_SIZE without orientation change
if (FoldableIAMFeature.isEnabled && current != null && hasScreenSizeChanged(newConfig)) {
Logging.debug(
"ApplicationService.onConfigurationChanged: Screen size changed " +
"(foldable device fold/unfold detected) - " +
"width: ${newConfig.screenWidthDp}dp, height: ${newConfig.screenHeightDp}dp",
)
onScreenSizeChanged(current!!)
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The screen-size change handler will also fire on normal rotations because screenWidthDp/screenHeightDp change during orientation changes. When the activity uses CONFIG_ORIENTATION, this can cause both onOrientationChanged(...) and onScreenSizeChanged(...) to run back-to-back for the same configuration change, potentially duplicating IAM teardown/recreate work. Consider suppressing the screen-size path when orientation changed (e.g., track last orientation and require it to be unchanged) or when the orientation branch already ran for this callback.

Copilot uses AI. Check for mistakes.
Comment on lines +409 to +412
activityLifecycleNotifier.fire { it.onActivityAvailable(activity) }

activity.window.decorView.viewTreeObserver.addOnGlobalLayoutListener(this)
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

onScreenSizeChanged unconditionally calls addOnGlobalLayoutListener(this) again. Since this class never removes the listener, repeated fold/unfold events can register the same listener multiple times and amplify onGlobalLayout callbacks. Consider ensuring the listener is only added once (or removing it before re-adding) as part of this recreation flow.

Copilot uses AI. Check for mistakes.
Comment on lines 130 to +131
*/
private val localFeatureOverrides: Set<String> = emptySet()
// private val localFeatureOverrides: Set<String> =
// setOf(FeatureFlag.BACKGROUND_THREADING.key)
private val localFeatureOverrides: Set<String> =
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 The latest commit (d46b773) hardcodes localFeatureOverrides to setOf(FeatureFlag.SDK_050800_FOLDABLE_IAM_FIX.key), force-enabling this feature for every install regardless of backend config — directly contradicting the PR description ("When OFF (default): Legacy behavior is used") and the KDoc above the field declaring it a "Local-only test hook for forcing features ON without backend config". This defeats the entire safe-rollout/rollback strategy the flag was introduced for, and causes refreshEnabledFeatures to log the warning Local feature override enabled for testing only on every config refresh in production. Revert to emptySet() (matching the sibling SDK_050800_BACKGROUND_THREADING rollout) before merging.

Extended reasoning...

What the bug is. FeatureManager.kt:130-131 declares:

private val localFeatureOverrides: Set<String> =
    setOf(FeatureFlag.SDK_050800_FOLDABLE_IAM_FIX.key)

The KDoc directly above (lines 125-129) is explicit that this is a "Local-only test hook for forcing features ON without backend config" and shows it being used as a local-development example. The most recent commit on this branch — d46b773 "chore: force SDK_050800_FOLDABLE_IAM_FIX on via localFeatureOverrides" — even acknowledges in its own message that this is a forced-on test override.

The code path that triggers it. refreshEnabledFeatures unconditionally unions localFeatureOverrides into the enabled-feature set:

val enabledFeatureKeys = (model.features + localFeatureOverrides).toSet()
if (localFeatureOverrides.isNotEmpty()) {
    Logging.warn("OneSignal: Local feature override enabled for testing only: $localFeatureOverrides")
}

Because SDK_050800_FOLDABLE_IAM_FIX is IMMEDIATE, every call to applySideEffects will then toggle FoldableIAMFeature.isEnabled = true, regardless of what the backend config returned. The new WindowMetrics-based ViewUtils paths and the new onScreenSizeChanged flow in ApplicationService therefore activate for every user on this SDK build — exactly the opposite of what the PR description guarantees ("When OFF (default): Legacy behavior is used - no changes to existing functionality"). Remote rollback is also impossible: even if the backend disables the flag, the local override re-enables it locally on the next config refresh.

Why existing code doesn't prevent it. There is no environment / debug-build guard around localFeatureOverrides. It is a release-mode constant compiled into the SDK, so consumers ship it as-is. Compare with the sibling SDK_050800_BACKGROUND_THREADING rollout (PR #2595/#2598), which correctly left localFeatureOverrides = emptySet() and only used the field as a commented-out example. The difference here is that the example and the live override were both rewritten to point at SDK_050800_FOLDABLE_IAM_FIX.

Impact. (1) Every consumer of this SDK build silently activates the new fold/unfold detection and WindowMetrics dimension calculations on first launch — the gradual remote rollout is bypassed. (2) The Logging.warn("Local feature override enabled for testing only") line fires on every config refresh in production, polluting consumer logs. (3) Any latent issue in the new code path (e.g., the unconditional onScreenSizeChanged view recreation, the new WindowMetrics API usage on edge OEM builds) cannot be remotely disabled.

Fix. Revert the constant to emptySet(), and (optionally) restore the prior commented-out example shape:

private val localFeatureOverrides: Set<String> = emptySet()
// private val localFeatureOverrides: Set<String> =
//     setOf(FeatureFlag.SDK_050800_FOLDABLE_IAM_FIX.key)

This matches the pattern used for SDK_050800_BACKGROUND_THREADING and preserves the developer-test-hook usability of the field without shipping a forced-on flag.

Step-by-step proof.

  1. SDK starts up; FeatureManager.init calls refreshEnabledFeatures(configModelStore.model, applyNextRunOnlyFeatures = true).
  2. Suppose the backend returns an empty model.features set (the default, matching the PR's claim of OFF-by-default).
  3. enabledFeatureKeys = (emptySet() + setOf("SDK_050800_FOLDABLE_IAM_FIX")).toSet(){"SDK_050800_FOLDABLE_IAM_FIX"}.
  4. The localFeatureOverrides.isNotEmpty() branch logs OneSignal: Local feature override enabled for testing only: [SDK_050800_FOLDABLE_IAM_FIX] — a production log line.
  5. Iterating FeatureFlag.entries, SDK_050800_FOLDABLE_IAM_FIX.isEnabledIn(enabledFeatureKeys) returns true; activationMode is IMMEDIATE, so applySideEffects(SDK_050800_FOLDABLE_IAM_FIX, true) runs and calls FoldableIAMFeature.updateEnabled(enabled = true, ...).
  6. FoldableIAMFeature.isEnabled flips to true for the rest of the process. ViewUtils.getWindowHeight/Width/getFullbleedWindowWidth now take the WindowMetrics branches on API 30+, and ApplicationService.onConfigurationChanged now invokes onScreenSizeChanged on any screenWidthDp/screenHeightDp delta — none of which the backend-config-driven rollout was supposed to permit.
  7. Even if the backend later pushes a config with the flag absent, step 3 will still re-include it via the local override, so remote rollback is impossible.

Comment on lines +100 to +109

// Handle foldable device screen size changes (fold/unfold events)
// Only enabled when FoldableIAMFeature is on
// Foldable devices trigger CONFIG_SCREEN_SIZE without orientation change
if (FoldableIAMFeature.isEnabled && current != null && hasScreenSizeChanged(newConfig)) {
Logging.debug(
"ApplicationService.onConfigurationChanged: Screen size changed " +
"(foldable device fold/unfold detected) - " +
"width: ${newConfig.screenWidthDp}dp, height: ${newConfig.screenHeightDp}dp",
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 The new screen-size branch in onConfigurationChanged (lines 100-109) fires onScreenSizeChanged unconditionally, unlike the orientation branch directly above which gates on AndroidUtils.hasConfigChangeFlag(current!!, ActivityInfo.CONFIG_ORIENTATION). Most apps do not declare screenSize in android:configChanges, so on a fold/unfold Android destroys+recreates the activity while this code simultaneously fires onActivityStopped/onActivityAvailable on the dying activity and re-attaches a layout listener to its doomed decor view — racing the real lifecycle and producing duplicate IAM teardown/recreate. Fix by mirroring the orientation gate: if (FoldableIAMFeature.isEnabled && current != null && AndroidUtils.hasConfigChangeFlag(current!!, ActivityInfo.CONFIG_SCREEN_SIZE) && hasScreenSizeChanged(newConfig)) (and likely also CONFIG_SMALLEST_SCREEN_SIZE on API 24+).

Extended reasoning...

What is wrong

In ApplicationService.onConfigurationChanged, the orientation handler at lines 92-99 deliberately gates onOrientationChanged() behind AndroidUtils.hasConfigChangeFlag(current!!, ActivityInfo.CONFIG_ORIENTATION):

if (current != null &&
    AndroidUtils.hasConfigChangeFlag(current!!, ActivityInfo.CONFIG_ORIENTATION)
) {
    onOrientationChanged(newConfig.orientation, current!!)
}

The new screen-size branch added in this PR (lines 100-109) does not include the analogous CONFIG_SCREEN_SIZE check:

if (FoldableIAMFeature.isEnabled && current != null && hasScreenSizeChanged(newConfig)) {
    Logging.debug(...)
    onScreenSizeChanged(current!!)
}

Why the gate exists

Per the Android documentation, Application.ComponentCallbacks.onConfigurationChanged is invoked on every configuration change. However, if the host activity has not declared the relevant flag in its manifest android:configChanges, Android destroys and recreates the activity for that change. The normal ActivityLifecycleCallbacks path (onActivityStopped on the old activity, then onActivityAvailable on the new one) already re-shows the IAM. Manually re-firing those callbacks ourselves is therefore redundant and races the real lifecycle. This is exactly why the orientation branch checks the flag first.

Step-by-step proof of the race

Assume a typical app whose <activity android:configChanges="..."> does not include screenSize (the default — most apps do not declare this). On a Galaxy Fold unfold:

  1. The OS issues a configuration change with new screenWidthDp / screenHeightDp.
  2. Because the activity does not handle screenSize, the OS schedules the activity to be destroyed and recreated.
  3. Application.ComponentCallbacks.onConfigurationChanged fires — this is independent of activity recreation. current still references the old (about-to-die) activity at this point.
  4. The PR code evaluates FoldableIAMFeature.isEnabled && current != null && hasScreenSizeChanged(newConfig) → all true, so onScreenSizeChanged(current!!) runs.
  5. Inside onScreenSizeChanged, the SDK fires activityLifecycleNotifier.fire { it.onActivityStopped(activity) } and then it.onActivityAvailable(activity) on the doomed activity, and calls activity.window.decorView.viewTreeObserver.addOnGlobalLayoutListener(this) on a decor view that is about to be torn down.
  6. WebViewManager.onActivityAvailable runs calculateHeightAndShowWebViewAfterNewActivity() (because lastActivityName == currentActivityName), which calls setWebViewToMaxSize on the dying activity and posts work to the message-view mutex.
  7. Shortly afterward, Android delivers the real onActivityStopped for the dying activity (which WebViewManager.onActivityStopped handles by calling messageView!!.removeAllViews()), then a fresh onActivityStarted / onActivityAvailable for the recreated activity — which itself triggers another calculateHeightAndShowWebViewAfterNewActivity().

Net effect: two layered teardown/recreate flows running concurrently against different activity references, plus a OnGlobalLayoutListener registered on a decor view that will be detached. At minimum this is duplicate work guarded by a mutex; at worst it is laying out an IAM on a dying window.

Why the existing checks do not save us

  • current != null is true at this point — current is set to null only in onActivityStopped after --activityReferences <= 0, which has not happened yet.
  • hasScreenSizeChanged(newConfig) is comparing dp dimensions; on a fold/unfold these change regardless of whether the activity handles the change.
  • The FoldableIAMFeature.isEnabled flag is force-on in this PR via localFeatureOverrides = setOf(FeatureFlag.SDK_050800_FOLDABLE_IAM_FIX.key) (FeatureManager.kt) plus the previous commit d46b773 ("force SDK_050800_FOLDABLE_IAM_FIX on via localFeatureOverrides"), so the buggy path is live by default for any app consuming this build.

Impact

For the common case (apps that have not opted into android:configChanges="screenSize"), every fold/unfold triggers a redundant IAM stop+available+layout-listener cycle on a dying activity simultaneously with the OS-driven recreate. Beyond duplicated work, attaching to a soon-detached decor view and laying out IAMs on a doomed window can cause visible glitches and is exactly the kind of race the orientation gate was added to avoid.

Fix

Mirror the orientation branch:

if (FoldableIAMFeature.isEnabled &&
    current != null &&
    AndroidUtils.hasConfigChangeFlag(current!!, ActivityInfo.CONFIG_SCREEN_SIZE) &&
    hasScreenSizeChanged(newConfig)
) {
    onScreenSizeChanged(current!!)
}

Optionally also accept CONFIG_SMALLEST_SCREEN_SIZE (and possibly CONFIG_SCREEN_LAYOUT) since on API 24+ apps that handle fold/unfold typically declare smallestScreenSize|screenSize|screenLayout together. The point is the same as the orientation case: only do the manual SDK recreation when the host activity is not going to be recreated by the OS.

Comment on lines +405 to +411
// Remove view
activityLifecycleNotifier.fire { it.onActivityStopped(activity) }

// Show view with new dimensions
activityLifecycleNotifier.fire { it.onActivityAvailable(activity) }

activity.window.decorView.viewTreeObserver.addOnGlobalLayoutListener(this)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 The new onScreenSizeChanged method calls activity.window.decorView.viewTreeObserver.addOnGlobalLayoutListener(this) even though this is already registered on the same decor view by the current setter (line 54). Since fold/unfold does not destroy the activity, the existing registration is still live, and ViewTreeObserver allows duplicates — each fold/unfold accumulates another registration so onGlobalLayout fires N+1 times per layout pass after N folds. The same pattern pre-exists in onOrientationChanged (line 392), so this is a copy-paste of an existing minor smell rather than a new bug; the simplest fix is to drop the redundant addOnGlobalLayoutListener call here (the listener is already attached).

Extended reasoning...

What

ApplicationService.onScreenSizeChanged (added at lines 397–411 in this PR) ends with:

activity.window.decorView.viewTreeObserver.addOnGlobalLayoutListener(this)

But this (the ApplicationService, which implements OnGlobalLayoutListener) was already registered as an OnGlobalLayoutListener on the same decor view when the activity first became current, in the current setter at line 54:

value.window.decorView.viewTreeObserver.addOnGlobalLayoutListener(this)

The whole point of this fix is to handle config changes where the activity is not destroyed/recreated (i.e. the manifest declares configChanges="screenSize"). So when onScreenSizeChanged runs, it is the same activity, the same decor view, and the same ViewTreeObserver — and that observer still has the original registration.

ViewTreeObserver.addOnGlobalLayoutListener does not deduplicate; it appends to a CopyOnWriteArray and dispatches each entry on every layout pass. So after one fold/unfold there are 2 registrations of the same listener, after N folds there are N+1, and onGlobalLayout() fires that many times per layout pass for the rest of the activity's lifetime.

Step-by-step proof

  1. App starts, activity A becomes current → current setter runs, decorView.viewTreeObserver.addOnGlobalLayoutListener(this) (registration gradle? #1).
  2. User folds device. Activity is preserved (manifest declares screenSize), onConfigurationChanged fires, hasScreenSizeChanged returns true, onScreenSizeChanged(activity) runs.
  3. onScreenSizeChanged calls decorView.viewTreeObserver.addOnGlobalLayoutListener(this) again → registration Added possibility to send status 'opened' for a message back to onesignal backend from client source code #2 on the same observer.
  4. Next layout pass: Android iterates OnGlobalLayoutListeners and calls onGlobalLayout() once per registration → fires twice → systemConditionNotifier.fire { systemConditionChanged() } runs twice per layout.
  5. User unfolds → step 3 repeats → registration onPause NPE #3. After N folds: N+1 fires per layout for the activity's remaining lifetime.

Addressing the refutation

The refutation correctly notes that (a) the same pattern pre-exists in onOrientationChanged (line 392), (b) there is no removeOnGlobalLayoutListener anywhere in the file, and (c) the actual harm is bounded because Waiter.wake() is idempotent and systemConditionChanged() re-checks isKeyboardUp each call. All of those are accurate, and they are the reason this is nit-level rather than a correctness bug — the original submitter's "prematurely wake waiters" framing is overstated.

However, the issue is still worth flagging in this specific PR because:

  • The new method is explicitly designed for the case where the activity is not recreated, making the duplicate registration more visible/wasteful than the pre-existing case (orientation changes typically cause activity recreation unless configChanges is declared, in which case it has the same problem).
  • The PR is copying a flawed pattern verbatim into a brand-new code path, so this is the natural moment to fix it before propagating further.

Suggested fix

The simplest fix is to drop the line entirely from onScreenSizeChanged — the listener is already attached and stays attached for the lifetime of the activity:

private fun onScreenSizeChanged(activity: Activity) {
    activityLifecycleNotifier.fire { it.onActivityStopped(activity) }
    activityLifecycleNotifier.fire { it.onActivityAvailable(activity) }
    // (removed: addOnGlobalLayoutListener — already registered in the `current` setter)
}

Alternatively, pair removeOnGlobalLayoutListener(this) with the addOnGlobalLayoutListener(this) so the registration count stays at 1. Either fix could also be applied to onOrientationChanged while you're here.

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.

3 participants