From 1c227e81e4fb53323633ee8da75674760b86e39a Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Mon, 4 Mar 2019 14:40:46 -0300
Subject: [PATCH] [android] update unread counts with a ScrollListener in
 threaded conversations

---
 .../threaded/BaseThreadItemViewHolder.java    |  1 -
 .../android/threaded/ThreadItemAdapter.java   | 40 +--------
 .../android/threaded/ThreadListActivity.java  | 59 ++-----------
 .../threaded/ThreadScrollListener.java        | 85 +++++++++++++++++++
 .../android/util/ItemReturningAdapter.java    |  9 ++
 .../view/BriarRecyclerViewScrollListener.java | 80 +++++++++++++++++
 6 files changed, 184 insertions(+), 90 deletions(-)
 create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java
 create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/util/ItemReturningAdapter.java
 create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/view/BriarRecyclerViewScrollListener.java

diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java
index 529fa7d00..a0f54bad3 100644
--- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java
+++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java
@@ -51,7 +51,6 @@ public abstract class BaseThreadItemViewHolder<I extends ThreadItem>
 		} else if (!item.isRead()) {
 			layout.setActivated(true);
 			animateFadeOut();
-			listener.onUnreadItemVisible(item);
 		} else {
 			layout.setActivated(false);
 		}
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java
index 15bd410e6..c9af77016 100644
--- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java
+++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java
@@ -10,6 +10,7 @@ import android.view.ViewGroup;
 
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.briar.R;
+import org.briarproject.briar.android.util.ItemReturningAdapter;
 import org.briarproject.briar.android.util.VersionedAdapter;
 
 import java.util.Collection;
@@ -21,7 +22,7 @@ import static android.support.v7.widget.RecyclerView.NO_POSITION;
 @UiThread
 public class ThreadItemAdapter<I extends ThreadItem>
 		extends RecyclerView.Adapter<BaseThreadItemViewHolder<I>>
-		implements VersionedAdapter {
+		implements VersionedAdapter, ItemReturningAdapter<I> {
 
 	static final int UNDEFINED = -1;
 
@@ -136,30 +137,6 @@ public class ThreadItemAdapter<I extends ThreadItem>
 		return null;
 	}
 
-	/**
-	 * Gets the number of unread items above and below the current view port.
-	 *
-	 * Attention: Do not call this when the list is still scrolling,
-	 *            because then the view port is unknown.
-	 */
-	public UnreadCount getUnreadCount() {
-		int positionTop = layoutManager.findFirstVisibleItemPosition();
-		int positionBottom = layoutManager.findLastVisibleItemPosition();
-		if (positionTop == NO_POSITION && positionBottom == NO_POSITION)
-			return new UnreadCount(0, 0);
-
-		int unreadCounterTop = 0, unreadCounterBottom = 0;
-		for (int i = 0; i < items.size(); i++) {
-			I item = items.get(i);
-			if (i < positionTop && !item.isRead()) {
-				unreadCounterTop++;
-			} else if (i > positionBottom && !item.isRead()) {
-				unreadCounterBottom++;
-			}
-		}
-		return new UnreadCount(unreadCounterTop, unreadCounterBottom);
-	}
-
 	/**
 	 * Returns the position of the first unread item below the current viewport
 	 */
@@ -188,20 +165,7 @@ public class ThreadItemAdapter<I extends ThreadItem>
 		return NO_POSITION;
 	}
 
-	static class UnreadCount {
-
-		final int top, bottom;
-
-		private UnreadCount(int top, int bottom) {
-			this.top = top;
-			this.bottom = bottom;
-		}
-	}
-
 	public interface ThreadItemListener<I> {
-
-		void onUnreadItemVisible(I item);
-
 		void onReplyClick(I item);
 	}
 
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java
index ed0ac5b94..c679ce2ad 100644
--- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java
+++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java
@@ -9,7 +9,6 @@ import android.support.annotation.UiThread;
 import android.support.design.widget.Snackbar;
 import android.support.v7.app.ActionBar;
 import android.support.v7.widget.LinearLayoutManager;
-import android.support.v7.widget.RecyclerView;
 import android.view.MenuItem;
 
 import org.briarproject.bramble.api.contact.ContactId;
@@ -43,10 +42,7 @@ import javax.inject.Inject;
 
 import static android.support.design.widget.Snackbar.make;
 import static android.support.v7.widget.RecyclerView.NO_POSITION;
-import static android.support.v7.widget.RecyclerView.SCROLL_STATE_IDLE;
-import static java.util.logging.Level.INFO;
 import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty;
-import static org.briarproject.briar.android.threaded.ThreadItemAdapter.UnreadCount;
 
 @MethodsNotNullByDefault
 @ParametersNotNullByDefault
@@ -61,11 +57,11 @@ public abstract class ThreadListActivity<G extends NamedGroup, I extends ThreadI
 			Logger.getLogger(ThreadListActivity.class.getName());
 
 	protected A adapter;
+	private ThreadScrollListener<I> scrollListener;
 	protected BriarRecyclerView list;
 	private LinearLayoutManager layoutManager;
 	protected TextInputView textInput;
 	protected GroupId groupId;
-	private UnreadMessageButton upButton, downButton;
 	@Nullable
 	private MessageId replyId;
 
@@ -76,7 +72,6 @@ public abstract class ThreadListActivity<G extends NamedGroup, I extends ThreadI
 
 	@CallSuper
 	@Override
-	@SuppressWarnings("ConstantConditions")
 	public void onCreate(@Nullable Bundle state) {
 		super.onCreate(state);
 
@@ -93,37 +88,18 @@ public abstract class ThreadListActivity<G extends NamedGroup, I extends ThreadI
 				new TextSendController(textInput, this, false);
 		textInput.setSendController(sendController);
 		textInput.setMaxTextLength(getMaxTextLength());
+		UnreadMessageButton upButton = findViewById(R.id.upButton);
+		UnreadMessageButton downButton = findViewById(R.id.downButton);
+
 		list = findViewById(R.id.list);
 		layoutManager = new LinearLayoutManager(this);
-		// FIXME pre-fetching messes with read state, find better solution #1289
-		layoutManager.setItemPrefetchEnabled(false);
 		list.setLayoutManager(layoutManager);
 		adapter = createAdapter(layoutManager);
 		list.setAdapter(adapter);
+		scrollListener = new ThreadScrollListener<>(adapter, getController(),
+				upButton, downButton);
+		list.getRecyclerView().addOnScrollListener(scrollListener);
 
-		list.getRecyclerView().addOnScrollListener(
-				new RecyclerView.OnScrollListener() {
-					@Override
-					public void onScrolled(RecyclerView recyclerView, int dx,
-							int dy) {
-						super.onScrolled(recyclerView, dx, dy);
-						if (dx == 0 && dy == 0) {
-							// scrollToPosition has been called and finished
-							updateUnreadCount();
-						}
-					}
-
-					@Override
-					public void onScrollStateChanged(RecyclerView recyclerView,
-							int newState) {
-						super.onScrollStateChanged(recyclerView, newState);
-						if (newState == SCROLL_STATE_IDLE) {
-							updateUnreadCount();
-						}
-					}
-				});
-		upButton = findViewById(R.id.upButton);
-		downButton = findViewById(R.id.downButton);
 		upButton.setOnClickListener(v -> {
 			int position = adapter.getVisibleUnreadPosTop();
 			if (position != NO_POSITION) {
@@ -211,7 +187,6 @@ public abstract class ThreadListActivity<G extends NamedGroup, I extends ThreadI
 		MessageId messageId = items.getFirstVisibleItemId();
 		if (messageId != null)
 			adapter.setItemWithIdVisible(messageId);
-		updateUnreadCount();
 		list.showData();
 	}
 
@@ -280,14 +255,6 @@ public abstract class ThreadListActivity<G extends NamedGroup, I extends ThreadI
 		}
 	}
 
-	@Override
-	public void onUnreadItemVisible(I item) {
-		if (!item.isRead()) {
-			item.setRead(true);
-			getController().markItemRead(item);
-		}
-	}
-
 	@Override
 	public void onReplyClick(I item) {
 		replyId = item.getId();
@@ -403,18 +370,8 @@ public abstract class ThreadListActivity<G extends NamedGroup, I extends ThreadI
 		if (isLocal) {
 			scrollToItemAtTop(item);
 		} else {
-			updateUnreadCount();
+			scrollListener.updateUnreadButtons(layoutManager);
 		}
 	}
 
-	private void updateUnreadCount() {
-		UnreadCount unreadCount = adapter.getUnreadCount();
-		if (LOG.isLoggable(INFO)) {
-			LOG.info("Updating unread count: top=" + unreadCount.top +
-					" bottom=" + unreadCount.bottom);
-		}
-		upButton.setUnreadCount(unreadCount.top);
-		downButton.setUnreadCount(unreadCount.bottom);
-	}
-
 }
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java
new file mode 100644
index 000000000..d9f915048
--- /dev/null
+++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java
@@ -0,0 +1,85 @@
+package org.briarproject.briar.android.threaded;
+
+import android.support.v7.widget.LinearLayoutManager;
+
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.briar.android.view.BriarRecyclerViewScrollListener;
+import org.briarproject.briar.android.view.UnreadMessageButton;
+
+import java.util.logging.Logger;
+
+import static android.support.v7.widget.RecyclerView.NO_POSITION;
+import static java.util.Objects.requireNonNull;
+import static java.util.logging.Level.INFO;
+import static java.util.logging.Logger.getLogger;
+
+@NotNullByDefault
+class ThreadScrollListener<I extends ThreadItem>
+		extends BriarRecyclerViewScrollListener<ThreadItemAdapter<I>, I> {
+
+	private static final Logger LOG =
+			getLogger(ThreadScrollListener.class.getName());
+
+	private final ThreadListController<?, I> controller;
+	private final UnreadMessageButton upButton, downButton;
+
+	protected ThreadScrollListener(ThreadItemAdapter<I> adapter,
+			ThreadListController<?, I> controller,
+			UnreadMessageButton upButton,
+			UnreadMessageButton downButton) {
+		super(adapter);
+		this.controller = controller;
+		this.upButton = upButton;
+		this.downButton = downButton;
+	}
+
+	@Override
+	protected void onNewItemVisible(int firstVisible, int lastVisible,
+			int itemCount) {
+		super.onNewItemVisible(firstVisible, lastVisible, itemCount);
+		updateUnreadButtons(firstVisible, lastVisible, itemCount);
+	}
+
+	@Override
+	protected void onNewItemVisible(I item) {
+		if (!item.isRead()) {
+			item.setRead(true);
+			controller.markItemRead(item);
+		}
+	}
+
+	void updateUnreadButtons(LinearLayoutManager layoutManager) {
+		int firstVisible = layoutManager.findFirstVisibleItemPosition();
+		int lastVisible = layoutManager.findLastVisibleItemPosition();
+		int itemCount = adapter.getItemCount();
+		updateUnreadButtons(firstVisible, lastVisible, itemCount);
+	}
+
+	private void updateUnreadButtons(int firstVisible, int lastVisible,
+			int count) {
+		if (firstVisible == NO_POSITION && lastVisible == NO_POSITION) {
+			setUnreadButtons(0, 0);
+			return;
+		}
+		int unreadCounterFirst = 0, unreadCounterLast = 0;
+		for (int i = 0; i < count; i++) {
+			I item = requireNonNull(adapter.getItemAt(i));
+			if (i < firstVisible && !item.isRead()) {
+				unreadCounterFirst++;
+			} else if (i > lastVisible && !item.isRead()) {
+				unreadCounterLast++;
+			}
+		}
+		setUnreadButtons(unreadCounterFirst, unreadCounterLast);
+	}
+
+	private void setUnreadButtons(int upCount, int downCount) {
+		if (LOG.isLoggable(INFO)) {
+			LOG.info("Updating unread count: top=" + upCount +
+					" bottom=" + downCount);
+		}
+		upButton.setUnreadCount(upCount);
+		downButton.setUnreadCount(downCount);
+	}
+
+}
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/util/ItemReturningAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/util/ItemReturningAdapter.java
new file mode 100644
index 000000000..6657c7c04
--- /dev/null
+++ b/briar-android/src/main/java/org/briarproject/briar/android/util/ItemReturningAdapter.java
@@ -0,0 +1,9 @@
+package org.briarproject.briar.android.util;
+
+public interface ItemReturningAdapter<I> {
+
+	I getItemAt(int position);
+
+	int getItemCount();
+
+}
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/BriarRecyclerViewScrollListener.java b/briar-android/src/main/java/org/briarproject/briar/android/view/BriarRecyclerViewScrollListener.java
new file mode 100644
index 000000000..fc7cf1214
--- /dev/null
+++ b/briar-android/src/main/java/org/briarproject/briar/android/view/BriarRecyclerViewScrollListener.java
@@ -0,0 +1,80 @@
+package org.briarproject.briar.android.view;
+
+import android.support.annotation.CallSuper;
+import android.support.v7.widget.LinearLayoutManager;
+import android.support.v7.widget.RecyclerView;
+import android.support.v7.widget.RecyclerView.OnScrollListener;
+
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.briar.android.util.ItemReturningAdapter;
+
+import static android.support.v7.widget.RecyclerView.NO_POSITION;
+import static java.util.Objects.requireNonNull;
+
+@NotNullByDefault
+public abstract class BriarRecyclerViewScrollListener<A extends ItemReturningAdapter<I>, I>
+		extends OnScrollListener {
+
+	protected final A adapter;
+
+	private int prevFirstVisible = NO_POSITION;
+	private int prevLastVisible = NO_POSITION;
+	private int prevItemCount = 0;
+
+	protected BriarRecyclerViewScrollListener(A adapter) {
+		this.adapter = adapter;
+	}
+
+	@Override
+	public void onScrolled(RecyclerView recyclerView, int dx, int dy) {
+		LinearLayoutManager layoutManager = (LinearLayoutManager)
+				requireNonNull(recyclerView.getLayoutManager());
+		int firstVisible = layoutManager.findFirstVisibleItemPosition();
+		int lastVisible = layoutManager.findLastVisibleItemPosition();
+		int itemCount = adapter.getItemCount();
+
+		if (firstVisible != prevFirstVisible ||
+				lastVisible != prevLastVisible ||
+				itemCount != prevItemCount) {
+			onNewItemVisible(firstVisible, lastVisible, itemCount);
+			prevFirstVisible = firstVisible;
+			prevLastVisible = lastVisible;
+			prevItemCount = itemCount;
+		}
+	}
+
+	@CallSuper
+	protected void onNewItemVisible(int firstVisible, int lastVisible,
+			int itemCount) {
+		boolean init = prevFirstVisible == NO_POSITION ||
+				prevLastVisible == NO_POSITION;
+		int visibleItems = prevLastVisible - prevFirstVisible;
+		boolean jump = Math.abs(firstVisible - prevFirstVisible) > visibleItems;
+
+		if (init || jump) {
+			// If the list has loaded, or we jump scrolled,
+			// all visible items are newly visible
+			for (int i = firstVisible; i <= lastVisible; i++)
+				onNewItemVisible(i);
+			return;
+		}
+
+		boolean up = firstVisible > prevFirstVisible ||
+				lastVisible > prevLastVisible;
+		if (up) {
+			for (int i = prevLastVisible + 1; i <= lastVisible; i++)
+				onNewItemVisible(i);
+		} else {
+			for (int i = firstVisible; i < prevFirstVisible; i++)
+				onNewItemVisible(i);
+		}
+	}
+
+	private void onNewItemVisible(int position) {
+		I item = requireNonNull(adapter.getItemAt(position));
+		onNewItemVisible(item);
+	}
+
+	protected abstract void onNewItemVisible(I item);
+
+}