[interactive_media_ads] Adds initial iOS implementation #7063

bparrishMines · 2024-07-05T18:10:11Z

iOS implementation for flutter/flutter#134228

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

bparrishMines · 2024-07-09T20:52:42Z

@stuartmorgan @jmagman I'm getting an error when importing the IMA SDK on iOS. How should I handle importing the SDK so the that it works with apps using and not using Swift Package Manager. I currently have it included in the podspec and Package.swift. Should I only have it in one or both?

Also here are the iOS docs for importing the IMA SDK for reference: https://developers.google.com/interactive-media-ads/docs/sdks/ios/client-side

The error:

Building for: iOS


BUILDING interactive_media_ads/example for iOS
Running command: "flutter build ios --no-codesign" in /Volumes/Work/s/w/ir/x/w/packages/packages/interactive_media_ads/example
Warning: Building for device with codesigning disabled. You will have to manually codesign before deploying to device.
Building dev.flutter.packages.interactiveMediaAdsExample for device (ios-release)...
Adding Swift Package Manager integration...                        11.8s
An error occurred when adding Swift Package Manager integration:
  Error: Unable to get Xcode project information:
 2024-07-05 12:59:11.342 xcodebuild[48157:453818] Writing error result bundle to /var/folders/cs/cqkgmb4n27n6k5bhvx0_34d00000gp/T/ResultBundle_2024-05-07_12-59-0011.xcresult
xcodebuild: error: Could not resolve package dependencies:
  unknown dependency 'GoogleInteractiveMediaAds' in target 'interactive_media_ads'; valid dependencies are: 'swift-package-manager-google-interactive-media-ads-ios'
  unknown dependency 'GoogleInteractiveMediaAds' in target 'interactive_media_ads'; valid dependencies are: 'swift-package-manager-google-interactive-media-ads-ios'
  exhausted attempts to resolve the dependencies graph, with 'interactive_media_ads fileSystem /Volumes/Work/s/w/ir/x/w/packages/packages/interactive_media_ads/ios/interactive_media_ads' unresolved.



Swift Package Manager is currently an experimental feature, please file a bug at
  https://github.com/flutter/flutter/issues/new?template=1_activation.yml 
Consider including a copy of the following files in your bug report:
  ios/Runner.xcodeproj/project.pbxproj
  ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme (or the scheme for the flavor used)

To avoid this failure, disable Flutter Swift Package Manager integration for the project
by adding the following in the project's pubspec.yaml under the "flutter" section:
  "disable-swift-package-manager: true"
Alternatively, disable Flutter Swift Package Manager integration globally with the
following command:
  "flutter config --no-enable-swift-package-manager"


[packages/interactive_media_ads completed in 0m 30s]

packages/interactive_media_ads/ios/interactive_media_ads/Package.swift

jmagman · 2024-07-09T22:52:14Z

packages/interactive_media_ads/ios/interactive_media_ads/Package.swift

+ dependencies: [
+ .package(
+ url: "https://github.com/googleads/swift-package-manager-google-interactive-media-ads-ios",
+ from: "3.23.0")


Do we want upToNextMajor instead?

Does the iOS team already have policy on this? On Android, I think we only use pinned versions because gradle lacked the equivalent of a Podfile.lock or pubspec.lock. Should we be consistent with Android or should iOS implementations generally use upToNextMajor if possible?

cc @stuartmorgan

Pinning is sort of a myth on Android because of optimistic upgrades. For CocoaPods we've generally allowed major version ranges for dependencies with an authoritative/trusted source (e.g., the vendor of the thing the plugin is wrapping).

jmagman · 2024-07-09T22:53:25Z

I'm getting an error when importing the IMA SDK on iOS. How should I handle importing the SDK so the that it works with apps using and not using Swift Package Manager.

I added a comment to Package.swift, does that fix it?

cc @loic-sharma

...ios/interactive_media_ads/Sources/interactive_media_ads/ViewControllerProxyAPIDelegate.swift

bparrishMines · 2024-07-10T18:47:57Z

packages/interactive_media_ads/lib/src/ad_display_container.dart

@@ -37,6 37,7 @@ class AdDisplayContainer extends StatelessWidget {
 AdDisplayContainer({
 Key? key,
 required void Function(AdDisplayContainer container) onContainerAdded,
+ TextDirection layoutDirection = TextDirection.ltr,


This value is required for UIKitView, so I added to the interface.

stuartmorgan · 2024-07-17T20:29:11Z

packages/interactive_media_ads/example/ios/RunnerTests/TestBinaryMessenger.swift

+class TestBinaryMessenger<T>: NSObject, FlutterBinaryMessenger {
+ let codec: FlutterMessageCodec
+ var result: T?
+ private(set) var handlers: [String: FlutterBinaryMessageHandler] = [:]


Do we ever actually need the functionality provided by all these ivars? I would expect that any real test would use a fake Pigeon API instance, not a fake messenger, since tests shouldn't be relying on Pigeon internals, so this functionality seems like a potential footgun.

Good catch. I think I originally was planning on using the handles, but ending up not. I removed all this code.

packages/interactive_media_ads/ios/interactive_media_ads.podspec

stuartmorgan · 2024-07-18T16:48:51Z

...ads/ios/interactive_media_ads/Sources/interactive_media_ads/AdsRequestProxyAPIDelegate.swift

+ contentPlayhead: IMAContentPlayhead?
+ ) throws -> IMAAdsRequest {
+ let adTagWithRequestAgent =
+ "\(adTagUrl)&request_agent=Flutter-IMA-\(AdsRequestProxyAPIDelegate.pluginVersion)"


What guarantees that this concatenation is valid? E.g., if adTagUrl doesn't have a ?, or has a #, this wouldn't work.

...e_media_ads/ios/interactive_media_ads/Sources/interactive_media_ads/FlutterViewFactory.swift

..._ads/ios/interactive_media_ads/Sources/interactive_media_ads/InteractiveMediaAdsPlugin.swift

packages/interactive_media_ads/lib/src/android/android_view_widget.dart

stuartmorgan · 2024-07-18T19:25:08Z

packages/interactive_media_ads/lib/src/ios/enum_converter_extensions.dart

+
+/// Adds a method to convert native AdErrorType to platform interface
+/// AdErrorType.
+extension NativeAdErrorTypeConverter on ima.AdErrorType {


These should be stand-alone utility functions, per https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#avoid-using-extension

I did this for Android as well. I'll update that as well

stuartmorgan · 2024-07-18T20:25:58Z

packages/interactive_media_ads/lib/src/ios/ios_ad_display_container.dart

+ // A delay is added since this callback only indicates when the View is
+ // created, but not when it has been added to the native View hierarchy.
+ // See https://github.com/flutter/flutter/issues/150802
+ await Future<void>.delayed(const Duration(seconds: 1));


Do we think this is going to always be enough time?

stuartmorgan · 2024-07-18T20:31:47Z

packages/interactive_media_ads/test/ios/ads_manager_delegate_tests.dart

+ final IOSAdsManagerDelegate adsManagerDelegate = IOSAdsManagerDelegate(
+ IOSAdsManagerDelegateCreationParams(
+ onAdEvent: expectAsync1((AdEvent event) {
+ expect(event.type, AdEventType.allAdsCompleted);


It looks like these tests only expect within the callback, in which case failing to call the callback wouldn't fail the test. If that's the case, these should all have a completer declared up front, and completed with the expectation, so the end of the test can also assert that the completer completes (and thus the test did call the callback).

…ndow to uiview

bparrishMines added 30 commits June 20, 2024 16:05

ios pigeons

b80c780

Merge branch 'main' of github.com:flutter/packages into ima_ios

0501906

add addsiplaycontainer

8cf10f4

fix pigeon and implement rest of platform interface

f262b71

add platform thing and update stuff

bc6ca1f

setup to run example

e1d33e8

fix open class errors

7a2a567

implement sdk

5a5bb31

fix bugs and use workaround

cafe9e9

formatting

35d42e1

set issue at delay

87947af

Merge branch 'main' of github.com:flutter/packages into ima_ios

28b23fd

undo conditional

439263f

unused import

b02f27c

version bump

3e6d305

license

29b4840

some tests added

9994b83

test work

161e0bf

some added tests that are broken

2253f7b

Merge branch 'main' of github.com:flutter/packages into ima_ios

a50610c

formatting and add a protocols for apisc

f762faf

update tests to passg

2a93e74

ads loader delegate tests

786a665

more tests

2053d6f

more tests

f9f5aac

Merge branch 'main' of github.com:flutter/packages into ima_ios

8067944

more more tests

bff367c

rest of the native swift tests

823c2a4

fix doc

a0f32bb

change to getView

2a472d9

bparrishMines added 2 commits July 5, 2024 15:40

try correct name maybe

7ef0081

try name again maybe

f4c4c71

jmagman reviewed Jul 9, 2024

View reviewed changes

bparrishMines added 3 commits July 10, 2024 12:48

Merge branch 'main' of github.com:flutter/packages into ima_ios

79cac5a

use product instead

58f0aab

import uikit

9ab595b

jmagman reviewed Jul 10, 2024

View reviewed changes

...ios/interactive_media_ads/Sources/interactive_media_ads/ViewControllerProxyAPIDelegate.swift Show resolved Hide resolved

bparrishMines added 9 commits July 10, 2024 13:19

change binary messenger name and add comment on workaround

0015633

change binary messenger name and add comment on workaround

0bdd871

class docs for all delegate classes

f35004b

wildcards

b4e9b76

add layout direction support

4647293

add the request agent

c4560c1

Merge branch 'main' of github.com:flutter/packages into ima_ios

c03e146

use upToNextMajor

17153b1

uncomment ios pigeon

f78b103

bparrishMines marked this pull request as ready for review July 10, 2024 18:44

bparrishMines changed the title ~~Ima ios~~ [interactive_media_ads] Adds initial iOS implementation Jul 10, 2024

comment

d631f2e

bparrishMines commented Jul 10, 2024

View reviewed changes

bparrishMines requested a review from stuartmorgan July 10, 2024 18:50

stuartmorgan requested changes Jul 18, 2024

View reviewed changes

bparrishMines added 6 commits July 21, 2024 20:49

Merge branch 'main' of github.com:flutter/packages into ima_ios

6e7267a

pr comments

4d3f5f6

Merge branch 'main' of github.com:flutter/packages into ima_ios

28a6e69

android utils

76490fd

add value observing, update to latest pigeon implementation, add uiwi…

830b3ef

…ndow to uiview

Merge branch 'main' of github.com:flutter/packages into ima_ios

9e2be27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[interactive_media_ads] Adds initial iOS implementation #7063

[interactive_media_ads] Adds initial iOS implementation #7063

bparrishMines commented Jul 5, 2024 •

edited

Loading

bparrishMines commented Jul 9, 2024

jmagman Jul 9, 2024

bparrishMines Jul 10, 2024

stuartmorgan Jul 10, 2024

jmagman commented Jul 9, 2024

bparrishMines Jul 10, 2024

stuartmorgan Jul 17, 2024

bparrishMines Jul 23, 2024

stuartmorgan Jul 18, 2024

stuartmorgan Jul 18, 2024

bparrishMines Jul 23, 2024

stuartmorgan Jul 18, 2024

stuartmorgan Jul 18, 2024

[interactive_media_ads] Adds initial iOS implementation #7063

Are you sure you want to change the base?

[interactive_media_ads] Adds initial iOS implementation #7063

Conversation

bparrishMines commented Jul 5, 2024 • edited Loading

Pre-launch Checklist

bparrishMines commented Jul 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmagman commented Jul 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bparrishMines commented Jul 5, 2024 •

edited

Loading