Platform view devicelab ad banner scroll list real ads by hellohuanlin · Pull Request #144745 · flutter/flutter

hellohuanlin · 2024-03-07T02:04:44Z

This benchmark is to measure the platform view performance improvement.

This project displays multiple AdMob banners in a scrollable list. There will be around 4-5 standard banners (320x50) on screen at the same time.

It is similar to https://github.com/lucalooz/flutter_ads_list_perf

List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.

Fixes #143534
Fixes #143257

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

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

jonahwilliams · 2024-03-08T21:05:07Z

@hellohuanlin is this ready for review? Are you blocked on something?

hellohuanlin · 2024-03-08T22:43:43Z

@jonahwilliams I am still trying to solve the infra issue #144339 (comment)

But yes the code itself is ready for review.

.ci.yaml

dev/devicelab/bin/tasks/platform_views_scroll_perf_ios__timeline_summary.dart

jonahwilliams · 2024-03-08T22:45:39Z

I can't follow the link you sent, you can @me in the internal chat

jonahwilliams · 2024-03-08T22:46:09Z

If the code is ready for review you need to mark it ready for review and ask someone to review it, otherwise everyone will assume you are still tinkering on it.

hellohuanlin · 2024-03-08T23:15:31Z

@jonahwilliams Sorry i linked wrongly, so here you go: #144745

If the code is ready for review you need to mark it ready for review and ask someone to review it, otherwise everyone will assume you are still tinkering on it.

Actually I am still tinkering on it, mainly on the infra side tho. The app side (dart code) should be ready.

jonahwilliams · 2024-03-11T21:29:35Z

If you want feedback you should mark as ready for review. Don't wait until its 100% ready

dev/benchmarks/platform_views_layout/test_driver/scroll_perf_ad_banners_test.dart

dev/benchmarks/platform_views_layout/pubspec.yaml

dev/benchmarks/platform_views_layout/ios/Runner.xcodeproj/project.pbxproj

dev/benchmarks/platform_views_layout/lib/main_ad_banners.dart

dev/benchmarks/platform_views_layout/pubspec.yaml

dev/benchmarks/platform_views_layout/test_driver/scroll_perf_ad_banners_test.dart

jmagman · 2024-03-13T19:22:33Z

    info • Use 'const' with the constructor to improve performance • dev/benchmarks/platform_views_layout/lib/main_ad_banners.dart:35:17 • prefer_const_constructors
    info • Use a 'SizedBox' to add whitespace to a layout • dev/benchmarks/platform_views_layout/lib/main_ad_banners.dart:54:17 • sized_box_for_whitespace

dev/benchmarks/platform_views_layout/pubspec.yaml

gspencergoog · 2024-03-15T00:19:05Z

Reason for revert: appears to have broken the build. (Flutter Gardener)

…)" This reverts commit be78542.

)" (#145189) Reverts: #144745 Initiated by: gspencergoog Reason for reverting: appears to have broken the build. Original PR Author: hellohuanlin Reviewed By: {jmagman, jonahwilliams} This change reverts the following previous change: This benchmark is to measure the platform view performance improvement. It is similar to https://github.com/lucalooz/flutter_ads_list_perf There's still a pending issue #144339 *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* Fixes #143534 Fixes #143257 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

jonahwilliams · 2024-03-15T00:51:01Z

I think we just need to update the gradle lockfiles for the android app

…er/flutter#144745)

…5224) Reland #144745, which got reverted due to Android lockfile. Fixed by `dart dev/tools/bin/generate_gradle_lockfiles.dart` *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* Fixes #143534 Fixes #143257 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

…ads" (#145224)" (#145228) Reverts: #145224 Initiated by: hellohuanlin Reason for reverting: breaks the tree Original PR Author: hellohuanlin Reviewed By: {gmackall, jmagman} This change reverts the following previous change: Reland #144745, which got reverted due to Android lockfile. Fixed by `dart dev/tools/bin/generate_gradle_lockfiles.dart` *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* Fixes #143534 Fixes #143257 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

Reland #144745, which was reverted due to a the Android app ads not being set up correctly, crashing on launch: #145228 Add the missing [`com.google.android.gms.ads.APPLICATION_ID` `meta-data` tag](https://developers.google.com/admob/android/quick-start#import_the_mobile_ads_sdk) to the manifest. Validated both `platform_views_scroll_perf__timeline_summary` and `platform_views_scroll_perf_impeller__timeline_summary` ran locally on an Android emulator. Successful presubmit runs: https://ci.chromium.org/ui/p/flutter/builders/try/Linux_pixel_7pro%20platform_views_scroll_perf__timeline_summary/4/overview https://ci.chromium.org/ui/p/flutter/builders/try/Linux_pixel_7pro%20platform_views_scroll_perf_impeller__timeline_summary/4/overview Original commit message: _________ This benchmark is to measure the platform view performance improvement. It is similar to https://github.com/lucalooz/flutter_ads_list_perf There's still a pending issue #144339 Fixes #143534 Fixes #143257

…er/flutter#144745)

github-actions bot added the f: scrolling Viewports, list views, slivers, etc. label Mar 7, 2024

hellohuanlin force-pushed the platform_view_devicelab_ad_banner_scroll_list_real_ads branch 7 times, most recently from b97d3ad to 31a95d8 Compare March 7, 2024 23:17

hellohuanlin commented Mar 8, 2024

View reviewed changes

.ci.yaml Outdated Show resolved Hide resolved

hellohuanlin commented Mar 8, 2024

View reviewed changes

dev/devicelab/bin/tasks/platform_views_scroll_perf_ios__timeline_summary.dart Outdated Show resolved Hide resolved