chore: separate the start and end methods of screenRenderCollector

Author: AndrewAminInstabug

example/lib/main.dart

@@ -44,6 +44,7 @@ void main() {
 
       Instabug.init(
         token: 'ed6f659591566da19b67857e1b9d40ab',
+        // token: '4d75635ae06e5afb4360c04cfcf1987c',
         invocationEvents: [InvocationEvent.floatingButton],
         debugLogsLevel: LogLevel.verbose,
       ).then((_) {

lib/src/modules/apm.dart

@@ -196,6 +196,9 @@ class APM {
       (_) async {
         // Start screen render collector for custom ui trace if enabled.
         if (await FlagsConfig.screenRendering.isEnabled()) {
+          InstabugScreenRenderManager.I.endScreenRenderCollector();
+
+          // final uiTraceId = IBGDateTime.I.now().millisecondsSinceEpoch;
           InstabugScreenRenderManager.I
               .startScreenRenderCollectorForTraceId(0, UiTraceType.custom);
         }
@@ -211,7 +214,7 @@ class APM {
     // End screen render collector for custom ui trace if enabled.
     if (InstabugScreenRenderManager.I.screenRenderEnabled) {
       return InstabugScreenRenderManager.I
-          .endScreenRenderCollectorForCustomUiTrace();
+          .endScreenRenderCollector();
     }
 
     return _host.endUITrace();

lib/src/utils/instabug_navigator_observer.dart

@@ -1,4 +1,5 @@
 import 'dart:async';
+import 'dart:developer';
 
 import 'package:flutter/material.dart';
 import 'package:instabug_flutter/instabug_flutter.dart';
@@ -27,6 +28,7 @@ class InstabugNavigatorObserver extends NavigatorObserver {
         name: maskedScreenName,
       );
 
+      InstabugScreenRenderManager.I.endScreenRenderCollector();
       ScreenLoadingManager.I
           .startUiTrace(maskedScreenName, screenName)
           .then(_startScreenRenderCollector);
@@ -67,6 +69,7 @@ class InstabugNavigatorObserver extends NavigatorObserver {
 
   FutureOr<void> _startScreenRenderCollector(int? uiTraceId) async {
     final isScreenRenderEnabled = await FlagsConfig.screenRendering.isEnabled();
+    log("isScreenRenderEnabled $isScreenRenderEnabled" , name: "Andrew");
     await _checkForScreenRenderInitialization(isScreenRenderEnabled);
     if (uiTraceId != null && isScreenRenderEnabled) {
       InstabugScreenRenderManager.I

lib/src/utils/screen_rendering/instabug_screen_render_manager.dart

@@ -132,73 +132,68 @@ class InstabugScreenRenderManager {
         return;
       }
 
-      //Save the memory cached data to be sent to native side
-      if (_delayedFrames.isNotEmpty) {
-        _saveCollectedData();
-        _resetCachedFrameData();
-      }
-
-      //Sync the captured screen render data of the Custom UI trace when starting new one
       if (type == UiTraceType.custom) {
-        // Report only if the collector was active
-        if (_screenRenderForCustomUiTrace.isActive) {
-          _reportScreenRenderForCustomUiTrace(_screenRenderForCustomUiTrace);
-          _screenRenderForCustomUiTrace.clear();
-        }
         _screenRenderForCustomUiTrace.traceId = traceId;
       }
 
-      //Sync the captured screen render data of the Auto UI trace when starting new one
       if (type == UiTraceType.auto) {
-        // Report only if the collector was active
-        if (_screenRenderForAutoUiTrace.isActive) {
-          _reportScreenRenderForAutoUiTrace(_screenRenderForAutoUiTrace);
-          _screenRenderForAutoUiTrace.clear();
-        }
         _screenRenderForAutoUiTrace.traceId = traceId;
       }
     } catch (error, stackTrace) {
       _logExceptionErrorAndStackTrace(error, stackTrace);
     }
   }
 
-  /// Stop screen render collector and sync the captured data.
   @internal
-  void stopScreenRenderCollector() {
+  void endScreenRenderCollector([
+    UiTraceType type = UiTraceType.auto,
+  ]) {
     try {
+      // Return if frameTimingListener not attached
+      if (!screenRenderEnabled || !_isTimingsListenerAttached) {
+        return;
+      }
+
+      //Save the memory cached data to be sent to native side
       if (_delayedFrames.isNotEmpty) {
         _saveCollectedData();
+        _resetCachedFrameData();
       }
 
-      // Sync Screen Render data for custom ui trace if exists
-      if (_screenRenderForCustomUiTrace.isActive) {
+      //Sync the captured screen render data of the Custom UI trace if the collector was active
+      if (type == UiTraceType.custom &&
+          _screenRenderForCustomUiTrace.isActive) {
         _reportScreenRenderForCustomUiTrace(_screenRenderForCustomUiTrace);
+        _screenRenderForCustomUiTrace.clear();
       }
 
-      // Sync Screen Render data for auto ui trace if exists
-      if (_screenRenderForAutoUiTrace.isActive) {
+      //Sync the captured screen render data of the Auto UI trace if the collector was active
+      if (type == UiTraceType.auto && _screenRenderForAutoUiTrace.isActive) {
         _reportScreenRenderForAutoUiTrace(_screenRenderForAutoUiTrace);
+        _screenRenderForAutoUiTrace.clear();
       }
     } catch (error, stackTrace) {
       _logExceptionErrorAndStackTrace(error, stackTrace);
     }
   }
 
-  /// Sync the capture screen render data of the custom UI trace without stopping the collector.
+  /// Stop screen render collector and sync the captured data.
   @internal
-  void endScreenRenderCollectorForCustomUiTrace() {
+  void stopScreenRenderCollector() {
     try {
-      if (!_screenRenderForCustomUiTrace.isActive) {
-        return;
+      if (_delayedFrames.isNotEmpty) {
+        _saveCollectedData();
       }
 
-      // Save the captured screen rendering data to be synced
-      _updateCustomUiData();
-
-      // Sync the saved screen rendering data
-      _reportScreenRenderForCustomUiTrace(_screenRenderForCustomUiTrace);
+      // Sync Screen Render data for custom ui trace if exists
+      if (_screenRenderForCustomUiTrace.isActive) {
+        _reportScreenRenderForCustomUiTrace(_screenRenderForCustomUiTrace);
+      }
 
-      _screenRenderForCustomUiTrace.clear();
+      // Sync Screen Render data for auto ui trace if exists
+      if (_screenRenderForAutoUiTrace.isActive) {
+        _reportScreenRenderForAutoUiTrace(_screenRenderForAutoUiTrace);
+      }
     } catch (error, stackTrace) {
       _logExceptionErrorAndStackTrace(error, stackTrace);
     }

test/apm_test.dart

@@ -346,13 +346,10 @@ void main() {
         () async {
       when(mHost.isScreenRenderEnabled()).thenAnswer((_) async => true);
       when(mScreenRenderManager.screenRenderEnabled).thenReturn(true);
-      const traceName = "traceNameTest";
-      await APM.startUITrace(traceName);
       await APM.endUITrace();
 
-      verify(mHost.startUITrace(traceName)).called(1);
       verify(
-        mScreenRenderManager.endScreenRenderCollectorForCustomUiTrace(),
+        mScreenRenderManager.endScreenRenderCollector(),
       ).called(1);
       verifyNever(mHost.endUITrace());
     });
@@ -371,7 +368,7 @@ void main() {
         mHost.endUITrace(),
       ).called(1);
       verifyNever(
-        mScreenRenderManager.endScreenRenderCollectorForCustomUiTrace(),
+        mScreenRenderManager.endScreenRenderCollector(),
       );
     });
   });

test/utils/screen_render/instabug_screen_render_manager_test.dart

@@ -1,12 +1,18 @@
+import 'dart:ui' show FrameTiming;
+
+import 'package:flutter/widgets.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:instabug_flutter/instabug_flutter.dart';
+import 'package:instabug_flutter/src/generated/apm.api.g.dart';
 import 'package:instabug_flutter/src/models/instabug_frame_data.dart';
 import 'package:instabug_flutter/src/models/instabug_screen_render_data.dart';
 import 'package:instabug_flutter/src/utils/screen_rendering/instabug_screen_render_manager.dart';
+import 'package:mockito/annotations.dart';
 import 'package:mockito/mockito.dart';
 
 import 'instabug_screen_render_manager_test.mocks.dart';
 
+@GenerateMocks([ApmHostApi, WidgetsBinding, FrameTiming])
 void main() {
   TestWidgetsFlutterBinding.ensureInitialized();
 
@@ -55,25 +61,6 @@ void main() {
       ); // the one form initForTesting()
     });
 
-    test(
-        'should report data to native when starting new trace from the same type',
-        () async {
-      final frameTestData = InstabugScreenRenderData(
-        traceId: 123,
-        frameData: [
-          InstabugFrameData(10000, 200),
-          InstabugFrameData(20000, 1000),
-        ],
-        frozenFramesTotalDurationMicro: 1000,
-        slowFramesTotalDurationMicro: 200,
-      );
-
-      manager.startScreenRenderCollectorForTraceId(frameTestData.traceId);
-      manager.setFrameData(frameTestData);
-      manager.startScreenRenderCollectorForTraceId(2);
-      verify(mApmHost.endScreenRenderForAutoUiTrace(any)).called(1);
-    });
-
     test('should attach timing listener if it is not attached', () async {
       manager.stopScreenRenderCollector(); // this should detach listener safely
 
@@ -272,7 +259,7 @@ void main() {
 
       manager.setFrameData(frameTestData);
 
-      manager.endScreenRenderCollectorForCustomUiTrace();
+      manager.endScreenRenderCollector();
 
       expect(manager.screenRenderForCustomUiTrace.isActive, false);
       expect(manager.screenRenderForCustomUiTrace == frameTestData, false);
@@ -298,11 +285,11 @@ void main() {
 
       manager.setFrameData(frameTestData);
 
-      manager.endScreenRenderCollectorForCustomUiTrace();
+      manager.endScreenRenderCollector();
     });
 
     test('should not remove timing callback listener', () {
-      manager.endScreenRenderCollectorForCustomUiTrace();
+      manager.endScreenRenderCollector();
 
       verifyNever(mWidgetBinding.removeTimingsCallback(any));
     });
@@ -320,7 +307,7 @@ void main() {
 
       manager.startScreenRenderCollectorForTraceId(0, UiTraceType.custom);
       manager.setFrameData(frameTestData);
-      manager.endScreenRenderCollectorForCustomUiTrace();
+      manager.endScreenRenderCollector(UiTraceType.custom);
       verify(mApmHost.endScreenRenderForCustomUiTrace(any)).called(1);
     });
   });
@@ -419,4 +406,50 @@ void main() {
       );
     });
   });
+
+  group('InstabugScreenRenderManager.endScreenRenderCollector', () {
+    test('should save and reset cached data if delayed frames exist', () {
+      final frameTestData = InstabugScreenRenderData(
+        traceId: 123,
+        frameData: [
+          InstabugFrameData(10000, 200),
+          InstabugFrameData(20000, 1000),
+        ],
+        frozenFramesTotalDurationMicro: 1000,
+        slowFramesTotalDurationMicro: 200,
+      );
+      manager.startScreenRenderCollectorForTraceId(1);
+      manager.setFrameData(frameTestData);
+      manager.endScreenRenderCollector();
+      verify(mApmHost.endScreenRenderForAutoUiTrace(any)).called(1);
+      expect(manager.screenRenderForAutoUiTrace.isEmpty, true);
+      expect(manager.screenRenderForAutoUiTrace.isActive, false);
+    });
+
+    test('should report and clear custom trace if type is custom and active',
+        () {
+      final frameTestData = InstabugScreenRenderData(
+        traceId: 123,
+        frameData: [
+          InstabugFrameData(10000, 200),
+          InstabugFrameData(20000, 1000),
+        ],
+        frozenFramesTotalDurationMicro: 1000,
+        slowFramesTotalDurationMicro: 200,
+      );
+      manager.startScreenRenderCollectorForTraceId(1, UiTraceType.custom);
+      manager.setFrameData(frameTestData);
+      manager.endScreenRenderCollector(UiTraceType.custom);
+      verify(mApmHost.endScreenRenderForCustomUiTrace(any)).called(1);
+      expect(manager.screenRenderForCustomUiTrace.isEmpty, true);
+      expect(manager.screenRenderForCustomUiTrace.isActive, false);
+    });
+
+    test('should return early if not enabled or timings not attached', () {
+      manager.screenRenderEnabled = false;
+      manager.endScreenRenderCollector();
+      verifyNever(mApmHost.endScreenRenderForAutoUiTrace(any));
+      verifyNever(mApmHost.endScreenRenderForCustomUiTrace(any));
+    });
+  });
 }