Skip to content

fix: Made SDK pause event agnostic #1484

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

Merged
merged 4 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Fixes

- Fixed an issue with the SDK failing to properly detect application pause and focus lost events and creating false positive ANR events (specifically on Android) ([#1484](https://github.com/getsentry/sentry-unity/pull/1484))

### Dependencies

- Bump CLI from v2.21.2 to v2.21.5 ([#1485](https://github.com/getsentry/sentry-unity/pull/1485), [#1494](https://github.com/getsentry/sentry-unity/pull/1494), [#1495](https://github.com/getsentry/sentry-unity/pull/1495))
Expand Down
29 changes: 5 additions & 24 deletions src/Sentry.Unity/SentryMonoBehaviour.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public partial class SentryMonoBehaviour
public event Action? ApplicationPausing;

// Keeping internal track of running state because OnApplicationPause and OnApplicationFocus get called during startup and would fire false resume events
private bool _isRunning = true;
internal bool _isRunning = true;

private IApplication? _application;
internal IApplication Application
Expand Down Expand Up @@ -91,34 +91,15 @@ internal void UpdatePauseStatus(bool paused)
}

/// <summary>
/// To receive Leaving/Resuming events on Android.
/// <remarks>
/// On Android, when the on-screen keyboard is enabled, it causes a OnApplicationFocus(false) event.
/// Additionally, if you press "Home" at the moment the keyboard is enabled, the OnApplicationFocus() event is
/// not called, but OnApplicationPause() is called instead.
/// </remarks>
/// <seealso href="https://docs.unity3d.com/2019.4/Documentation/ScriptReference/MonoBehaviour.OnApplicationPause.html"/>
/// To receive Pause events.
/// </summary>
internal void OnApplicationPause(bool pauseStatus)
{
if (Application.Platform == RuntimePlatform.Android)
{
UpdatePauseStatus(pauseStatus);
}
}
internal void OnApplicationPause(bool pauseStatus) => UpdatePauseStatus(pauseStatus);

/// <summary>
/// To receive Leaving/Resuming events on all platforms except Android.
/// To receive Focus events.
/// </summary>
/// <param name="hasFocus"></param>
internal void OnApplicationFocus(bool hasFocus)
{
// To avoid event duplication on Android since the pause event will be handled via OnApplicationPause
if (Application.Platform != RuntimePlatform.Android)
{
UpdatePauseStatus(!hasFocus);
}
}
internal void OnApplicationFocus(bool hasFocus) => UpdatePauseStatus(!hasFocus);

// The GameObject has to destroy itself since it was created with HideFlags.HideAndDontSave
private void OnApplicationQuit() => Destroy(gameObject);
Expand Down
110 changes: 22 additions & 88 deletions test/Sentry.Unity.Tests/SentryMonoBehaviourTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ public class SentryMonoBehaviourTests
{
private class Fixture
{
public SentryMonoBehaviour GetSut(RuntimePlatform platform)
public SentryMonoBehaviour GetSut()
{
var gameObject = new GameObject("PauseTest");
var sentryMonoBehaviour = gameObject.AddComponent<SentryMonoBehaviour>();
sentryMonoBehaviour.Application = new TestApplication(platform: platform);
sentryMonoBehaviour.Application = new TestApplication();

return sentryMonoBehaviour;
}
Expand All @@ -24,11 +24,11 @@ public SentryMonoBehaviour GetSut(RuntimePlatform platform)
public void SetUp() => _fixture = new Fixture();

[Test]
public void OnApplicationPause_OnAndroid_ApplicationPausingTriggered()
public void OnApplicationPause_PauseStatusTrue_ApplicationPausingInvoked()
{
var wasPausingCalled = false;

var sut = _fixture.GetSut(RuntimePlatform.Android);
var sut = _fixture.GetSut();
sut.ApplicationPausing += () => wasPausingCalled = true;

sut.OnApplicationPause(true);
Expand All @@ -37,72 +37,11 @@ public void OnApplicationPause_OnAndroid_ApplicationPausingTriggered()
}

[Test]
public void OnApplicationPause_NotOnAndroid_ApplicationPausingNotTriggered()
public void OnApplicationFocus_FocusFalse_ApplicationPausingInvoked()
{
var wasPausingCalled = false;

var sut = _fixture.GetSut(RuntimePlatform.IPhonePlayer);
sut.ApplicationPausing += () => wasPausingCalled = true;

sut.OnApplicationPause(true);

Assert.IsFalse(wasPausingCalled);
}

[Test]
public void OnApplicationPause_OnAndroid_ApplicationPausingTriggeredOnlyOnce()
{
var pauseEventTriggerCounter = 0;

var sut = _fixture.GetSut(RuntimePlatform.Android);
sut.ApplicationPausing += () => pauseEventTriggerCounter++;

sut.OnApplicationPause(true);
sut.OnApplicationPause(true);

Assert.AreEqual(1, pauseEventTriggerCounter);
}

[Test]
public void OnApplicationPause_OnAndroid_PausingIsRequiredBeforeApplicationResumingTrigger()
{
var wasPausingCalled = false;
var wasResumingCalled = false;

var sut = _fixture.GetSut(RuntimePlatform.Android);
sut.ApplicationPausing += () => wasPausingCalled = true;
sut.ApplicationResuming += () => wasResumingCalled = true;

sut.OnApplicationPause(false);

Assert.IsFalse(wasResumingCalled);

sut.OnApplicationPause(true);
sut.OnApplicationPause(false);

Assert.IsTrue(wasPausingCalled);
Assert.IsTrue(wasResumingCalled);
}

[Test]
public void OnApplicationFocus_OnAndroid_ApplicationPausingNotTriggered()
{
var wasPausingCalled = false;

var sut = _fixture.GetSut(RuntimePlatform.Android);
sut.ApplicationPausing += () => wasPausingCalled = true;

sut.OnApplicationFocus(false);

Assert.IsFalse(wasPausingCalled);
}

[Test]
public void OnApplicationFocus_NotOnAndroid_ApplicationPausingTriggered()
{
var wasPausingCalled = false;

var sut = _fixture.GetSut(RuntimePlatform.IPhonePlayer);
var sut = _fixture.GetSut();
sut.ApplicationPausing += () => wasPausingCalled = true;

sut.OnApplicationFocus(false);
Expand All @@ -111,38 +50,33 @@ public void OnApplicationFocus_NotOnAndroid_ApplicationPausingTriggered()
}

[Test]
public void OnApplicationFocus_NotOnAndroid_ApplicationPausingTriggeredOnlyOnce()
public void UpdatePauseStatus_PausedTwice_ApplicationPausingInvokedOnlyOnce()
{
var pauseEventTriggerCounter = 0;
var counter = 0;

var sut = _fixture.GetSut(RuntimePlatform.IPhonePlayer);
sut.ApplicationPausing += () => pauseEventTriggerCounter++;
var sut = _fixture.GetSut();
sut.ApplicationPausing += () => counter++;

sut.OnApplicationFocus(false);
sut.OnApplicationFocus(false);
sut.UpdatePauseStatus(true);
sut.UpdatePauseStatus(true);

Assert.AreEqual(1, pauseEventTriggerCounter);
Assert.AreEqual(1, counter);
}

[Test]
public void OnApplicationFocus_NotOnAndroid_PausingIsRequiredBeforeApplicationResumingTrigger()
public void UpdatePauseStatus_ResumedTwice_ApplicationResumingInvokedOnlyOnce()
{
var wasPausingCalled = false;
var wasResumingCalled = false;

var sut = _fixture.GetSut(RuntimePlatform.IPhonePlayer);
sut.ApplicationPausing += () => wasPausingCalled = true;
sut.ApplicationResuming += () => wasResumingCalled = true;
var counter = 0;

sut.OnApplicationFocus(true);
var sut = _fixture.GetSut();
sut.ApplicationResuming += () => counter++;
// We need to pause it first to resume it.
sut.UpdatePauseStatus(true);

Assert.IsFalse(wasResumingCalled);
sut.UpdatePauseStatus(false);
sut.UpdatePauseStatus(false);

sut.OnApplicationFocus(false);
sut.OnApplicationFocus(true);

Assert.IsTrue(wasPausingCalled);
Assert.IsTrue(wasResumingCalled);
Assert.AreEqual(1, counter);
}
}
}