[firebase_analytics]Keep a local registrar to get activity for foreground actions for v1 …

[zhenqiu1101]

…embedder.

Description

The plugin is intended to use in both background and foreground. On v1 embedder, currently we get activity from registrar during registration. Activity is null when the plugin is registered in background and remains null when it's used for foreground actions.

The change is to keep a local registrar instead of activity for v1 embedder and lazily get activity from registrar whenever needed.

Related Issues

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• If the pull request affects only one plugin, the PR title starts with the name of the plugin in brackets (e.g. [cloud_firestore])
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
• All existing and new tests are passing.
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (flutter analyze) does not report any problems on my PR.
• I read and followed the Flutter Style Guide.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
• I updated CHANGELOG.md to add a description of the change.
• I signed the CLA.
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
• No, this is not a breaking change.

[zhenqiu1101]

@mehmetf

[mehmetf]

This is causing a bug internally. See: b/147253757 for details.

We are not sure how to test this properly since it is difficult to replicate an environment where there's no activity with the existing testing harness.

[cyanglaz]

LGTM. We might want to at least add some test for it.

[zhenqiu1101]

Could you suggest what tests to add? I'm not sure how to test the exact behavior of the change as Mehmet mentioned above. We need to replicate the environment as the app is running in the background without activity.

[cyanglaz]

@zhenqiu1101 we might be able to at least add JUnit test to test the getActivity() method.

[zhenqiu1101]

@cyanglaz I don't see any existing Junit test in the entire flutterfire package. It seems all the Java code are covered by E2E test? The firebase analytics e2e test only covers "logEvent" method. I could add for other methods which call getActivity() to have it covered.

If you prefer JUnit test, please suggest where to create the test package (or point me to the existing package if I miss it).

[kroikie]

This looks good to me