[FSSDK-12503] fix: guard static MethodChannel from multi-engine overwrite

[muzahidul-opti]

Summary

• Fix notification listeners silently breaking when onAttachedToEngine (Android) / register(with:) (iOS) is called more than once
• Root cause: static MethodChannel was unconditionally overwritten by a second Flutter engine (e.g. Firebase background messaging), causing notification callbacks to route to the wrong engine
• Fix: guard the channel assignment with a null check; null the channel in onDetachedFromEngine for proper lifecycle

Changes

• Android OptimizelyFlutterSdkPlugin.java: Early return in onAttachedToEngine if channel already set; set channel = null in onDetachedFromEngine
• Android FlutterLogbackAppender.java: Guard setChannel() from overwriting active channel
• iOS SwiftOptimizelyFlutterSdkPlugin.swift: Early return in register(with:) if channel already set
• iOS OptimizelyFlutterLogger.swift: Guard setChannel() from overwriting active channel

Test plan

• Run existing Dart unit tests (flutter test) - 140/140 passed, no regressions
• Run flutter analyze - no issues found
• Run multi-engine integration test on Android emulator (see flutter-testapp PR)
• Verify decision notification listener fires after second engine creation
• Verify track notification listener fires after second engine creation
• Verify SDK still works normally in single-engine scenario

Fixes FSSDK-12503

🤖 Generated with Claude Code

[pvcraven]

This looks ok by my read, and the claude feedback doesn't look like there are any show-stoppers. You might want to double check #1 though:

  PR Review: [FSSDK-12503] Guard static MethodChannel from multi-engine overwrite

  Overall: Good fix for a real problem, with one notable gap on iOS.

  What it does right

  The root cause is well-identified: when a second FlutterEngine is created (e.g. by Firebase background messaging),
  onAttachedToEngine / register(with:) is called again, unconditionally overwriting the static MethodChannel. This
  silently breaks notification callbacks. The guard pattern (check channel != null before assigning) is the standard
  Flutter approach for this.

  The setChannel / clearChannel split on the loggers is clean — makes intent obvious and prevents accidental null
  overwrites through the setter.

  Issues

  1. iOS has no onDetachedFromEngine equivalent — channel is never cleaned up (medium severity)

  On Android, onDetachedFromEngine now nulls the channel and calls FlutterLogbackAppender.clearChannel(), so a real
  detach-then-reattach cycle works correctly.

  On iOS, there is no corresponding cleanup. SwiftOptimizelyFlutterSdkPlugin has no detach method, and
  OptimizelyFlutterLogger.clearChannel() is added but never called from anywhere. If the original engine is destroyed,
  channel stays non-null pointing at a dead messenger, and the early-return guard will prevent a legitimate new engine
  from attaching.

  This may not be a practical problem today (Flutter iOS doesn't typically create/destroy multiple engines the same
  way), but it's an asymmetry worth either fixing or documenting.

  2. FlutterLogbackAppender.channel is still public static (low severity)

  Line 17 of FlutterLogbackAppender.java — the field is public static MethodChannel channel. The PR adds proper
  setChannel / clearChannel methods, but anyone can still bypass them and write directly to the field. Consider making
  it private static so the guard is enforceable.

  3. Thread safety on the guard check (low severity)

  Both Android and iOS check-then-assign without synchronization:
  if (channel == null) {
      channel = newChannel;
  }
  On Android, onAttachedToEngine is called on the main thread, so this is safe in practice. On iOS, register(with:) is
  also main-thread. Worth confirming there's no codepath that calls these from a background thread, but this is likely
  fine.

  Nits

  - The OptimizelyFlutterLogger.clearChannel() method on iOS is dead code (never called). Either wire it up in a plugin
  detach path or remove it to avoid confusion.

  Summary

  The fix correctly addresses the multi-engine overwrite bug for the common case. The main gap is that iOS lacks a
  cleanup path — if the first engine is destroyed, the SDK will be stuck with a dead channel reference that the guard
  prevents from being replaced. I'd suggest either adding an iOS detach handler or documenting why it's safe to skip.

[muzahidul-opti]

This looks ok by my read, and the claude feedback doesn't look like there are any show-stoppers. You might want to double check #1 though:

  PR Review: [FSSDK-12503] Guard static MethodChannel from multi-engine overwrite

  Overall: Good fix for a real problem, with one notable gap on iOS.

  What it does right

  The root cause is well-identified: when a second FlutterEngine is created (e.g. by Firebase background messaging),
  onAttachedToEngine / register(with:) is called again, unconditionally overwriting the static MethodChannel. This
  silently breaks notification callbacks. The guard pattern (check channel != null before assigning) is the standard
  Flutter approach for this.

  The setChannel / clearChannel split on the loggers is clean — makes intent obvious and prevents accidental null
  overwrites through the setter.

  Issues

  1. iOS has no onDetachedFromEngine equivalent — channel is never cleaned up (medium severity)

  On Android, onDetachedFromEngine now nulls the channel and calls FlutterLogbackAppender.clearChannel(), so a real
  detach-then-reattach cycle works correctly.

  On iOS, there is no corresponding cleanup. SwiftOptimizelyFlutterSdkPlugin has no detach method, and
  OptimizelyFlutterLogger.clearChannel() is added but never called from anywhere. If the original engine is destroyed,
  channel stays non-null pointing at a dead messenger, and the early-return guard will prevent a legitimate new engine
  from attaching.

  This may not be a practical problem today (Flutter iOS doesn't typically create/destroy multiple engines the same
  way), but it's an asymmetry worth either fixing or documenting.

  2. FlutterLogbackAppender.channel is still public static (low severity)

  Line 17 of FlutterLogbackAppender.java — the field is public static MethodChannel channel. The PR adds proper
  setChannel / clearChannel methods, but anyone can still bypass them and write directly to the field. Consider making
  it private static so the guard is enforceable.

  3. Thread safety on the guard check (low severity)

  Both Android and iOS check-then-assign without synchronization:
  if (channel == null) {
      channel = newChannel;
  }
  On Android, onAttachedToEngine is called on the main thread, so this is safe in practice. On iOS, register(with:) is
  also main-thread. Worth confirming there's no codepath that calls these from a background thread, but this is likely
  fine.

  Nits

  - The OptimizelyFlutterLogger.clearChannel() method on iOS is dead code (never called). Either wire it up in a plugin
  detach path or remove it to avoid confusion.

  Summary

  The fix correctly addresses the multi-engine overwrite bug for the common case. The main gap is that iOS lacks a
  cleanup path — if the first engine is destroyed, the SDK will be stuck with a dead channel reference that the guard
  prevents from being replaced. I'd suggest either adding an iOS detach handler or documenting why it's safe to skip.

Resolved

[pvcraven]

Ok, this now adds the cleanup.
It'd be quicker if instead of just saying "resolved" you tell me what you resolved so I don't have to comb through change-history.