Use custom tab only for web actions #2310

dlackty · 2021-01-10T08:37:17Z

Currently, FIAM uses Custom Tabs to open every action URI and results in errors for deep links.

Refering to iOS SDK's implementation, which checks whether URI schems is http/https, I implemented a similar logic in this PR.

Not pretty sure if my test change is sufficient as this is my first time to contribute to the project.
So please let me know if there's anything missing.

google-oss-bot · 2021-01-10T08:37:32Z

Hi @dlackty. Thanks for your PR.

I'm waiting for a firebase member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dlackty · 2021-01-10T08:38:53Z

According to the commit history, I guess @JasonAHeron might want to have a look. Sorry if I ping the wrong person.

eldhosembabu

This PR looks good to me.

.../src/main/java/com/google/firebase/inappmessaging/display/FirebaseInAppMessagingDisplay.java

dlackty · 2021-06-02T14:47:10Z

@eldhosembabu Pushed some changes according the suggestions, and also rebased the branch. Thank you.

samtstern · 2021-06-03T14:29:32Z

/ok-to-test

google-oss-bot · 2021-06-03T14:35:49Z

Coverage Report

Affected SDKs

firebase-inappmessaging-display

SDK overall coverage changed from 33.51% (257f96b) to 33.60% (738fc7ae) by +0.09%.

Filename Base (257f96b) Head (738fc7ae) Diff

FirebaseInAppMessagingDisplay.java 88.55% 88.31% -0.23%

Test Logs

Notes

HTML coverage reports can be produced locally with ./gradlew <product>:checkCoverage.
Report files are located at <product-build-dir>/reports/jacoco/.

Head commit (738fc7ae) is created by Prow via merging commits: 257f96b 846ebf3.

samtstern · 2021-06-03T14:53:08Z

/retest

samtstern · 2021-06-03T14:53:58Z

@dlackty here's the test failure:

> Task :firebase-inappmessaging-display:compileReleaseJavaWithJavac FAILED
/home/prow/go/src/github.com/firebase/firebase-android-sdk/firebase-inappmessaging-display/src/main/java/com/google/firebase/inappmessaging/display/FirebaseInAppMessagingDisplay.java:569: error: ';' expected
      return false
                  ^

.../src/main/java/com/google/firebase/inappmessaging/display/FirebaseInAppMessagingDisplay.java

google-oss-bot · 2021-06-03T19:13:12Z

Binary Size Report

Affected SDKs

firebase-inappmessaging-display

Type Base (257f96b) Head (738fc7ae) Diff

aar 203 kB 204 kB +179 B (+0.1%)

apk (aggressive) 1.44 MB 1.44 MB +100 B (+0.0%)

apk (release) 4.56 MB 4.56 MB -48 B (-0.0%)

Test Logs

Notes

Head commit (738fc7ae) is created by Prow via merging commits: 257f96b 846ebf3.

google-oss-bot · 2021-06-03T19:55:59Z

Macrobenchmark Report

Affected SDKs

Measurements are for head commit (846ebf3). Diffing against base commit (257f96b) is working in progress.

baseline

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	118.0 ms	163.0 ms	136.5 ms	149.9 ms	162.1 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	39.0 ms	66.0 ms	56.5 ms	61.5 ms	66.0 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	55.0 ms	91.0 ms	76.5 ms	82.8 ms	90.8 ms

firebase-common

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	134.0 ms	176.0 ms	142.0 ms	162.7 ms	174.7 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	35.0 ms	74.0 ms	53.0 ms	61.4 ms	72.3 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	59.0 ms	86.0 ms	75.5 ms	82.0 ms	85.2 ms

firebase-config

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	124.0 ms	152.0 ms	138.0 ms	151.0 ms	151.8 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	37.0 ms	65.0 ms	52.0 ms	60.4 ms	64.8 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	57.0 ms	87.0 ms	72.0 ms	83.0 ms	86.2 ms

firebase-crashlytics

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	140.0 ms	175.0 ms	159.0 ms	165.6 ms	174.2 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	36.0 ms	73.0 ms	54.0 ms	58.2 ms	70.5 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	51.0 ms	90.0 ms	66.5 ms	87.2 ms	89.8 ms

firebase-database

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	121.0 ms	168.0 ms	141.0 ms	157.3 ms	166.5 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	40.0 ms	64.0 ms	48.0 ms	62.2 ms	64.0 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	52.0 ms	84.0 ms	67.5 ms	80.3 ms	83.8 ms

firebase-dynamic-links

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	126.0 ms	163.0 ms	142.5 ms	156.0 ms	161.7 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	41.0 ms	67.0 ms	50.5 ms	63.2 ms	66.6 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	50.0 ms	94.0 ms	77.5 ms	84.1 ms	92.3 ms

firebase-firestore

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	136.0 ms	163.0 ms	148.5 ms	155.6 ms	162.6 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	37.0 ms	68.0 ms	49.5 ms	61.4 ms	67.4 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	54.0 ms	100.0 ms	75.0 ms	91.5 ms	99.2 ms

firebase-functions

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	133.0 ms	171.0 ms	144.5 ms	149.1 ms	167.0 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	36.0 ms	59.0 ms	54.0 ms	58.0 ms	58.8 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	54.0 ms	94.0 ms	75.0 ms	80.1 ms	91.5 ms

firebase-inappmessaging-display

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	175.0 ms	201.0 ms	183.0 ms	193.0 ms	199.5 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	41.0 ms	70.0 ms	50.0 ms	68.1 ms	69.8 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	53.0 ms	100.0 ms	75.5 ms	84.3 ms	99.2 ms

firebase-messaging

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	132.0 ms	178.0 ms	145.0 ms	160.2 ms	175.0 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	44.0 ms	68.0 ms	53.5 ms	63.3 ms	67.6 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	59.0 ms	87.0 ms	69.5 ms	77.8 ms	86.6 ms

firebase-perf

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	140.0 ms	183.0 ms	160.5 ms	174.6 ms	182.4 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	45.0 ms	69.0 ms	56.0 ms	62.1 ms	67.9 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	58.0 ms	94.0 ms	71.0 ms	84.1 ms	92.3 ms

firebase-storage

Name	Device	Min	Max	P50	P90	P99
BenchmarkTest.startup[mode=COLD]	flame-30-en-portrait	123.0 ms	164.0 ms	141.0 ms	157.6 ms	163.8 ms
BenchmarkTest.startup[mode=HOT]	flame-30-en-portrait	34.0 ms	67.0 ms	55.0 ms	65.1 ms	66.8 ms
BenchmarkTest.startup[mode=WARM]	flame-30-en-portrait	59.0 ms	85.0 ms	75.5 ms	81.1 ms	84.4 ms

Test Logs

samtstern · 2021-06-08T11:18:07Z

@eldhosembabu this is green and approved, do you want to merge it?

google-cla bot added the cla: yes Override cla label Jan 10, 2021

googlebot added the cla: yes Override cla label Jan 10, 2021

google-oss-bot added the needs-ok-to-test label Jan 10, 2021

google-oss-bot added the size/S label Jan 10, 2021

samtstern requested review from JasonAHeron and MeghaB and removed request for JasonAHeron May 27, 2021 11:03

eldhosembabu reviewed May 27, 2021

View reviewed changes

.../src/main/java/com/google/firebase/inappmessaging/display/FirebaseInAppMessagingDisplay.java Show resolved Hide resolved

.../src/main/java/com/google/firebase/inappmessaging/display/FirebaseInAppMessagingDisplay.java Outdated Show resolved Hide resolved

Use custom tabs only for web actions

ca31556

google-oss-bot added size/M and removed size/S labels Jun 2, 2021

samtstern requested a review from eldhosembabu June 3, 2021 14:29

samtstern added ok-to-test and removed needs-ok-to-test labels Jun 3, 2021

samtstern assigned eldhosembabu Jun 3, 2021

samtstern reviewed Jun 3, 2021

View reviewed changes

.../src/main/java/com/google/firebase/inappmessaging/display/FirebaseInAppMessagingDisplay.java Outdated Show resolved Hide resolved

Update helper method naming and add nullity check

846ebf3

eldhosembabu approved these changes Jun 4, 2021

View reviewed changes

samtstern merged commit 0d95ad5 into firebase:master Jul 12, 2021

firebase locked and limited conversation to collaborators Aug 12, 2021

dlackty deleted the fix-fiam-custom-tab branch August 31, 2021 18:41

Use custom tab only for web actions #2310

Use custom tab only for web actions #2310

Uh oh!

Conversation

dlackty commented Jan 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-oss-bot commented Jan 10, 2021

Uh oh!

dlackty commented Jan 10, 2021

Uh oh!

eldhosembabu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dlackty commented Jun 2, 2021

Uh oh!

samtstern commented Jun 3, 2021

Uh oh!

google-oss-bot commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Affected SDKs

firebase-inappmessaging-display

Test Logs

Notes

Uh oh!

samtstern commented Jun 3, 2021

Uh oh!

samtstern commented Jun 3, 2021

Uh oh!

Uh oh!

google-oss-bot commented Jun 3, 2021

Binary Size Report

Affected SDKs

firebase-inappmessaging-display

Test Logs

Notes

Uh oh!

google-oss-bot commented Jun 3, 2021

Macrobenchmark Report

Affected SDKs

baseline

firebase-common

firebase-config

firebase-crashlytics

firebase-database

firebase-dynamic-links

firebase-firestore

firebase-functions

firebase-inappmessaging-display

firebase-messaging

firebase-perf

firebase-storage

Test Logs

Uh oh!

samtstern commented Jun 8, 2021

Uh oh!

Uh oh!

dlackty commented Jan 10, 2021 •

edited

Loading

google-oss-bot commented Jun 3, 2021 •

edited

Loading

`firebase-inappmessaging-display`

`firebase-inappmessaging-display`