Merge pull request #1303 from bugsnag/PLAT-6763/orientation-tracking

Avoid unnecessary BroadcastReceiver registration for monitoring device orientation

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## TBD
+
+* Avoid unnecessary BroadcastReceiver registration for monitoring device orientation
+  [#1303](https://github.com/bugsnag/bugsnag-android/pull/1303)
+
 ## 5.9.5 (2021-06-25)
 
 * Unity: Properly handle ANRs after multiple calls to autoNotify and autoDetectAnrs

bugsnag-android-core/detekt-baseline.xml

@@ -9,7 +9,7 @@
     <ID>LongParameterList:AppWithState.kt$AppWithState$( config: ImmutableConfig, binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, duration: Number?, durationInForeground: Number?, inForeground: Boolean?, isLaunching: Boolean? )</ID>
     <ID>LongParameterList:Device.kt$Device$( buildInfo: DeviceBuildInfo, /** * The Application Binary Interface used */ var cpuAbi: Array&lt;String&gt;?, /** * Whether the device has been jailbroken */ var jailbroken: Boolean?, /** * A UUID generated by Bugsnag and used for the individual application on a device */ var id: String?, /** * The IETF language tag of the locale used */ var locale: String?, /** * The total number of bytes of memory on the device */ var totalMemory: Long?, /** * A collection of names and their versions of the primary languages, frameworks or * runtimes that the application is running on */ var runtimeVersions: MutableMap&lt;String, Any&gt;? )</ID>
     <ID>LongParameterList:DeviceBuildInfo.kt$DeviceBuildInfo$( val manufacturer: String?, val model: String?, val osVersion: String?, val apiLevel: Int?, val osBuild: String?, val fingerprint: String?, val tags: String?, val brand: String?, val cpuAbis: Array&lt;String&gt;? )</ID>
-    <ID>LongParameterList:DeviceDataCollector.kt$DeviceDataCollector$( private val connectivity: Connectivity, private val appContext: Context, private val resources: Resources?, private val deviceId: String?, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, rootDetector: RootDetector, bgTaskService: BackgroundTaskService, private val logger: Logger )</ID>
+    <ID>LongParameterList:DeviceDataCollector.kt$DeviceDataCollector$( private val connectivity: Connectivity, private val appContext: Context, resources: Resources, private val deviceId: String?, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, rootDetector: RootDetector, bgTaskService: BackgroundTaskService, private val logger: Logger )</ID>
     <ID>LongParameterList:DeviceWithState.kt$DeviceWithState$( buildInfo: DeviceBuildInfo, jailbroken: Boolean?, id: String?, locale: String?, totalMemory: Long?, runtimeVersions: MutableMap&lt;String, Any&gt;, /** * The number of free bytes of storage available on the device */ var freeDisk: Long?, /** * The number of free bytes of memory available on the device */ var freeMemory: Long?, /** * The orientation of the device when the event occurred: either portrait or landscape */ var orientation: String?, /** * The timestamp on the device when the event occurred */ var time: Date? )</ID>
     <ID>LongParameterList:EventFilenameInfo.kt$EventFilenameInfo.Companion$( obj: Any, uuid: String = UUID.randomUUID().toString(), apiKey: String?, timestamp: Long = System.currentTimeMillis(), config: ImmutableConfig, isLaunching: Boolean? = null )</ID>
     <ID>LongParameterList:StateEvent.kt$StateEvent.Install$( @JvmField val apiKey: String, @JvmField val autoDetectNdkCrashes: Boolean, @JvmField val appVersion: String?, @JvmField val buildUuid: String?, @JvmField val releaseStage: String?, @JvmField val lastRunInfoPath: String, @JvmField val consecutiveLaunchCrashes: Int )</ID>

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ClientContextInitTest.java

@@ -5,6 +5,9 @@
 import android.app.Application;
 import android.content.Context;
 import android.content.SharedPreferences;
+import android.content.res.Resources;
+
+import androidx.test.core.app.ApplicationProvider;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -68,6 +71,8 @@ public void testClientAppContext() {
     }
 
     private void mockContext(Context context) {
+        Resources resources = ApplicationProvider.getApplicationContext().getResources();
+        when(context.getResources()).thenReturn(resources);
         when(context.getPackageName()).thenReturn("mock.package.name");
         when(context.getCacheDir()).thenReturn(new File(System.getProperty("java.io.tmpdir")));
         when(context.getSharedPreferences("com.bugsnag.android", Context.MODE_PRIVATE))

bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java

@@ -11,8 +11,6 @@
 import android.app.ActivityManager;
 import android.app.Application;
 import android.content.Context;
-import android.content.Intent;
-import android.content.IntentFilter;
 import android.content.res.Resources;
 import android.os.Environment;
 import android.os.storage.StorageManager;
@@ -25,7 +23,6 @@
 import kotlin.jvm.functions.Function1;
 import kotlin.jvm.functions.Function2;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
@@ -225,9 +222,7 @@ public Unit invoke(String activity, Map<String, ?> metadata) {
 
         // register a receiver for automatic breadcrumbs
         systemBroadcastReceiver = SystemBroadcastReceiver.register(this, logger, bgTaskService);
-
-        registerOrientationChangeListener();
-        registerMemoryTrimListener();
+        registerComponentCallbacks();
 
         // load last run info
         lastRunInfoStore = new LastRunInfoStore(immutableConfig);
@@ -342,10 +337,9 @@ private MetadataState copyMetadataState(@NonNull Configuration configuration) {
         return configuration.impl.metadataState.copy(copy);
     }
 
-    private void registerOrientationChangeListener() {
-        IntentFilter configFilter = new IntentFilter();
-        configFilter.addAction(Intent.ACTION_CONFIGURATION_CHANGED);
-        ConfigChangeReceiver receiver = new ConfigChangeReceiver(deviceDataCollector,
+    private void registerComponentCallbacks() {
+        appContext.registerComponentCallbacks(new ClientComponentCallbacks(
+                deviceDataCollector,
                 new Function2<String, String, Unit>() {
                     @Override
                     public Unit invoke(String oldOrientation, String newOrientation) {
@@ -356,14 +350,7 @@ public Unit invoke(String oldOrientation, String newOrientation) {
                         clientObservable.postOrientationChange(newOrientation);
                         return null;
                     }
-                }
-        );
-        ContextExtensionsKt.registerReceiverSafe(appContext, receiver, configFilter, logger);
-    }
-
-    private void registerMemoryTrimListener() {
-        appContext.registerComponentCallbacks(new ClientComponentCallbacks(
-                new Function1<Boolean, Unit>() {
+                }, new Function1<Boolean, Unit>() {
                     @Override
                     public Unit invoke(Boolean isLowMemory) {
                         clientObservable.postMemoryTrimEvent(isLowMemory);

bugsnag-android-core/src/main/java/com/bugsnag/android/ClientComponentCallbacks.kt

@@ -4,9 +4,19 @@ import android.content.ComponentCallbacks
 import android.content.res.Configuration
 
 internal class ClientComponentCallbacks(
+    private val deviceDataCollector: DeviceDataCollector,
+    private val cb: (oldOrientation: String?, newOrientation: String?) -> Unit,
     val callback: (Boolean) -> Unit
 ) : ComponentCallbacks {
-    override fun onConfigurationChanged(newConfig: Configuration) {}
+
+    override fun onConfigurationChanged(newConfig: Configuration) {
+        val oldOrientation = deviceDataCollector.getOrientationAsString()
+
+        if (deviceDataCollector.updateOrientation(newConfig.orientation)) {
+            val newOrientation = deviceDataCollector.getOrientationAsString()
+            cb(oldOrientation, newOrientation)
+        }
+    }
 
     override fun onLowMemory() {
         callback(true)

bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt

@@ -16,13 +16,14 @@ import java.util.Locale
 import java.util.concurrent.Callable
 import java.util.concurrent.Future
 import java.util.concurrent.RejectedExecutionException
+import java.util.concurrent.atomic.AtomicInteger
 import kotlin.math.max
 import kotlin.math.min
 
 internal class DeviceDataCollector(
     private val connectivity: Connectivity,
     private val appContext: Context,
-    private val resources: Resources?,
+    resources: Resources,
     private val deviceId: String?,
     private val buildInfo: DeviceBuildInfo,
     private val dataDirectory: File,
@@ -31,7 +32,7 @@ internal class DeviceDataCollector(
     private val logger: Logger
 ) {
 
-    private val displayMetrics = resources?.displayMetrics
+    private val displayMetrics = resources.displayMetrics
     private val emulator = isEmulator()
     private val screenDensity = getScreenDensity()
     private val dpi = getScreenDensityDpi()
@@ -40,6 +41,7 @@ internal class DeviceDataCollector(
     private val cpuAbi = getCpuAbi()
     private val runtimeVersions: MutableMap<String, Any>
     private val rootedFuture: Future<Boolean>?
+    private var orientation = AtomicInteger(resources.configuration.orientation)
 
     init {
         val map = mutableMapOf<String, Any>()
@@ -79,7 +81,7 @@ internal class DeviceDataCollector(
         runtimeVersions.toMutableMap(),
         calculateFreeDisk(),
         calculateFreeMemory(),
-        calculateOrientation(),
+        getOrientationAsString(),
         Date(now)
     )
 
@@ -235,14 +237,23 @@ internal class DeviceDataCollector(
     }
 
     /**
-     * Get the device orientation, eg. "landscape"
+     * Get the current device orientation, eg. "landscape"
      */
-    internal fun calculateOrientation() = when (resources?.configuration?.orientation) {
+    internal fun getOrientationAsString(): String? = when (orientation.get()) {
         ORIENTATION_LANDSCAPE -> "landscape"
         ORIENTATION_PORTRAIT -> "portrait"
         else -> null
     }
 
+    /**
+     * Called whenever the orientation is updated so that the device information is accurate.
+     * Currently this is only invoked by [ClientComponentCallbacks]. Returns true if the
+     * orientation has changed, otherwise false.
+     */
+    internal fun updateOrientation(newOrientation: Int): Boolean {
+        return orientation.getAndSet(newOrientation) != newOrientation
+    }
+
     fun addRuntimeVersionInfo(key: String, value: String) {
         runtimeVersions[key] = value
     }

bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt

@@ -1,59 +1,132 @@
 package com.bugsnag.android
 
+import android.content.Context
+import android.content.res.Configuration
+import android.content.res.Resources
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
 import org.mockito.Mockito.`when`
 import org.mockito.junit.MockitoJUnitRunner
+import java.io.File
 
 @RunWith(MockitoJUnitRunner::class)
-class ConfigChangeReceiverTest {
+internal class ConfigChangeReceiverTest {
 
-    private lateinit var receiver: ConfigChangeReceiver
+    @Mock
+    lateinit var connectivity: Connectivity
+
+    @Mock
+    lateinit var appContext: Context
+
+    @Mock
+    lateinit var resources: Resources
+
+    @Mock
+    lateinit var config: Configuration
+
+    @Mock
+    lateinit var configUpdate: Configuration
 
     @Mock
-    private lateinit var deviceDataCollector: DeviceDataCollector
+    lateinit var buildInfo: DeviceBuildInfo
+
+    @Mock
+    lateinit var dataDirectory: File
+
+    @Mock
+    lateinit var rootDetector: RootDetector
+
+    @Mock
+    lateinit var bgTaskService: BackgroundTaskService
+
+    private fun createDeviceDataCollector(): DeviceDataCollector = DeviceDataCollector(
+        connectivity,
+        appContext,
+        resources,
+        "",
+        buildInfo,
+        dataDirectory,
+        rootDetector,
+        bgTaskService,
+        NoopLogger
+    )
+
+    @Test
+    fun testStringMapping() {
+        `when`(resources.configuration).thenReturn(config)
+        config.orientation = Configuration.ORIENTATION_LANDSCAPE
+
+        val collector = createDeviceDataCollector()
+        assertEquals("landscape", collector.getOrientationAsString())
+
+        collector.updateOrientation(Configuration.ORIENTATION_PORTRAIT)
+        assertEquals("portrait", collector.getOrientationAsString())
+
+        collector.updateOrientation(Configuration.ORIENTATION_UNDEFINED)
+        assertNull(collector.getOrientationAsString())
+    }
+
+    @Test
+    fun testOrientationTracking() {
+        `when`(resources.configuration).thenReturn(config)
+        config.orientation = Configuration.ORIENTATION_LANDSCAPE
+
+        val collector = createDeviceDataCollector()
+        assertFalse(collector.updateOrientation(Configuration.ORIENTATION_LANDSCAPE))
+        assertEquals("landscape", collector.getOrientationAsString())
+
+        assertTrue(collector.updateOrientation(Configuration.ORIENTATION_PORTRAIT))
+        assertEquals("portrait", collector.getOrientationAsString())
+    }
 
     @Test
     fun verifyDefaultOrientation() {
-        `when`(deviceDataCollector.calculateOrientation()).thenReturn("portrait")
-        receiver = ConfigChangeReceiver(deviceDataCollector) { _, _ -> }
-        assertEquals("portrait", receiver.orientation)
+        `when`(resources.configuration).thenReturn(config)
+        val orientationCb = { old: String?, new: String? ->
+            assertNull(old)
+            assertEquals("portrait", new)
+        }
+        config.orientation = Configuration.ORIENTATION_UNDEFINED
+        val callbacks = ClientComponentCallbacks(createDeviceDataCollector(), orientationCb, {})
+
+        configUpdate.orientation = Configuration.ORIENTATION_PORTRAIT
+        callbacks.onConfigurationChanged(configUpdate)
     }
 
     @Test
     fun verifyOrientationChange() {
-        `when`(deviceDataCollector.calculateOrientation()).thenReturn("portrait")
-        var old: String? = null
-        var new: String? = null
-
-        receiver = ConfigChangeReceiver(deviceDataCollector) { a, b ->
-            old = a
-            new = b
+        `when`(resources.configuration).thenReturn(config)
+        val orientationCb = { old: String?, new: String? ->
+            assertEquals("portrait", old)
+            assertEquals("landscape", new)
         }
+        config.orientation = Configuration.ORIENTATION_PORTRAIT
+        val callbacks = ClientComponentCallbacks(createDeviceDataCollector(), orientationCb, {})
 
-        `when`(deviceDataCollector.calculateOrientation()).thenReturn("landscape")
-        receiver.onReceive(null, null)
-        assertEquals("portrait", old)
-        assertEquals("landscape", new)
+        configUpdate.orientation = Configuration.ORIENTATION_LANDSCAPE
+        callbacks.onConfigurationChanged(configUpdate)
     }
 
     @Test
-    fun unrelatedConfigurationChange() {
-        `when`(deviceDataCollector.calculateOrientation()).thenReturn("portrait")
+    fun dupeConfigurationChange() {
         var old: String? = null
         var new: String? = null
 
-        receiver = ConfigChangeReceiver(deviceDataCollector) { a, b ->
+        val orientationCb = { a: String?, b: String? ->
             old = a
             new = b
         }
+        `when`(resources.configuration).thenReturn(config)
+        config.orientation = Configuration.ORIENTATION_PORTRAIT
+        val callbacks = ClientComponentCallbacks(createDeviceDataCollector(), orientationCb, {})
 
-        // orientation has not changed, don't invoke a callback
-        `when`(deviceDataCollector.calculateOrientation()).thenReturn("portrait")
-        receiver.onReceive(null, null)
+        configUpdate.orientation = Configuration.ORIENTATION_PORTRAIT
+        callbacks.onConfigurationChanged(configUpdate)
         assertNull(old)
         assertNull(new)
     }