FIAM: Fix FirebaseInAppMessagingDisplayErrorListener not being called #5719

jadenlin-g · 2024-02-16T21:05:37Z

Changed GlideErrorListener to a normal class and pass a new instance of the listener into each load image request instead of being injected as a defaultRequestListener.

Refactored InAppMessage and FirebaseInAppMessagingDisplayCallbacks assignment code for clarity.

Internal tracking: b/321732964

Issue: #5644

github-actions · 2024-02-16T21:05:52Z

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

github-actions · 2024-02-16T21:11:44Z

Unit Test Results

  64 files -   186   64 suites - 186 1m 59s ⏱️ - 2m 51s
319 tests - 1 325 318 ✔️ - 1 308 1 💤 - 17 0 ❌ ±0
642 runs - 2 696 640 ✔️ - 2 662 2 💤 - 34 0 ❌ ±0

Results for commit 9d54fca. ± Comparison against base commit 833a1cc.

♻️ This comment has been updated with latest results.

google-oss-bot · 2024-02-16T21:11:48Z

Coverage Report ¹

Affected Products

`firebase-inappmessaging-display`

Overall coverage changed from 41.69% (833a1cc) to 41.98% (9385b50) by +0.28%.

Filename	Base (`833a1cc`)	Merge (`9385b50`)	Diff
DaggerAppComponent.java	91.11%	93.02%	+1.91%
FiamImageLoader.java	77.94%	78.57%	+0.63%
FirebaseInAppMessagingDisplay.java	88.41%	88.89%	+0.48%
GlideErrorListener.java	8.33%	33.33%	+25.00%

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/c7rpYh2Am8.html

google-oss-bot · 2024-02-16T21:19:04Z

Size Report ¹

Affected Products

firebase-inappmessaging-display
Type Base (833a1cc) Merge (9385b50) Diff
aar 185 kB 184 kB -1.36 kB (-0.7%)
apk (aggressive) 2.57 MB 2.57 MB +1.06 kB (+0.0%)
apk (release) 14.1 MB 14.1 MB -904 B (-0.0%)

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/kIQSXKPF0Y.html

google-oss-bot · 2024-02-16T21:32:11Z

Startup Time Report ¹

Note: Layout is sometimes suboptimal due to limited formatting support on GitHub. Please check this report on GCS.

Notes

This report is for comparing the base commit (833a1cc) and the CI merge commit (9385b50)
Please check below reports for each individual commit to find more details (Perfetto traces, histograms, detailed measurements)
- 833a1cc: https://storage.googleapis.com/firebase-sdk-metric-reports/RNQdKM2g5T/index.html
- 9385b50: https://storage.googleapis.com/firebase-sdk-metric-reports/jXQaelw2ZS/index.html

Startup Times

`fire-fiam`

Device Statistics Distributions

oriole-32

Percentile	`833a1cc`	`9385b50`	Diff	Significant (?)
p10	5.63 ±3 ms	5.78 ±2 ms	+149 μs (+2.6%)	NO
p25	6.02 ±3 ms	6.13 ±2 ms	+109 μs (+1.8%)	NO
p50	6.69 ±3 ms	6.83 ±3 ms	+146 μs (+2.2%)	NO
p75	7.58 ±3 ms	7.79 ±3 ms	+214 μs (+2.8%)	NO
p90	8.61 ±3 ms	9.22 ±3 ms	+615 μs (+7.1%)	NO

20 test runs in comparison

Commit	Test Runs
`833a1cc`	2024-02-20_15:05:08.253567_chOl 2024-02-20_15:05:08.253599_hEij 2024-02-20_15:05:08.253611_ijEd 2024-02-20_15:05:08.253620_ryqZ 2024-02-20_15:05:08.253627_dWqY 2024-02-20_15:05:08.253634_uqGi 2024-02-20_15:05:08.253641_ifnk 2024-02-20_15:05:08.253648_OFKh 2024-02-20_15:05:08.253655_TvBt 2024-02-20_15:05:08.253662_tiiM
`9385b50`	2024-02-20_18:07:38.866121_OyxD 2024-02-20_18:07:38.866152_aAkD 2024-02-20_18:07:38.866161_Wiao 2024-02-20_18:07:38.866171_OzQQ 2024-02-20_18:07:38.866180_wOAV 2024-02-20_18:07:38.866188_FGYS 2024-02-20_18:07:38.866196_dSEz 2024-02-20_18:07:38.866202_abEv 2024-02-20_18:07:38.866209_Cdka 2024-02-20_18:07:38.866216_pIlC

redfin-30

Percentile	`833a1cc`	`9385b50`	Diff	Significant (?)
p10	14.3 ±6 ms	10.9 ±5 ms	-3.39 ms (-23.7%)	NO
p25	15.5 ±6 ms	11.8 ±6 ms	-3.70 ms (-23.8%)	NO
p50	17.7 ±6 ms	13.3 ±6 ms	-4.38 ms (-24.8%)	NO
p75	20.8 ±7 ms	15.7 ±7 ms	-5.03 ms (-24.2%)	NO
p90	24.5 ±7 ms	18.8 ±8 ms	-5.64 ms (-23.0%)	NO

20 test runs in comparison

Commit	Test Runs
`833a1cc`	2024-02-20_15:05:08.253567_chOl 2024-02-20_15:05:08.253599_hEij 2024-02-20_15:05:08.253611_ijEd 2024-02-20_15:05:08.253620_ryqZ 2024-02-20_15:05:08.253627_dWqY 2024-02-20_15:05:08.253634_uqGi 2024-02-20_15:05:08.253641_ifnk 2024-02-20_15:05:08.253648_OFKh 2024-02-20_15:05:08.253655_TvBt 2024-02-20_15:05:08.253662_tiiM
`9385b50`	2024-02-20_18:07:38.866121_OyxD 2024-02-20_18:07:38.866152_aAkD 2024-02-20_18:07:38.866161_Wiao 2024-02-20_18:07:38.866171_OzQQ 2024-02-20_18:07:38.866180_wOAV 2024-02-20_18:07:38.866188_FGYS 2024-02-20_18:07:38.866196_dSEz 2024-02-20_18:07:38.866202_abEv 2024-02-20_18:07:38.866209_Cdka 2024-02-20_18:07:38.866216_pIlC

`fire-fiamd`

Device Statistics Distributions

oriole-32

Percentile	`833a1cc`	`9385b50`	Diff	Significant (?)
p10	9.41 ±4 ms	8.31 ±4 ms	-1.10 ms (-11.7%)	NO
p25	10.1 ±4 ms	8.90 ±4 ms	-1.18 ms (-11.7%)	NO
p50	11.0 ±5 ms	9.76 ±5 ms	-1.19 ms (-10.9%)	NO
p75	12.2 ±5 ms	10.9 ±5 ms	-1.23 ms (-10.1%)	NO
p90	13.6 ±5 ms	12.4 ±5 ms	-1.17 ms (-8.6%)	NO

20 test runs in comparison

Commit	Test Runs
`833a1cc`	2024-02-20_15:05:08.253567_chOl 2024-02-20_15:05:08.253599_hEij 2024-02-20_15:05:08.253611_ijEd 2024-02-20_15:05:08.253620_ryqZ 2024-02-20_15:05:08.253627_dWqY 2024-02-20_15:05:08.253634_uqGi 2024-02-20_15:05:08.253641_ifnk 2024-02-20_15:05:08.253648_OFKh 2024-02-20_15:05:08.253655_TvBt 2024-02-20_15:05:08.253662_tiiM
`9385b50`	2024-02-20_18:07:38.866121_OyxD 2024-02-20_18:07:38.866152_aAkD 2024-02-20_18:07:38.866161_Wiao 2024-02-20_18:07:38.866171_OzQQ 2024-02-20_18:07:38.866180_wOAV 2024-02-20_18:07:38.866188_FGYS 2024-02-20_18:07:38.866196_dSEz 2024-02-20_18:07:38.866202_abEv 2024-02-20_18:07:38.866209_Cdka 2024-02-20_18:07:38.866216_pIlC

redfin-30

Percentile	`833a1cc`	`9385b50`	Diff	Significant (?)
p10	12.0 ±7 ms	14.6 ±6 ms	+2.50 ms (+20.8%)	NO
p25	12.7 ±7 ms	15.4 ±6 ms	+2.76 ms (+21.7%)	NO
p50	13.7 ±8 ms	16.6 ±7 ms	+2.88 ms (+21.0%)	NO
p75	15.1 ±9.7 ms	18.7 ±8 ms	+3.64 ms (+24.1%)	NO
p90	17.0 ±12 ms	21.8 ±9 ms	+4.72 ms (+27.7%)	NO

20 test runs in comparison

Commit	Test Runs
`833a1cc`	2024-02-20_15:05:08.253567_chOl 2024-02-20_15:05:08.253599_hEij 2024-02-20_15:05:08.253611_ijEd 2024-02-20_15:05:08.253620_ryqZ 2024-02-20_15:05:08.253627_dWqY 2024-02-20_15:05:08.253634_uqGi 2024-02-20_15:05:08.253641_ifnk 2024-02-20_15:05:08.253648_OFKh 2024-02-20_15:05:08.253655_TvBt 2024-02-20_15:05:08.253662_tiiM
`9385b50`	2024-02-20_18:07:38.866121_OyxD 2024-02-20_18:07:38.866152_aAkD 2024-02-20_18:07:38.866161_Wiao 2024-02-20_18:07:38.866171_OzQQ 2024-02-20_18:07:38.866180_wOAV 2024-02-20_18:07:38.866188_FGYS 2024-02-20_18:07:38.866196_dSEz 2024-02-20_18:07:38.866202_abEv 2024-02-20_18:07:38.866209_Cdka 2024-02-20_18:07:38.866216_pIlC

`timeToInitialDisplay`

Device Statistics Distributions

oriole-32

Percentile	`833a1cc`	`9385b50`	Diff	Significant (?)
p10	205 ±7 ms	208 ±3 ms	+3.31 ms (+1.6%)	NO
p25	212 ±7 ms	215 ±4 ms	+3.45 ms (+1.6%)	NO
p50	219 ±7 ms	223 ±3 ms	+4.21 ms (+1.9%)	NO
p75	227 ±7 ms	233 ±5 ms	+6.42 ms (+2.8%)	NO
p90	236 ±9 ms	246 ±6 ms	+9.75 ms (+4.1%)	NO

20 test runs in comparison

Commit	Test Runs
`833a1cc`	2024-02-20_15:05:08.253567_chOl 2024-02-20_15:05:08.253599_hEij 2024-02-20_15:05:08.253611_ijEd 2024-02-20_15:05:08.253620_ryqZ 2024-02-20_15:05:08.253627_dWqY 2024-02-20_15:05:08.253634_uqGi 2024-02-20_15:05:08.253641_ifnk 2024-02-20_15:05:08.253648_OFKh 2024-02-20_15:05:08.253655_TvBt 2024-02-20_15:05:08.253662_tiiM
`9385b50`	2024-02-20_18:07:38.866121_OyxD 2024-02-20_18:07:38.866152_aAkD 2024-02-20_18:07:38.866161_Wiao 2024-02-20_18:07:38.866171_OzQQ 2024-02-20_18:07:38.866180_wOAV 2024-02-20_18:07:38.866188_FGYS 2024-02-20_18:07:38.866196_dSEz 2024-02-20_18:07:38.866202_abEv 2024-02-20_18:07:38.866209_Cdka 2024-02-20_18:07:38.866216_pIlC

redfin-30

Percentile	`833a1cc`	`9385b50`	Diff	Significant (?)
p10	250 ±3 ms	272 ±5 ms	+22.2 ms (+8.9%)	MAYBE
p25	256 ±3 ms	278 ±5 ms	+22.7 ms (+8.9%)	YES
p50	263 ±3 ms	286 ±5 ms	+22.6 ms (+8.6%)	MAYBE
p75	273 ±4 ms	296 ±7 ms	+23.4 ms (+8.6%)	MAYBE
p90	285 ±6 ms	312 ±9.6 ms	+27.7 ms (+9.7%)	MAYBE

20 test runs in comparison

Commit	Test Runs
`833a1cc`	2024-02-20_15:05:08.253567_chOl 2024-02-20_15:05:08.253599_hEij 2024-02-20_15:05:08.253611_ijEd 2024-02-20_15:05:08.253620_ryqZ 2024-02-20_15:05:08.253627_dWqY 2024-02-20_15:05:08.253634_uqGi 2024-02-20_15:05:08.253641_ifnk 2024-02-20_15:05:08.253648_OFKh 2024-02-20_15:05:08.253655_TvBt 2024-02-20_15:05:08.253662_tiiM
`9385b50`	2024-02-20_18:07:38.866121_OyxD 2024-02-20_18:07:38.866152_aAkD 2024-02-20_18:07:38.866161_Wiao 2024-02-20_18:07:38.866171_OzQQ 2024-02-20_18:07:38.866180_wOAV 2024-02-20_18:07:38.866188_FGYS 2024-02-20_18:07:38.866196_dSEz 2024-02-20_18:07:38.866202_abEv 2024-02-20_18:07:38.866209_Cdka 2024-02-20_18:07:38.866216_pIlC

https://storage.googleapis.com/firebase-sdk-metric-reports/v01kj8jTxR/index.html

eldhosembabu

LGTM

eldhosembabu · 2024-02-21T16:52:58Z

_ No description provided. _

Please provide a description for the PR

Cleeb90

Working to resolve issues
Cb developer

…#5719) Changed `GlideErrorListener` to a normal class and pass a new instance of the listener into each load image request instead of being injected as a defaultRequestListener. Refactored `InAppMessage` and `FirebaseInAppMessagingDisplayCallbacks` assignment code for clarity. Internal tracking: b/321732964 Issue: #5644

jadenlin-g added 5 commits February 15, 2024 15:13

Set FIAM and callback values for GlideErrorListener

367800c

refactor to common functions

5beb207

Make GlideErrorListener per request instead of a singleton

359034a

added changelog

f6b5896

remove .external

97f0b5b

google-oss-bot added the size/M label Feb 16, 2024

jadenlin-g changed the title ~~Fix FIAM FirebaseInAppMessagingDisplayErrorListener not being called~~ FIAM: Fix FirebaseInAppMessagingDisplayErrorListener not being called Feb 16, 2024

Update changelog for inappmessaging, added unit tests and run lint

5c2f0fd

jadenlin-g requested a review from eldhosembabu February 20, 2024 16:24

Merge branch 'master' into issue5644

9d54fca

eldhosembabu approved these changes Feb 21, 2024

View reviewed changes

eldhosembabu requested a review from rlazo February 21, 2024 16:52

rlazo approved these changes Feb 21, 2024

View reviewed changes

jadenlin-g merged commit f419204 into master Feb 21, 2024
27 of 29 checks passed

jadenlin-g deleted the issue5644 branch February 21, 2024 23:35

Cleeb90 approved these changes Feb 24, 2024

View reviewed changes

firebase locked and limited conversation to collaborators Mar 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIAM: Fix FirebaseInAppMessagingDisplayErrorListener not being called #5719

FIAM: Fix FirebaseInAppMessagingDisplayErrorListener not being called #5719

`firebase-inappmessaging-display`

`firebase-inappmessaging-display`

`fire-fiam`

`fire-fiamd`

`timeToInitialDisplay`

FIAM: Fix FirebaseInAppMessagingDisplayErrorListener not being called #5719

FIAM: Fix FirebaseInAppMessagingDisplayErrorListener not being called #5719

Conversation

📝 PRs merging into main branch

Unit Test Results

Coverage Report 1

Affected Products

firebase-inappmessaging-display

Test Logs

Size Report 1

Affected Products

firebase-inappmessaging-display

Test Logs

Startup Time Report 1

Notes

Startup Times

fire-fiam

fire-fiamd

timeToInitialDisplay

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Coverage Report ¹

`firebase-inappmessaging-display`

Size Report ¹

`firebase-inappmessaging-display`

Startup Time Report ¹

`fire-fiam`

`fire-fiamd`

`timeToInitialDisplay`