From 9e932321a5ff6364747b89baadd5af0a613a7d69 Mon Sep 17 00:00:00 2001
From: maniac103 <dannybaumann@web.de>
Date: Tue, 11 Sep 2018 16:13:20 +0200
Subject: [PATCH] Fix image refresh regressions (#987)
* Re-bind all widgets after pull-to-refresh.
When not doing so, image views won't reload the respective images.
Signed-off-by: Danny Baumann <dannybaumann@web.de>
* Discard last image request when detaching view.
Doing so is important in two scenarios:
- View is detached, then reattached and bound to the same URL with refresh
In that case, the refresh was canceled in onDetachedFromWindow(), but
never resumed as the setImageUrl() call would break out early due to
old and new URL matching
- Widget list is refreshed
In that case, views are detached and reattached, but due to the URLs
being the same the load of the new image was never actually done.
Signed-off-by: Danny Baumann <dannybaumann@web.de>
* Next take on fixing image refresh.
Don't clear out last request when detaching view, so we can reuse it
after reattaching for resuming the refresh ... and start actually doing
the resume. Also make sure to create a request for the same purpose if
the image view was freshly created and the initial load request could be
satisfied from the cache.
Signed-off-by: Danny Baumann <dannybaumann@web.de>
* Fix image load 'has completed' logic.
The load is considered completed when there is *no* pending call around.
Also stop clearing out the call when canceling, as otherwise
hasCompleted() is always going to return true when reattaching views.
Signed-off-by: Danny Baumann <dannybaumann@web.de>
---
.../habdroid/ui/OpenHABWidgetAdapter.java | 4 +--
.../ui/OpenHABWidgetListFragment.java | 2 +-
.../habdroid/ui/widget/WidgetImageView.java | 34 +++++++++++--------
3 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/mobile/src/main/java/org/openhab/habdroid/ui/OpenHABWidgetAdapter.java b/mobile/src/main/java/org/openhab/habdroid/ui/OpenHABWidgetAdapter.java
index 85b0f26a..992038a3 100644
--- a/mobile/src/main/java/org/openhab/habdroid/ui/OpenHABWidgetAdapter.java
+++ b/mobile/src/main/java/org/openhab/habdroid/ui/OpenHABWidgetAdapter.java
@@ -131,10 +131,10 @@ public class OpenHABWidgetAdapter extends RecyclerView.Adapter<OpenHABWidgetAdap
mChartTheme = tv.string;
}
- public void update(List<OpenHABWidget> widgets) {
+ public void update(List<OpenHABWidget> widgets, boolean forceFullUpdate) {
boolean compatibleUpdate = true;
- if (widgets.size() != mItems.size()) {
+ if (widgets.size() != mItems.size() || forceFullUpdate) {
compatibleUpdate = false;
} else {
for (int i = 0; i < widgets.size(); i++) {
diff --git a/mobile/src/main/java/org/openhab/habdroid/ui/OpenHABWidgetListFragment.java b/mobile/src/main/java/org/openhab/habdroid/ui/OpenHABWidgetListFragment.java
index 6b7128c0..8a3c0f48 100644
--- a/mobile/src/main/java/org/openhab/habdroid/ui/OpenHABWidgetListFragment.java
+++ b/mobile/src/main/java/org/openhab/habdroid/ui/OpenHABWidgetListFragment.java
@@ -293,7 +293,7 @@ public class OpenHABWidgetListFragment extends Fragment
mWidgets = widgets;
if (openHABWidgetAdapter != null) {
- openHABWidgetAdapter.update(widgets);
+ openHABWidgetAdapter.update(widgets, refreshLayout.isRefreshing());
setHighlightedPageLink(mHighlightedPageLink);
refreshLayout.setRefreshing(false);
}
diff --git a/mobile/src/main/java/org/openhab/habdroid/ui/widget/WidgetImageView.java b/mobile/src/main/java/org/openhab/habdroid/ui/widget/WidgetImageView.java
index f4b527e5..7e9d8a21 100644
--- a/mobile/src/main/java/org/openhab/habdroid/ui/widget/WidgetImageView.java
+++ b/mobile/src/main/java/org/openhab/habdroid/ui/widget/WidgetImageView.java
@@ -109,21 +109,23 @@ public class WidgetImageView extends AppCompatImageView {
AsyncHttpClient client = connection.getAsyncHttpClient();
HttpUrl actualUrl = client.buildUrl(url);
- if (mLastRequest != null && mLastRequest.isForUrl(actualUrl)) {
- // Nothing to do
+ if (mLastRequest != null && mLastRequest.isActiveForUrl(actualUrl)) {
+ // We're already in the process of loading this image, thus there's nothing to do
return;
}
cancelCurrentLoad();
- // Make sure to discard last request (which was for a different URL) to ensure
- // it's not re-triggered later, e.g. when being attached to the window
- mLastRequest = null;
if (actualUrl == null) {
applyFallbackDrawable();
+ mLastRequest = null;
return;
}
+
Bitmap cached = CacheManager.getInstance(getContext()).getCachedBitmap(actualUrl);
+
+ mLastRequest = new HttpImageRequest(client, actualUrl, timeoutMillis);
+
if (cached != null) {
setBitmapInternal(cached);
} else {
@@ -131,7 +133,6 @@ public class WidgetImageView extends AppCompatImageView {
}
if (cached == null || forceLoad) {
- mLastRequest = new HttpImageRequest(client, actualUrl, timeoutMillis);
mLastRequest.execute(forceLoad);
}
}
@@ -188,8 +189,12 @@ public class WidgetImageView extends AppCompatImageView {
@Override
protected void onAttachedToWindow() {
super.onAttachedToWindow();
- if (mLastRequest != null && !mLastRequest.hasCompleted()) {
- mLastRequest.execute(false);
+ if (mLastRequest != null) {
+ if (!mLastRequest.hasCompleted()) {
+ mLastRequest.execute(false);
+ } else {
+ scheduleNextRefresh();
+ }
}
}
@@ -288,10 +293,6 @@ public class WidgetImageView extends AppCompatImageView {
mCall = null;
}
- public boolean hasCompleted() {
- return mCall != null;
- }
-
public void execute(boolean avoidCache) {
Log.i(TAG, "Refreshing image at " + mUrl);
HttpClient.CachingMode cachingMode = avoidCache
@@ -303,12 +304,15 @@ public class WidgetImageView extends AppCompatImageView {
public void cancel() {
if (mCall != null) {
mCall.cancel();
- mCall = null;
}
}
- public boolean isForUrl(HttpUrl url) {
- return mCall != null && mCall.request().url().equals(url);
+ public boolean hasCompleted() {
+ return mCall == null;
+ }
+
+ public boolean isActiveForUrl(HttpUrl url) {
+ return mCall != null && mCall.request().url().equals(url) && !mCall.isCanceled();
}
}
}
--
GitLab