Skip to content

Fix FIAM ANR Issue 1430 #1488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Apr 22, 2020
Merged

Fix FIAM ANR Issue 1430 #1488

merged 13 commits into from
Apr 22, 2020

Conversation

JasonAHeron
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes Override cla label Apr 21, 2020
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 21, 2020

Coverage Report

Affected SDKs

  • firebase-inappmessaging

    SDK overall coverage did not change between base commit (205b523) and head commit (d538bab). However there are changes in individual files.

    Filename Base (205b523) Head (d538bab) Diff
    ApiClient.java 98.33% 100.00% +1.67%
    InAppMessageStreamManager.java 92.31% 91.11% -1.20%
    InAppMessagingSdkServingGrpc.java 63.24% 67.65% +4.41%

Test Logs

Notes

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 21, 2020

Binary Size Report

Affected SDKs

  • firebase-abt

    Type Base (205b523) Head (d538bab) Diff
    aar ? 35.4 kB ? (?)
    apk (aggressive) ? 85.3 kB ? (?)
    apk (debug) ? 894 kB ? (?)
    apk (release) ? 737 kB ? (?)
  • firebase-common

    Type Base (205b523) Head (d538bab) Diff
    aar ? 34.7 kB ? (?)
    apk (aggressive) ? 82.5 kB ? (?)
    apk (debug) ? 770 kB ? (?)
    apk (release) ? 636 kB ? (?)
  • firebase-components

    Type Base (205b523) Head (d538bab) Diff
    aar ? 34.5 kB ? (?)
    apk (aggressive) ? 11.0 kB ? (?)
    apk (debug) ? 35.8 kB ? (?)
    apk (release) ? 25.4 kB ? (?)
  • firebase-datatransport

    Type Base (205b523) Head (d538bab) Diff
    aar ? 5.04 kB ? (?)
    apk (aggressive) ? 117 kB ? (?)
    apk (debug) ? 846 kB ? (?)
    apk (release) ? 698 kB ? (?)
  • firebase-encoders-json

    Type Base (205b523) Head (d538bab) Diff
    aar ? 15.3 kB ? (?)
    apk (aggressive) ? 11.0 kB ? (?)
    apk (debug) ? 27.4 kB ? (?)
    apk (release) ? 18.9 kB ? (?)
  • firebase-inappmessaging

    Type Base (205b523) Head (d538bab) Diff
    aar ? 469 kB ? (?)
    apk (aggressive) ? 854 kB ? (?)
    apk (debug) ? 4.29 MB ? (?)
    apk (release) ? 3.55 MB ? (?)
  • protolite-well-known-types

    Type Base (205b523) Head (d538bab) Diff
    aar ? 1.20 MB ? (?)
    apk (aggressive) ? 122 kB ? (?)
    apk (debug) ? 643 kB ? (?)
    apk (release) ? 561 kB ? (?)
  • transport-api

    Type Base (205b523) Head (d538bab) Diff
    aar ? 6.59 kB ? (?)
    apk (aggressive) ? 11.0 kB ? (?)
    apk (debug) ? 23.0 kB ? (?)
    apk (release) ? 15.1 kB ? (?)
  • transport-backend-cct

    Type Base (205b523) Head (d538bab) Diff
    aar ? 39.0 kB ? (?)
    apk (aggressive) ? 47.8 kB ? (?)
    apk (debug) ? 101 kB ? (?)
    apk (release) ? 81.6 kB ? (?)
  • transport-runtime

    Type Base (205b523) Head (d538bab) Diff
    aar ? 127 kB ? (?)
    apk (aggressive) ? 35.6 kB ? (?)
    apk (debug) ? 80.5 kB ? (?)
    apk (release) ? 62.8 kB ? (?)

Test Logs

@JasonAHeron JasonAHeron changed the title Fix FIAM ANR Issue #1430 Fix FIAM ANR Issue 1430 Apr 21, 2020
}

// blocking get occurs on the IO thread because that's what we observeOn above
return Maybe.fromCallable(getIID::blockingGet)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to just return getIID here, which is already a Maybe

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah but I need to do blocking get which turns it into a not maybe, if I don't do blocking get here the waiting ends up hitting the main thread below

if (!dataCollectionHelper.isAutomaticDataCollectionEnabled()) {
Logging.logi(
"Automatic data collection is disabled, not attempting campaign fetch from service.");
return Maybe.just(cacheExpiringResponse());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking out loud: Can this be modelled as an empty as well, similar to IID?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm maybe but I think this is pretty clear

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like it could map to empty or something like that but I think it's not worth it

@JasonAHeron JasonAHeron merged commit 2f8e8ac into master Apr 22, 2020
@JasonAHeron JasonAHeron deleted the anr_please_stop branch April 22, 2020 15:47
@firebase firebase locked and limited conversation to collaborators May 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Override cla size/L
Projects
None yet
5 participants