Skip to content

Always call foreground activity callbacks on the UI thread #4531

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 2 commits into from
Jan 6, 2023

Conversation

lfkellogg
Copy link
Contributor

This will make it easier to reason about what runs on what thread.

This will make it easier to reason about what runs on what thread.
@google-oss-bot
Copy link
Contributor

1 Warning
⚠️ Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

// construct FirebaseAppDistribution instance on startup so we can register for
// activity lifecycle callbacks before the API is called
.alwaysEager()
.build(),
Component.builder(FeedbackSender.class)
.add(Dependency.required(FirebaseApp.class))
.add(Dependency.requiredProvider(FirebaseInstallationsApi.class))
.add(Dependency.required(backgroundExecutor))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realized this wasn't used in FeedbackSender.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 5, 2023

Coverage Report 1

Affected Products

  • firebase-appdistribution

    Overall coverage changed from ? (8549013) to 76.47% (e4911fa) by ?.

    44 individual files with coverage change

    FilenameBase (8549013)Merge (e4911fa)Diff
    AabUpdater.java?98.41%?
    ApkInstaller.java?100.00%?
    ApkUpdater.java?93.00%?
    AppDistributionReleaseImpl.java?100.00%?
    AppDistributionReleaseInternal.java?100.00%?
    AppIconSource.java?84.62%?
    AutoValue_AppDistributionReleaseImpl.java?65.45%?
    AutoValue_AppDistributionReleaseInternal.java?71.58%?
    AutoValue_ImageUtils_ImageSize.java?35.00%?
    AutoValue_TesterApiDisabledErrorDetails.java?29.41%?
    AutoValue_TesterApiDisabledErrorDetails_HelpLink.java?54.17%?
    AutoValue_UpdateProgressImpl.java?65.96%?
    ErrorMessages.java?0.00%?
    FeedbackActivity.java?1.12%?
    FeedbackSender.java?84.62%?
    FirebaseAppDistributionExceptions.java?80.00%?
    FirebaseAppDistributionFileProvider.java?0.00%?
    FirebaseAppDistributionImpl.java?89.89%?
    FirebaseAppDistributionLifecycleNotifier.java?91.60%?
    FirebaseAppDistributionNotificationsManager.java?87.18%?
    FirebaseAppDistributionRegistrar.java?87.04%?
    FirebaseAppDistributionTesterApiClient.java?88.54%?
    HttpsUrlConnectionFactory.java?50.00%?
    ImageUtils.java?100.00%?
    InstallActivity.java?2.67%?
    LogWrapper.java?100.00%?
    NewReleaseFetcher.java?85.11%?
    PackageInfoUtils.java?42.86%?
    ReleaseIdentifier.java?85.54%?
    ReleaseUtils.java?83.33%?
    ScreenshotTaker.java?41.18%?
    SequentialReference.java?100.00%?
    SignInResultActivity.java?0.00%?
    SignInStorage.java?100.00%?
    TakeScreenshotAndStartFeedbackActivity.java?0.00%?
    TaskCache.java?100.00%?
    TaskCompletionSourceCache.java?72.41%?
    TaskUtils.java?77.50%?
    TesterApiDisabledErrorDetails.java?93.75%?
    TesterApiHttpClient.java?90.18%?
    TesterSignInManager.java?95.18%?
    UpdateProgressImpl.java?100.00%?
    UpdateTaskCache.java?91.30%?
    UpdateTaskImpl.java?76.32%?

Test Logs

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

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2023

Unit Test Results

172 tests   172 ✔️  42s ⏱️
  17 suites      0 💤
  17 files        0

Results for commit 5550fcc.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 5, 2023

Size Report 1

Affected Products

  • base

    TypeBase (8549013)Merge (e4911fa)Diff
    apk (aggressive)?8.39 kB? (?)
    apk (release)?8.65 kB? (?)
  • firebase-annotations

    TypeBase (8549013)Merge (e4911fa)Diff
    apk (aggressive)?8.39 kB? (?)
    apk (release)?9.46 kB? (?)
  • firebase-appdistribution

    TypeBase (8549013)Merge (e4911fa)Diff
    aar?194 kB? (?)
    apk (aggressive)?889 kB? (?)
    apk (release)?2.62 MB? (?)
  • firebase-appdistribution-api

    TypeBase (8549013)Merge (e4911fa)Diff
    aar?16.0 kB? (?)
    apk (aggressive)?112 kB? (?)
    apk (release)?1.27 MB? (?)
  • firebase-common

    TypeBase (8549013)Merge (e4911fa)Diff
    aar?67.4 kB? (?)
    apk (aggressive)?111 kB? (?)
    apk (release)?1.26 MB? (?)
  • firebase-components

    TypeBase (8549013)Merge (e4911fa)Diff
    aar?44.9 kB? (?)
    apk (aggressive)?23.1 kB? (?)
    apk (release)?596 kB? (?)
  • firebase-installations

    TypeBase (8549013)Merge (e4911fa)Diff
    aar?55.0 kB? (?)
    apk (aggressive)?116 kB? (?)
    apk (release)?1.28 MB? (?)
  • firebase-installations-interop

    TypeBase (8549013)Merge (e4911fa)Diff
    aar?8.05 kB? (?)
    apk (aggressive)?65.0 kB? (?)
    apk (release)?651 kB? (?)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 5, 2023

Startup Time Report 1

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

Startup time comparison between the CI merge commit (e4911fa) and the base commit (8549013) are not available.

No macrobenchmark data found for the base commit (8549013). Analysis for the CI merge commit (e4911fa) can be found at:

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

@lfkellogg lfkellogg merged commit 6788546 into fad/in-app-feedback Jan 6, 2023
@lfkellogg lfkellogg deleted the lk/notifier-main-thread branch January 6, 2023 15:27
kaibolay pushed a commit that referenced this pull request Jan 23, 2023
* Always call foreground activity callbacks on the UI thread

This will make it easier to reason about what runs on what thread.

* Refactor getActivityWithIgnoredClass to consolidate synchronized blocks
lfkellogg added a commit that referenced this pull request Jan 25, 2023
* Always call foreground activity callbacks on the UI thread

This will make it easier to reason about what runs on what thread.

* Refactor getActivityWithIgnoredClass to consolidate synchronized blocks
lfkellogg added a commit that referenced this pull request Jan 25, 2023
* Always call foreground activity callbacks on the UI thread

This will make it easier to reason about what runs on what thread.

* Refactor getActivityWithIgnoredClass to consolidate synchronized blocks
@firebase firebase locked and limited conversation to collaborators Feb 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants