Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Call FragmentStrictMode listeners on main thread of host #131

Closed
wants to merge 5 commits into from
Closed

Call FragmentStrictMode listeners on main thread of host #131

wants to merge 5 commits into from

Conversation

simonschiller
Copy link
Contributor

Proposed Changes

Testing

Test: FragmentStrictModeTest#listenerCalledOnCorrectThread

Issues Fixed

Fixes: 153737341

@google-cla google-cla bot added the cla: yes label Feb 19, 2021
@simonschiller
Copy link
Contributor Author

Please note that I moved the FragmentStrictMode class from androidx.fragment.app.strictmode to androidx.fragment.app, so that we can access package-private methods and fields. This probably also requires changes inside Jetifier, like mentioned here: #123 (comment)

@dlam dlam requested a review from jbw0033 February 19, 2021 17:11
@ianhanniballake
Copy link
Member

Please note that I moved the FragmentStrictMode class from androidx.fragment.app.strictmode to androidx.fragment.app, so that we can access package-private methods and fields. This probably also requires changes inside Jetifier, like mentioned here: #123 (comment)

Instead of doing this, can we make FragmentManager.getHost() @hide + @RestrictTo(RestrictTo.Scope.LIBRARY) and the same for FragmentHostCallback.getHandler(). That would allow you to use fragment.getParentFragmentManager().getHost().getHandler() from the strictmode package.

I definitely prefer it in a separate package if possible.

ianhanniballake
ianhanniballake approved these changes Mar 2, 2021
View reviewed changes
fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java
@NonNull
FragmentHostCallback<?> getHost() {
@RestrictTo(RestrictTo.Scope.LIBRARY)
Copy link
Member

Choose a reason for hiding this comment

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

Nit here and elsewhere: Always add the @RestrictTo annotation as the topmost annotation (i.e., directly below the Javadoc comment where the @hide is so that they are visually next to each other in the code).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense - fixed in 4c0f698

@copybara-service copybara-service bot closed this in 39b483d Mar 2, 2021
@simonschiller simonschiller deleted the feature/strict-mode-listener-on-handler branch March 2, 2021 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ianhanniballake ianhanniballake ianhanniballake approved these changes

@jbw0033 jbw0033 Awaiting requested review from jbw0033 jbw0033 is a code owner

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
2 participants
@simonschiller @ianhanniballake