OM SDK intergration for Android

[phiviet01]

No description provided.

[JakubMrozek]

@claude

[claude]

Claude finished @JakubMrozek's task in 2m 18s —— View job

---

Code Review: OM SDK Integration for Android

• Read changed files
• Analyze implementation
• Post review feedback

---

Overall the implementation is solid and follows the same patterns as the iOS OM integration. A few issues worth addressing:

---

android/build.gradle — Repository injection anti-pattern

rootProject.allprojects {
    repositories {
        maven { url omSdkLocalMavenRepo }
    }
}

Mutating rootProject.allprojects from inside a module's build.gradle is an anti-pattern. It injects a repository into every project in the build, which is unexpected behavior from a library module and can cause conflicts in the host app's build setup. Modern Gradle prefers declaring this in settings.gradle under dependencyResolutionManagement.repositories. Since this is a Flutter plugin, the consumer controls settings.gradle, so the local Maven workaround could instead be documented, or the AAR could be placed somewhere more conventional (e.g. bundled with the plugin and declared via a relative path in the plugin's own settings). At minimum, document the side effect prominently.

Fix this →

---

OMCreativeType.kt — Deprecated values() call

fun fromRawValue(value: String?): OMCreativeType? = values().firstOrNull { it.rawValue == value }

values() is deprecated in Kotlin; replace with entries:

fun fromRawValue(value: String?): OMCreativeType? = entries.firstOrNull { it.rawValue == value }

Fix this →

---

OMRetentionPool.kt — Thread safety

retainedWebViews is a plain mutableMapOf (not thread-safe). retain() can be called from any thread (it's not enforced to the main thread at the call site in WebViewOMLifecycle.dispose()), while the delayed removal lambda runs on mainHandler. If retain() is ever called off the main thread there's a potential race. Consider annotating or enforcing that retain() must be called on the main thread, or using @MainThread annotation for clarity.

---

OMManager.kt — activate() exception handling

activate() catches IllegalArgumentException and IllegalStateException, but Omid.activate() is documented to also throw NullPointerException in some versions if the context is invalid. A broader catch (exception: Exception) or at minimum adding NullPointerException would be safer here given this is SDK initialization code.

---

OMSession.kt — Unhandled retire-iframe message type

fun retire() {
    webView.evaluateJavascript("window.postMessage({ type: 'retire-iframe' }, '*');", null)
}

retire-iframe is not listed among the known message types handled by the Flutter ad iframe (init-iframe, show-iframe, resize-iframe, click-iframe, open-component-iframe, close-component-iframe, error-iframe, ad-done-iframe). Verify the web-side ad code actually handles this message — if it's unhandled, the OM session won't signal retirement correctly to the ad creative.

---

KontextInAppWebView.kt — markPageStarted called before loadUrl

omLifecycle.markPageStarted(initialUrl)  // line ~391
if (!initialUrl.isNullOrBlank()) {
    webView.loadUrl(initialUrl)
}

This pre-emptively resets hasLoadedPage = false in the lifecycle, which is then reset again when WebViewClient.onPageStarted fires. The double call is harmless but redundant. More importantly — if loadUrl is not called (empty initialUrl), markPageFinished is never called, so OM session start via requestStart() would never fire. This might be intentional (no URL = no ad = no OM session), but worth confirming.

---

omsdk_v1.js — Verify authenticity

The file is 99 lines of minified JS. The official IAB OMID Access Verification JS (omsdk-v1.js) is typically larger. Confirm this is the authentic, unmodified IAB Tech Lab file — shipping a modified OM verification script would break OMID certification compliance. The version should also be pinned and documented.

---

Minor: OMManager.createSession — empty string for customReferenceData

AdSessionContext.createHtmlAdSessionContext(partner, webView, contentUrl, "")

The last parameter is customReferenceData. Passing "" is technically valid, but null is more idiomatic Kotlin and clearer in intent.

---

lib/src/widgets/ads_provider.dart — Platform guard removed

-      if (Platform.isIOS) {
-        unawaited(OMSDKService.activate());
-      }
+      unawaited(OMSDKService.activate());

And in omsdk_service.dart the guard is now !Platform.isIOS && !Platform.isAndroid → returns false. This means calling activate() on unsupported platforms (web, desktop) will silently return false, which is fine. Just make sure the method channel name is only registered on supported platforms to avoid engine warnings on desktop.

---

Summary: The core architecture (session lifecycle, deferred WebView destroy, JS injection via document-start) looks well-designed and matches iOS patterns. The main items to address before merging are: the rootProject.allprojects Gradle anti-pattern, verifying retire-iframe is handled on the web side, and confirming the omsdk_v1.js authenticity.

[JakubMrozek]

@phiviet01 Merged to v4 (different repo)