From 5f4e1ecdfd7da00a723de0b8f4de706fa63fe8e1 Mon Sep 17 00:00:00 2001
From: Ernir Erlingsson <ernir@ymirmobile.com>
Date: Mon, 24 Apr 2017 23:34:34 +0200
Subject: [PATCH] improvements after code review #1

fix
---
 .../android/threaded/ThreadItemAdapter.java   | 26 +++++++++----------
 .../android/threaded/ThreadListActivity.java  |  5 ++--
 .../threaded/ThreadListController.java        |  6 ++---
 .../threaded/ThreadListControllerImpl.java    | 13 +++-------
 .../briar/client/MessageTrackerImpl.java      |  2 +-
 5 files changed, 22 insertions(+), 30 deletions(-)

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 9e4bbe51e..0a36ac80e 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
@@ -66,31 +66,29 @@ public class ThreadItemAdapter<I extends ThreadItem>
 		revision++;
 	}
 
-	void setBottomItem(MessageId messageId) {
+	// Useful when the adapter has not calculated the dimension yet
+	void postSetItemWithIdVisible(@Nullable final MessageId messageId) {
+		new Handler().post(new Runnable() {
+			@Override
+			public void run() {
+				setItemWithIdVisible(messageId);
+			}
+		});
+	}
+
+	void setItemWithIdVisible(@Nullable MessageId messageId) {
 		if (messageId != null) {
 			int pos = 0;
 			for (I item : items) {
 				if (item.getId().equals(messageId)) {
-					scrollToPosition(pos);
+					layoutManager.scrollToPosition(pos);
 					break;
-
 				}
 				pos++;
 			}
 		}
 	}
 
-	private void scrollToPosition(final int pos) {
-		// Post call ensures that the list scrolls AFTER it has been propagated
-		// and the layout has been calculated.
-		handler.post(new Runnable() {
-			@Override
-			public void run() {
-				layoutManager.scrollToPosition(pos);
-			}
-		});
-	}
-
 	public void setItems(Collection<I> items) {
 		this.items.clear();
 		this.items.addAll(items);
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 dc184365e..68cec97a2 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
@@ -148,7 +148,8 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI
 	}
 
 	@Override
-	public MessageId getBottomVisibleMessageId() {
+	@Nullable
+	public MessageId getLastVisibleMessageId() {
 		if (layoutManager != null && adapter != null) {
 			int position =
 					layoutManager.findLastCompletelyVisibleItemPosition();
@@ -190,7 +191,7 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI
 								list.showData();
 							} else {
 								adapter.setItems(items);
-								adapter.setBottomItem(
+								adapter.postSetItemWithIdVisible(
 										items.getBottomVisibleItemId());
 								list.showData();
 								updateTextInput(replyId);
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListController.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListController.java
index a062c8322..45aa69a0a 100644
--- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListController.java
+++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListController.java
@@ -42,7 +42,7 @@ public interface ThreadListController<G extends NamedGroup, I extends ThreadItem
 
 	void deleteNamedGroup(ExceptionHandler<DbException> handler);
 
-	interface ThreadListListener<H> extends DestroyableContext {
+	interface ThreadListListener<H> extends ThreadListDataSource {
 		@UiThread
 		void onHeaderReceived(H header);
 
@@ -53,10 +53,10 @@ public interface ThreadListController<G extends NamedGroup, I extends ThreadItem
 		void onInvitationAccepted(ContactId c);
 	}
 
-	interface ThreadListDataSource {
+	interface ThreadListDataSource extends DestroyableContext {
 
 		@UiThread @Nullable
-		MessageId getBottomVisibleMessageId();
+		MessageId getLastVisibleMessageId();
 	}
 
 }
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListControllerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListControllerImpl.java
index 94b880f9d..c61b35458 100644
--- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListControllerImpl.java
+++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListControllerImpl.java
@@ -59,7 +59,6 @@ public abstract class ThreadListControllerImpl<G extends NamedGroup, I extends T
 	protected volatile L listener;
 	@Inject
 	MessageTracker messageTracker;
-	private ThreadListDataSource source;
 
 	protected ThreadListControllerImpl(@DatabaseExecutor Executor dbExecutor,
 			LifecycleManager lifecycleManager, IdentityManager identityManager,
@@ -83,13 +82,6 @@ public abstract class ThreadListControllerImpl<G extends NamedGroup, I extends T
 	@Override
 	public void onActivityCreate(Activity activity) {
 		listener = (L) activity;
-		if (activity instanceof ThreadListDataSource) {
-			source = (ThreadListDataSource) activity;
-		} else {
-			throw new ClassCastException(
-					"Activity " + activity.getClass().getSimpleName() +
-							" must implement ThreadListDataSource");
-		}
 	}
 
 	@CallSuper
@@ -111,9 +103,10 @@ public abstract class ThreadListControllerImpl<G extends NamedGroup, I extends T
 		try {
 			messageTracker
 					.storeMessageId(groupId,
-							source.getBottomVisibleMessageId());
+							listener.getLastVisibleMessageId());
 		} catch (DbException e) {
-			e.printStackTrace();
+			if (LOG.isLoggable(WARNING))
+				LOG.log(WARNING, e.toString(), e);
 		}
 	}
 
diff --git a/briar-core/src/main/java/org/briarproject/briar/client/MessageTrackerImpl.java b/briar-core/src/main/java/org/briarproject/briar/client/MessageTrackerImpl.java
index 716b1308e..18e034027 100644
--- a/briar-core/src/main/java/org/briarproject/briar/client/MessageTrackerImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/client/MessageTrackerImpl.java
@@ -65,7 +65,7 @@ class MessageTrackerImpl implements MessageTracker {
 		try {
 			BdfDictionary d = clientHelper.getGroupMetadataAsDictionary(g);
 			byte[] msgBytes = d.getOptionalRaw(GROUP_KEY_STORED_MESSAGE_ID);
-			return msgBytes != null? new MessageId(msgBytes) : null;
+			return msgBytes != null ? new MessageId(msgBytes) : null;
 		} catch (FormatException e) {
 			throw new DbException(e);
 		}