Skip to content

Call FragmentStrictMode listeners on main thread of host#131

Closed
simonschiller wants to merge 5 commits intoandroidx:androidx-mainfrom
simonschiller:feature/strict-mode-listener-on-handler
Closed

Call FragmentStrictMode listeners on main thread of host#131
simonschiller wants to merge 5 commits intoandroidx:androidx-mainfrom
simonschiller:feature/strict-mode-listener-on-handler

Conversation

@simonschiller
Copy link
Contributor

@simonschiller simonschiller commented Feb 19, 2021

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

simonschiller commented Feb 19, 2021

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

ianhanniballake commented Feb 22, 2021

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
/** @hide */
@NonNull
FragmentHostCallback<?> getHost() {
@RestrictTo(RestrictTo.Scope.LIBRARY)
Copy link
Member

@ianhanniballake ianhanniballake Mar 2, 2021

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

@simonschiller simonschiller Mar 2, 2021

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

Development

Successfully merging this pull request may close these issues.

2 participants

@simonschiller @ianhanniballake