Fixed
Status Update
Comments
ku...@google.com <ku...@google.com>
ku...@google.com <ku...@google.com> #2
Thank you for reporting this issue. For us to further investigate this issue, please provide the following additional information:
Steps to reproduce
Please provide source code or apk of a sample application to reproduce the issue.
Also kindly mention the steps to be followed for reproducing the issue with the given sample application.
Expected output
What is the expected output?
Current output
What is the current output?
Steps to reproduce
Please provide source code or apk of a sample application to reproduce the issue.
Also kindly mention the steps to be followed for reproducing the issue with the given sample application.
Expected output
What is the expected output?
Current output
What is the current output?
ms...@gmail.com <ms...@gmail.com> #3
Seeing this with a FragmentPagerAdapter as well. Our app has three static tabs, and opens by default on tab 1. Previously, the fragments for tab 2 and tab 3 would have their onResume() called when the activity's onResume() was called, even though those tabs weren't visible yet.
Now, since 27.1.0, onResume() on those tabs' fragments is only called when the user navigates to them.
Now, since 27.1.0, onResume() on those tabs' fragments is only called when the user navigates to them.
to...@gmail.com <to...@gmail.com> #4
#3 can you try to provide a repro app? I've lost 4 hours already and can't find the root cause, the basic AS sample does not produce that :(
Maybe it's easier from your code.
Maybe it's easier from your code.
ms...@gmail.com <ms...@gmail.com> #5
I also tried with the sample app (https://github.com/googlesamples/android-HorizontalPaging ) which doesn't have the issue.
I did find a clue in my app though: the first tab's fragment does getLoaderManager().initLoader() in its onActivityCreated(). If I comment this out, the onResume() methods of all the fragments are called at once as with support lib 27.0.2.
This might be related to this change in the support lib release notes:
> The underlying implementation of Loaders has been rewritten to use Lifecycle.
It's going to be quite a bit of work to get this into a small sample app though. I can't share my own app.
I did find a clue in my app though: the first tab's fragment does getLoaderManager().initLoader() in its onActivityCreated(). If I comment this out, the onResume() methods of all the fragments are called at once as with support lib 27.0.2.
This might be related to this change in the support lib release notes:
> The underlying implementation of Loaders has been rewritten to use Lifecycle.
It's going to be quite a bit of work to get this into a small sample app though. I can't share my own app.
to...@gmail.com <to...@gmail.com> #6
Thanks I can also confirm the same thing in my case.
Removing the call to getLoaderManager().restartLoader(id, null, this); from onResume of the fragment then allows proper resuming of all fragments. (In my case all loaders Id are different).
This give a clue at how to reproduce, will see if I can tonight.
Removing the call to getLoaderManager().restartLoader(id, null, this); from onResume of the fragment then allows proper resuming of all fragments. (In my case all loaders Id are different).
This give a clue at how to reproduce, will see if I can tonight.
ms...@gmail.com <ms...@gmail.com> #7
A sample app is here: https://github.com/calvarez-ov/android-HorizontalPaging
If you build it as-is, you see three onResume() logs at once, when the app is launched:
02-28 14:03:43.534 15309 15309 V MainActivity/DummySectionFragment: onResume 1
02-28 14:03:43.682 15309 15309 V MainActivity/DummySectionFragment: onResume 2
02-28 14:03:43.682 15309 15309 V MainActivity/DummySectionFragment: onResume 3
If you change it to use support library 27.1.0 (Application/build.gradle), you only see the first onResume(). You see the other ones after you manually click on the tabs:
02-28 14:07:15.491 15443 15443 V MainActivity/DummySectionFragment: onResume 1
...
02-28 14:07:21.504 15443 15443 V MainActivity/DummySectionFragment: onResume 3
...
02-28 14:07:25.119 15443 15443 V MainActivity/DummySectionFragment: onResume 2
Note, this sample app doesn't look anything like the real app which has the same problem :) It's just the smallest example I could make to reproduce the issue. There's no ListView or RecyclerView and no adapter to display the loader data. I explicitly call forceLoad() to make the loader's loadInBackground() be called. If I don't do this, I have the bug even with support lib 27.0.2.
If you build it as-is, you see three onResume() logs at once, when the app is launched:
02-28 14:03:43.534 15309 15309 V MainActivity/DummySectionFragment: onResume 1
02-28 14:03:43.682 15309 15309 V MainActivity/DummySectionFragment: onResume 2
02-28 14:03:43.682 15309 15309 V MainActivity/DummySectionFragment: onResume 3
If you change it to use support library 27.1.0 (Application/build.gradle), you only see the first onResume(). You see the other ones after you manually click on the tabs:
02-28 14:07:15.491 15443 15443 V MainActivity/DummySectionFragment: onResume 1
...
02-28 14:07:21.504 15443 15443 V MainActivity/DummySectionFragment: onResume 3
...
02-28 14:07:25.119 15443 15443 V MainActivity/DummySectionFragment: onResume 2
Note, this sample app doesn't look anything like the real app which has the same problem :) It's just the smallest example I could make to reproduce the issue. There's no ListView or RecyclerView and no adapter to display the loader data. I explicitly call forceLoad() to make the loader's loadInBackground() be called. If I don't do this, I have the bug even with support lib 27.0.2.
ms...@gmail.com <ms...@gmail.com> #8
In case this helps troubleshooting: if you post the restartLoader() (or initLoader()) call to a Handler, the bug goes away:
@Override
public void onActivityCreated(@Nullable Bundle savedInstanceState) {
super.onActivityCreated(savedInstanceState);
- getLoaderManager().restartLoader(1337, null, this);
+ new Handler().post(new Runnable() {
+ @Override
+ public void run() {
+ getLoaderManager().restartLoader(1337, null, FragmentWithLoader.this);
+ }
+ });
}
@Override
public void onActivityCreated(@Nullable Bundle savedInstanceState) {
super.onActivityCreated(savedInstanceState);
- getLoaderManager().restartLoader(1337, null, this);
+ new Handler().post(new Runnable() {
+ @Override
+ public void run() {
+ getLoaderManager().restartLoader(1337, null, FragmentWithLoader.this);
+ }
+ });
}
to...@gmail.com <to...@gmail.com> #9
Actually from Ian Lake: https://plus.google.com/u/0/+IanLake/posts/UNXxBrsBUzJ?cfem=1 This is a normal behavior from before that seems more visible now.
What you can test in the loaderinbackground is a long thread sleep the issue should be back even when running from the handler.
I guess the root issue is that before the loader was marked activated for a very very short amount of time (the actual loading so a few ms for most) and now it's marked active for a way longer time leading to the issue. Or maybe for the same amount of time but the loading occurs exactly at the time the fragment manager check for it before loading the others.
Won't be something easy to change I fear :(
What you can test in the loaderinbackground is a long thread sleep the issue should be back even when running from the handler.
I guess the root issue is that before the loader was marked activated for a very very short amount of time (the actual loading so a few ms for most) and now it's marked active for a way longer time leading to the issue. Or maybe for the same amount of time but the loading occurs exactly at the time the fragment manager check for it before loading the others.
Won't be something easy to change I fear :(
il...@google.com <il...@google.com> #10
There was pre-existing coupling between Loaders and FragmentManager:
1. FragmentManager wouldn't start new Fragments with userVisibleHint=false if current Fragments had running Loaders
2. LoaderManager would directly contact FragmentManager upon Loader completion to start any of those pending Fragments
With 27.1.0, #2 was removed, but #1 remained. This mismatch causes the issue.
While both removing #1 and re-instating #2 are options, we are going to proceed with removing #1 as that continues the goal to separate Loaders from the other APIs (it is not intuitive that adding Loaders affects the Fragment's lifecycle) and bring Loaders more in line with other APIs (such as ViewModels+LiveData).
1. FragmentManager wouldn't start new Fragments with userVisibleHint=false if current Fragments had running Loaders
2. LoaderManager would directly contact FragmentManager upon Loader completion to start any of those pending Fragments
With 27.1.0, #2 was removed, but #1 remained. This mismatch causes the issue.
While both removing #1 and re-instating #2 are options, we are going to proceed with removing #1 as that continues the goal to separate Loaders from the other APIs (it is not intuitive that adding Loaders affects the Fragment's lifecycle) and bring Loaders more in line with other APIs (such as ViewModels+LiveData).
to...@gmail.com <to...@gmail.com> #11
Thanks. Do you think it might worth extending the API to allow anyone who want that behavior for performance reasons to continue?
Like a way to query if there's active loaders for visible fragments and a listener for their end? With that and the already existing isVisibleToUser this would allow anyone to keep that.
Or maybe even better, to be more generic a busy indicator for visible fragments, with a listener when nothing busy. To handle that even with normal viewmodels. (Then it would be up to the user to manually toggle that in oncreate / onfinished)
I know one can use a viewmodel + livedata tied to activity to handle that, but native support for that, to allow anyone to easily priorise the visible fragment with an easy way to do something when the visible fragment have done it's delayed init would be really a nice addition.
It's tons of boiler plate code to achieve that currently and prone to mistakes.
Like a way to query if there's active loaders for visible fragments and a listener for their end? With that and the already existing isVisibleToUser this would allow anyone to keep that.
Or maybe even better, to be more generic a busy indicator for visible fragments, with a listener when nothing busy. To handle that even with normal viewmodels. (Then it would be up to the user to manually toggle that in oncreate / onfinished)
I know one can use a viewmodel + livedata tied to activity to handle that, but native support for that, to allow anyone to easily priorise the visible fragment with an easy way to do something when the visible fragment have done it's delayed init would be really a nice addition.
It's tons of boiler plate code to achieve that currently and prone to mistakes.
il...@google.com <il...@google.com> #12
As AsyncTaskLoader already limits its upper bound of running threads based on the number of cores (see https://developer.android.com/reference/android/os/AsyncTask.html#THREAD_POOL_EXECUTOR ), much of what you are requesting is done already when it comes to Loaders.
Capping the number of threads / concurrent operations should be something your repository/backend layer should already try to take into account (for example, Room already does this).
Please file separate feature requests for new API proposals.
Capping the number of threads / concurrent operations should be something your repository/backend layer should already try to take into account (for example, Room already does this).
Please file separate feature requests for new API proposals.
il...@google.com <il...@google.com> #13
As a note: one possible workaround is to move your initLoader call to onStart() - as Loaders do no work until onStart anyways, this is functionally equivalent to doing it in onActivityCreated().
It should also be noted that calling restartLoader in any lifecycle method is almost always the wrong choice as that negates the whole purpose of Loaders (restartLoader effectively throws away any results that were previously computed). restartLoader should only be called when your arguments to the Loader have changed.
It should also be noted that calling restartLoader in any lifecycle method is almost always the wrong choice as that negates the whole purpose of Loaders (restartLoader effectively throws away any results that were previously computed). restartLoader should only be called when your arguments to the Loader have changed.
to...@gmail.com <to...@gmail.com> #14
Well you missunderstood it's about the current functionality that will be lost.
Currently active fragment will load, then finish it's load then other offscreen fragments will start their loading. This is not related to threads limits or anything, this is being sure active fragment have more ressources and load before the unseen ones.
If you remove #1 then all fragments will start at the same time, meaning all loaders too, without any guaranteed order. Imagine a 1 core device (or multiple cores but multiple loaders per fragment) then without luck the offscreen fragments will load before the main that will be delayed until a thread is free.
Providing an API to allow users to keep that way of acting for low end devices or heavy / long loaders should be thought before removing #1. I was just proposing a more global approach as alternative, but at least there's need to have something that ensure active fragments loaders start first to avoid problems.
Currently active fragment will load, then finish it's load then other offscreen fragments will start their loading. This is not related to threads limits or anything, this is being sure active fragment have more ressources and load before the unseen ones.
If you remove #1 then all fragments will start at the same time, meaning all loaders too, without any guaranteed order. Imagine a 1 core device (or multiple cores but multiple loaders per fragment) then without luck the offscreen fragments will load before the main that will be delayed until a thread is free.
Providing an API to allow users to keep that way of acting for low end devices or heavy / long loaders should be thought before removing #1. I was just proposing a more global approach as alternative, but at least there's need to have something that ensure active fragments loaders start first to avoid problems.
to...@gmail.com <to...@gmail.com> #15
About your side note: Calling restart on return from a child fragment that can have modified any number of data if often wanted and normal.
Specially when we are dealing with those viewpager and child fragments.
ViewPager backed by cursor, click open detail views that you can swipe to see any number of details, in each details view you can change things -> on reenter to viewpager fragment you restart the loader to have the uptodate data, even if the query have not changed.
Specially when we are dealing with those viewpager and child fragments.
ViewPager backed by cursor, click open detail views that you can swipe to see any number of details, in each details view you can change things -> on reenter to viewpager fragment you restart the loader to have the uptodate data, even if the query have not changed.
il...@google.com <il...@google.com> #16
Fragments with setUserVisibleHint=true will still start before those with it set to false (it is a two step process). While the Loader specific delay between the two steps is being removed, the ordering of creation/starting is not.
to...@gmail.com <to...@gmail.com> #17
Can't edit but viewpager = recyclerview :)
to...@gmail.com <to...@gmail.com> #18
Ok thanks for the detail.
Will this be released as a fast .X release or in the next normal scheduled cycle? (Just to have an idea if I should already start again using it to find other issues or have time for that).
Will this be released as a fast .X release or in the next normal scheduled cycle? (Just to have an idea if I should already start again using it to find other issues or have time for that).
il...@google.com <il...@google.com> #19
This has been fixed for the next release, almost certainly as part of a fast .X release
to...@gmail.com <to...@gmail.com> #20
So doing some more tests I see other differences in loaders behavior from before :(
Like onLoadFinished is being called when returning to fragment when it was not the case before (before any call to reload or init made)
And much more important a change in how onLoaderReset is now called.
The documentation of restartLoader Says: "If a loader with the same id has previously been started it will automatically be destroyed when the new loader completes its work. The callback will be delivered before the old loader is destroyed."
But new implementation does not that at all. It calls onLoaderReset then onCreateLoader then onLoadFinished. (Previous implementation was sometimes broken and never called onLoaderReset but at least there was no long duration where we have no data to display).
This is a major change because this means you can't keep the previous data during the load as it's invalidated but you have nothing to display since the new loader is not finished. Meaning empty screen on reload = quite bad UX.
Like onLoadFinished is being called when returning to fragment when it was not the case before (before any call to reload or init made)
And much more important a change in how onLoaderReset is now called.
The documentation of restartLoader Says: "If a loader with the same id has previously been started it will automatically be destroyed when the new loader completes its work. The callback will be delivered before the old loader is destroyed."
But new implementation does not that at all. It calls onLoaderReset then onCreateLoader then onLoadFinished. (Previous implementation was sometimes broken and never called onLoaderReset but at least there was no long duration where we have no data to display).
This is a major change because this means you can't keep the previous data during the load as it's invalidated but you have nothing to display since the new loader is not finished. Meaning empty screen on reload = quite bad UX.
ta...@gmail.com <ta...@gmail.com> #21
The onLoaderReset problem described in #20 makes working with CursorLoader and CursorAdapter suddenly very cumbersome in 27.1.0.
In order to avoid the "empty data" period of time after onLoaderReset, my extremely hacky workaround is to create a second identical loader, with a separate Loader ID, and flip between the two Loaders with each call to restartLoader, so that the previous Cursor remains alive until the next onLoadFinished callback.
In order to avoid the "empty data" period of time after onLoaderReset, my extremely hacky workaround is to create a second identical loader, with a separate Loader ID, and flip between the two Loaders with each call to restartLoader, so that the previous Cursor remains alive until the next onLoadFinished callback.
to...@gmail.com <to...@gmail.com> #22
ja...@gmail.com <ja...@gmail.com> #23
Before 27.1.0, loaders initialized with getSupportLoaderManager() used to receive a call to onStartLoading() while the activity was executing super.onStart(). This allowed synchronous loaders to initialize resources before the remaining body of onStart() executed. In 27.1.0, loaders no longer start loading until some time after onStart(). This change caused a few NPEs in our onStart() methods. Is this 27.1.0 behavior a violation of the "between onStart/onStop" contract?
I thinkhttps://issuetracker.google.com/issues/73976255#comment13 is referencing this same guarantee which appears to be broken.
I think
ja...@gmail.com <ja...@gmail.com> #24
With a bit more digging I found https://issuetracker.google.com/issues/74225064 which appears to reference and fix the exact issue that I mentioned in #23. Sorry for the noise.
sa...@gmail.com <sa...@gmail.com> #25
please update your support libraries with 27.1.1
kk...@google.com <kk...@google.com> #26
This bug has been fixed in Support Library 27.1.1.
sa...@gmail.com <sa...@gmail.com> #27
(Program type already present: android.arch.lifecycle.LiveData$LifecycleBoundObserver Message{kind=ERROR, text=Program type already present: android.arch.lifecycle.LiveData$LifecycleBoundObserver, sources=[Unknown source file], tool name=Optional.of(D8)})
solv this problam
solv this problam
Description
Since support library 27.1.0 when using a FragmentStatePagerAdapter in a viewPager the offscreen fragment lifecycle have changed.
Before onStart / onResume where called on those fragment and it's no more the case. The offscreen fragments have the lifecycle stopping at onActivityCreated.
After a screen rotation those lifecycle are called correctly. So it's more a bug on first creation of the fragments than a wanted API change it seems.
If it's a normal wanted API change please confirm.