Bug 1377287 - review: Only update views if they're the same as when asyncTask began. r=mcomella draft
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 26 Jul 2017 15:32:14 -0700
changeset 616316 fc0bc57794c54b7fce325437a7b080b7147d04db
parent 616315 c077833c1fb71be64a0283646d62aa77e9edc4d6
child 639435 55db25d986f65d8fbf0e5e15c471075b7932690e
push id70644
push usermichael.l.comella@gmail.com
push dateWed, 26 Jul 2017 22:32:58 +0000
reviewersmcomella
bugs1377287
milestone56.0a1
Bug 1377287 - review: Only update views if they're the same as when asyncTask began. r=mcomella MozReview-Commit-ID: 8XA32VFNSH7
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
@@ -2,16 +2,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.activitystream.homepanel.stream;
 
 import android.content.Context;
 import android.graphics.Color;
+import android.support.annotation.UiThread;
 import android.text.TextUtils;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.ImageView;
 import android.widget.TextView;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.Telemetry;
 import org.mozilla.gecko.TelemetryContract;
@@ -25,16 +26,17 @@ import org.mozilla.gecko.icons.IconRespo
 import org.mozilla.gecko.icons.Icons;
 import org.mozilla.gecko.util.DrawableUtil;
 import org.mozilla.gecko.util.TouchTargetUtil;
 import org.mozilla.gecko.util.URIUtils;
 import org.mozilla.gecko.util.ViewUtil;
 import org.mozilla.gecko.widget.FaviconView;
 
 import java.lang.ref.WeakReference;
+import java.util.UUID;
 import java.util.concurrent.Future;
 
 public class HighlightItem extends StreamItem implements IconCallback {
     private static final String LOGTAG = "GeckoHighlightItem";
 
     public static final int LAYOUT_ID = R.layout.activity_stream_card_history_item;
 
     private Highlight highlight;
@@ -153,29 +155,46 @@ public class HighlightItem extends Strea
 
     @Override
     public void onIconResponse(IconResponse response) {
         pageIconView.updateImage(response);
     }
 
     /** Updates the text of the given view to the host second level domain. */
     private static class UpdatePageDomainAsyncTask extends URIUtils.GetHostSecondLevelDomainAsyncTask {
+        private static final int VIEW_TAG_ID = R.id.page; // same as the view.
+
         private final WeakReference<TextView> pageDomainViewWeakReference;
+        private final UUID viewTagAtStart;
 
         UpdatePageDomainAsyncTask(final Context contextReference, final String uriString, final TextView pageDomainView) {
             super(contextReference, uriString);
             this.pageDomainViewWeakReference = new WeakReference<>(pageDomainView);
+
+            // See isTagSameAsStartTag for details.
+            viewTagAtStart = UUID.randomUUID();
+            pageDomainView.setTag(VIEW_TAG_ID, viewTagAtStart);
         }
 
         @Override
         protected void onPostExecute(final String hostSLD) {
             super.onPostExecute(hostSLD);
             final TextView viewToUpdate = pageDomainViewWeakReference.get();
-            if (viewToUpdate == null) {
+
+            if (viewToUpdate == null || !isTagSameAsStartTag(viewToUpdate)) {
                 return;
             }
 
             // hostSLD will be the empty String if the host cannot be determined. This is fine: it's very unlikely
             // and the page title will be URL if there's an error there so we wouldn't want to write the URL again here.
             viewToUpdate.setText(hostSLD);
         }
+
+        /**
+         * Returns true if the tag on the given view matches the tag from the constructor. We do this to ensure
+         * the View we're making this request for hasn't been re-used by the time this request completes.
+         */
+        @UiThread
+        private boolean isTagSameAsStartTag(final View viewToCheck) {
+            return viewTagAtStart.equals(viewToCheck.getTag(VIEW_TAG_ID));
+        }
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topsites/TopSitesCard.java
@@ -2,16 +2,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 package org.mozilla.gecko.activitystream.homepanel.topsites;
 
 import android.content.Context;
 import android.graphics.Color;
 import android.graphics.drawable.Drawable;
+import android.support.annotation.UiThread;
 import android.support.v4.widget.TextViewCompat;
 import android.support.v7.widget.RecyclerView;
 import android.text.TextUtils;
 import android.view.View;
 import android.widget.FrameLayout;
 import android.widget.ImageView;
 import android.widget.TextView;
 import org.mozilla.gecko.R;
@@ -26,16 +27,17 @@ import org.mozilla.gecko.icons.IconRespo
 import org.mozilla.gecko.icons.Icons;
 import org.mozilla.gecko.util.DrawableUtil;
 import org.mozilla.gecko.util.TouchTargetUtil;
 import org.mozilla.gecko.util.URIUtils;
 import org.mozilla.gecko.util.ViewUtil;
 import org.mozilla.gecko.widget.FaviconView;
 
 import java.lang.ref.WeakReference;
+import java.util.UUID;
 import java.util.concurrent.Future;
 
 /* package-local */ class TopSitesCard extends RecyclerView.ViewHolder
         implements IconCallback {
     private final FaviconView faviconView;
 
     private final TextView title;
     private final ImageView menuButton;
@@ -130,31 +132,47 @@ import java.util.concurrent.Future;
         final int tintColor = !response.hasColor() || response.getColor() == Color.WHITE ? Color.LTGRAY : Color.WHITE;
 
         menuButton.setImageDrawable(
                 DrawableUtil.tintDrawable(menuButton.getContext(), R.drawable.menu, tintColor));
     }
 
     /** Updates the text of the given view to the page domain. */
     private static class UpdateCardTitleAsyncTask extends URIUtils.GetHostSecondLevelDomainAsyncTask {
+        private static final int VIEW_TAG_ID = R.id.title; // same as the view.
+
         private final WeakReference<TextView> titleViewWeakReference;
+        private final UUID viewTagAtStart;
 
         UpdateCardTitleAsyncTask(final Context contextReference, final String uriString, final TextView titleView) {
             super(contextReference, uriString);
             this.titleViewWeakReference = new WeakReference<>(titleView);
+
+            // See isTagSameAsStartTag for details.
+            viewTagAtStart = UUID.randomUUID();
+            titleView.setTag(VIEW_TAG_ID, viewTagAtStart);
         }
 
         @Override
         protected void onPostExecute(final String hostSLD) {
             super.onPostExecute(hostSLD);
             final TextView titleView = titleViewWeakReference.get();
-            if (titleView == null) {
+            if (titleView == null || !isTagSameAsStartTag(titleView)) {
                 return;
             }
 
             final String updateText = !TextUtils.isEmpty(hostSLD) ? hostSLD : uriString;
 
             // We use consistent padding all around the title, and the top padding is never modified,
             // so we can pass that in as the default padding:
             ViewUtil.setCenteredText(titleView, updateText, titleView.getPaddingTop());
         }
+
+        /**
+         * Returns true if the tag on the given view matches the tag from the constructor. We do this to ensure
+         * the View we're making this request for hasn't been re-used by the time this request completes.
+         */
+        @UiThread
+        private boolean isTagSameAsStartTag(final View viewToCheck) {
+            return viewTagAtStart.equals(viewToCheck.getTag(VIEW_TAG_ID));
+        }
     }
 }