Fixed scrolling on refresh in the feed

[litetex]

What is it?

• Bugfix (user facing)
• Feature (user facing)
• Codebase improvement (dev facing)
• Meta improvement to the project (dev facing)

Description of the changes in your PR

• When the feed is refreshed, the grid will now always scroll to the top
• The problem only seems to occur when more items are added than removed

Before/After Screenshots/Screen Record

Click to open

Before:

Broken.mp4

After:

Fixed.mp4

Fixes the following issue(s)

• Fixes After Feed Update, Requires Manual Scroll Up to show Latest Videos #6650

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

• I read the contribution guidelines.

How to test

• Install the apk / Checkout the branch
• Set your phone / emulator system time to 2021-07-15 at 23:55 (you might need to reset it as the clock counts)
• Import this: NewPipeData-20210716_190308.zip
• Go to Feed/"What's New" and refresh it

→ It should now scroll to the top 🥳

[TobiGr]

Thank you! That looks good.
I found a little bug which should be easy to solve by introducing a variable which stores whether items were inserted due to refreshing/updating the feed or making hidden items visible. Steps to reproduce:

• Watch a few videos from the feed to the end (or skip to the end).
• Go the the feed
• Press the "eye" button to hide already watched streams
• scroll down
• Press the "eye" button to make the watched streams visible again
• Notice the scroll to the top

[timjefferies]

To me this was a feature not a bug. If you fix it I won't be able to tell which new videos where added to the feed.

Could there be a line separator to show which new videos are added since the last update?

[TobiGr]

To me this was a feature not a bug. If you fix it I won't be able to tell which new videos where added to the feed.

That's true. Maybe we can add a chip at the top saying ":arrow_up: X new streams" with an onclick listener which triggers the scroll to top.

[litetex]

I will rethink the concept when I have time again 😄

[litetex]

FYI:

I'm currently experimenting using new approaches e.g.:

Code is here.

However there seem to be some problems:

1. When a feed is refreshed there are (randomly) multiple SuccessEvents:

NewPipeFeedDuplicateSuccessEvents.mp4

This causes problems as it can't be computed what items are new ones.

@Stypox
I think you are the only person that a) is currently active and b) has experience with the FeedLoadService according to git blame.
Do you have any ideas?

2. I'm a bit unsure how to implement the "X new items" popup/baloon whatever as seems to be dynamically inflated.
   However that should be doable - at least somehow.

[Stypox]

@litetex I can't reproduce the double success event. Maybe you can try debugging in the only place where a success event is generated, i.e. here? If the problem is not there climb up the call stack until you find where the events are duplicated.

Below I try to climb up the call stack that led to the code you are analyzing in the video above:

• In FeedLoadService, i.e. the only other place where Android Studio says SuccessResultEvent is used, it seems like the success event is generated only once when onComplete() is called.
• onComplete() is in turn called by the huge ReactiveX chain here which seems to be a Flowable<MutableList<Notification<Pair<Long, ListInfo<StreamInfoItem>>>>>, before subscribe(resultSubscriber) is called (try to add ; at the end of the last .observeOn(AndroidSchedulers.mainThread()), then Alt+Enter and "introduce local variable", then on the created variable Alt+Enter and "Specify type explicitly"). So a flowable should theoretically only call onComplete() once, i.e. when it has finished processing all items.
• Maybe then the reason is that startLoading() turns out to be called multiple times? But Android Studio says it's only called in FeedLoadService.onStartCommand.
• The documentation about onStartCommand says "Called by the system every time a client explicitly starts the service by calling Context.startService", which only seems to be happening in FeedFragment.reloadContent
• reloadContent() is overridden from BaseStateFragment and is called in multiple places, maybe the problem lies here?

I'm a bit unsure how to implement the "X new items" popup/baloon whatever as seems to be dynamically inflated.

I think you will need to add a new item holder to work with groupie

[litetex]

@Stypox
Thank you for the detailed feedback.

Disclaimer: The following text contains 2.5 hours of research so it got a bit longer...

However none of the above code seems to be the cause of the error.
I'm to 95% sure that it's some kind of problem with the database-events from streamItems.

I think to trigger the problem at least some new feed-items have to be fetched/added and some old ones to be removed.

Because when I remove

NewPipe/app/src/main/java/org/schabi/newpipe/local/feed/FeedViewModel.kt

Line 55 in be676ad

.throttleLatest(DEFAULT_THROTTLE_TIMEOUT, TimeUnit.MILLISECONDS)

I get event more events.
Most of them (however sometimes not all) have different List<StreamWithState>-sizes.

Usually it's 77->114->67 (last 2 differ according to current time and subscriptions) on my setup.

That looks like something on the internal database is raising these events.
And I'm pretty sure that's the case because:

• I got 37 inserts when monitoring the upsert-function:

  NewPipe/app/src/main/java/org/schabi/newpipe/local/feed/FeedDatabaseManager.kt

  Line 98 in be676ad

  feedTable.insertAll(feedEntities)

  which matches exactly the 77->114 gap
• I got 47 deletions when monitoring the removeOrphansOrOlderStreams

  NewPipe/app/src/main/java/org/schabi/newpipe/local/feed/FeedDatabaseManager.kt

  Lines 107 to 108 in be676ad

  	feedTable.unlinkStreamsOlderThan(oldestAllowedDate)
  	streamTable.deleteOrphans()

  which matches exactly the 114->67 gap

So I guess the database events come in too late.
That would also explain why the problems are only randomly: Sometimes these 114, 67-events occur with the ProgressEvent("Processing feedback")-phase and sometimes with the SuccessResultEvent phase.

Source-Code I used: https://github.com/litetex/NewPipe/tree/e7e48aba35e18531800b004a9d82da5448de2d61
Full protocol: log.txt

Maybe it's also just a race condition because I'm debugging / using an emulator...

But I have at least one question regarding:

NewPipe/app/src/main/java/org/schabi/newpipe/local/feed/FeedViewModel.kt

Lines 33 to 38 in be676ad

	private val streamItems = toggleShowPlayedItems
	.startWithItem(initialShowPlayedItems)
	.distinctUntilChanged()
	.switchMap { showPlayedItems ->
	feedDatabaseManager.getStreams(groupId, showPlayedItems)
	}

Why is there a Flowable in the first place?
Should a change in the streamItems really cause further processing?

[Stypox]

_{Sorry I closed by accident}

Maybe it's also just a race condition because I'm debugging / using an emulator...

This is really strange. It didn't seem to happen to me, and I was debugging on an emulator, too. It could as well be a race condition that should be solved.

Why is there a Flowable in the first place?

It should in theory be there so that updates to showPlayedItems can be sent one after the other, but I'm not so sure

Should a change in the streamItems really cause further processing?

I am not sure I understand correctly, but I don't think changing something in those four lines would affect much performance.

[litetex]

Succeeded by #7050