fix(youtube/return-youtube-dislike): stale dislike data shown after opening / closing the app during shorts playback (#356)

app/src/main/java/app/revanced/integrations/patches/ReturnYouTubeDislikePatch.java

@@ -6,7 +6,7 @@ import app.revanced.integrations.returnyoutubedislike.ReturnYouTubeDislike;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
- * Used by app.revanced.patches.youtube.layout.returnyoutubedislike.patch.ReturnYouTubeDislikePatch
+ * TODO: delete this empty class, and point the patch to {@link ReturnYouTubeDislike}
  */
 public class ReturnYouTubeDislikePatch {
 

app/src/main/java/app/revanced/integrations/returnyoutubedislike/ReturnYouTubeDislike.java

@@ -97,7 +97,7 @@ public class ReturnYouTubeDislike {
      */
     @Nullable
     @GuardedBy("videoIdLockObject")
-    private static Spanned replacementLikeDislikeSpan;
+    private static SpannableString replacementLikeDislikeSpan;
 
     public enum Vote {
         LIKE(1),
@@ -147,6 +147,18 @@ public class ReturnYouTubeDislike {
         }
     }
 
+    /**
+     * Should be called if user changes settings for dislikes appearance.
+     */
+    public static void clearCache() {
+        synchronized (videoIdLockObject) {
+            if (replacementLikeDislikeSpan != null) {
+                LogHelper.printDebug(() -> "Clearing cache");
+            }
+            replacementLikeDislikeSpan = null;
+        }
+    }
+
     @Nullable
     private static String getCurrentVideoId() {
         synchronized (videoIdLockObject) {
@@ -184,6 +196,12 @@ public class ReturnYouTubeDislike {
                 }
                 LogHelper.printDebug(() -> "New video loaded: " + videoId + " playerType: " + currentPlayerType);
                 setCurrentVideoId(videoId);
+
+                // If a Short is opened while a regular video is on screen, this will incorrectly set this as false.
+                // But this check is needed to fix unusual situations of opening/closing the app
+                // while both a regular video and a short are on screen.
+                lastVideoLoadedWasShort = PlayerType.getCurrent().isNoneOrHidden();
+
                 // no need to wrap the call in a try/catch,
                 // as any exceptions are propagated out in the later Future#Get call
                 voteFetchFuture = ReVancedUtils.submitOnBackgroundThread(() -> ReturnYouTubeDislikeApi.fetchVotes(videoId));
@@ -194,15 +212,18 @@ public class ReturnYouTubeDislike {
     }
 
     /**
+     * Called when a litho text component is created.
+     *
      * This method is sometimes called on the main thread, but it usually is called _off_ the main thread.
      * This method can be called multiple times for the same UI element (including after dislikes was added)
+     *
+     * @param textRef atomic reference should always be non null, but the spanned reference inside can be null.
      */
     public static void onComponentCreated(@NonNull Object conversionContext, @NonNull AtomicReference<Object> textRef) {
         try {
             if (!SettingsEnum.RYD_ENABLED.getBoolean()) return;
 
-            // do not set lastVideoLoadedWasShort to false. It will be cleared when the next regular video is loaded.
-            if (lastVideoLoadedWasShort || PlayerType.getCurrent().isNoneOrHidden()) {
+            if (PlayerType.getCurrent().isNoneOrHidden()) {
                 return;
             }
 
@@ -216,6 +237,16 @@ public class ReturnYouTubeDislike {
                 return;
             }
 
+            if (lastVideoLoadedWasShort) {
+                // user:
+                // 1, opened a video
+                // 2. opened a short (without closing the regular video)
+                // 3. closed the short
+                // 4. regular video is now present, but the videoId and RYD data is still for the short
+                LogHelper.printDebug(() -> "Ignoring onComponentCreated(), as data loaded is is for prior short");
+                return;
+            }
+
             Spanned replacement = waitForFetchAndUpdateReplacementSpan((Spanned) textRef.get(), isSegmentedButton);
             if (replacement != null) {
                 textRef.set(replacement);
@@ -225,11 +256,14 @@ public class ReturnYouTubeDislike {
         }
     }
 
-    public static Spanned onShortsComponentCreated(Spanned span) {
+    /**
+     * Called when a Shorts dislike Spannable is created.
+     */
+    public static Spanned onShortsComponentCreated(Spanned original) {
         try {
             if (SettingsEnum.RYD_ENABLED.getBoolean()) {
-                lastVideoLoadedWasShort = true;
-                Spanned replacement = waitForFetchAndUpdateReplacementSpan(span, false);
+                lastVideoLoadedWasShort = true; // it's now certain the video and data are a short
+                Spanned replacement = waitForFetchAndUpdateReplacementSpan(original, false);
                 if (replacement != null) {
                     return replacement;
                 }
@@ -237,7 +271,7 @@ public class ReturnYouTubeDislike {
         } catch (Exception ex) {
             LogHelper.printException(() -> "onShortsComponentCreated failure", ex);
         }
-        return span;
+        return original;
     }
 
     // alternatively, this could check if the span contains one of the custom created spans, but this is simple and quick
@@ -249,31 +283,27 @@ public class ReturnYouTubeDislike {
      * @return NULL if the span does not need changing or if RYD is not available
      */
     @Nullable
-    private static Spanned waitForFetchAndUpdateReplacementSpan(@Nullable Spanned oldSpannable, boolean isSegmentedButton) {
+    private static SpannableString waitForFetchAndUpdateReplacementSpan(@Nullable Spanned oldSpannable, boolean isSegmentedButton) {
         if (oldSpannable == null) {
             LogHelper.printDebug(() -> "Cannot add dislikes (injection code was called with null Span)");
             return null;
         }
-        // Must block the current thread until fetching is done
-        // There's no known way to edit the text after creation yet
-        long fetchStartTime = 0;
         try {
             synchronized (videoIdLockObject) {
                 if (oldSpannable.equals(replacementLikeDislikeSpan)) {
-                    LogHelper.printDebug(() -> "Ignoring previously created dislike span");
+                    LogHelper.printDebug(() -> "Ignoring span that already contains dislikes");
                     return null;
                 }
+                if (replacementLikeDislikeSpan != null) {
+                    LogHelper.printDebug(() -> "Using previously created dislike span");
+                    return replacementLikeDislikeSpan;
+                }
                 if (isSegmentedButton) {
                     if (isPreviouslyCreatedSegmentedSpan(oldSpannable)) {
                         // need to recreate using original, as oldSpannable has prior outdated dislike values
                         oldSpannable = originalDislikeSpan;
                         if (oldSpannable == null) {
-                            // Regular video is opened, then a short is opened then closed,
-                            // then the app is closed then reopened (causes a call of NewVideoId() of the original videoId)
-                            // The original video (that was opened the entire time), is still showing the dislikes count
-                            // but the oldSpannable is now null because it was reset when the videoId was set again
-                            LogHelper.printDebug(() -> "Cannot add dislikes - original span is null" +
-                                    " (short was opened/closed, then app was closed/opened?) "); // ignore, with no toast
+                            LogHelper.printDebug(() -> "Cannot add dislikes - original span is null"); // should never happen
                             return null;
                         }
                     } else {
@@ -282,21 +312,20 @@ public class ReturnYouTubeDislike {
                 }
             }
 
+            // Must block the current thread until fetching is done
+            // There's no known way to edit the text after creation yet
             Future<RYDVoteData> fetchFuture = getVoteFetchFuture();
             if (fetchFuture == null) {
                 LogHelper.printDebug(() -> "fetch future not available (user enabled RYD while video was playing?)");
                 return null;
             }
-            if (SettingsEnum.DEBUG.getBoolean() && !fetchFuture.isDone()) {
-                fetchStartTime = System.currentTimeMillis();
-            }
             RYDVoteData votingData = fetchFuture.get(MAX_MILLISECONDS_TO_BLOCK_UI_WHILE_WAITING_FOR_FETCH_VOTES_TO_COMPLETE, TimeUnit.MILLISECONDS);
             if (votingData == null) {
                 LogHelper.printDebug(() -> "Cannot add dislike to UI (RYD data not available)");
                 return null;
             }
 
-            Spanned replacement = createDislikeSpan(oldSpannable, isSegmentedButton, votingData);
+            SpannableString replacement = createDislikeSpan(oldSpannable, isSegmentedButton, votingData);
             synchronized (videoIdLockObject) {
                 replacementLikeDislikeSpan = replacement;
             }
@@ -306,13 +335,16 @@ public class ReturnYouTubeDislike {
         } catch (TimeoutException e) {
             LogHelper.printDebug(() -> "UI timed out while waiting for fetch votes to complete"); // show no toast
         } catch (Exception e) {
-            LogHelper.printException(() -> "createReplacementSpan failure", e); // should never happen
-        } finally {
-            recordTimeUISpentWaitingForNetworkCall(fetchStartTime);
+            LogHelper.printException(() -> "waitForFetchAndUpdateReplacementSpan failure", e); // should never happen
         }
         return null;
     }
 
+    /**
+     * Called when the like/dislike button is clicked.
+     *
+     * @param vote int that matches {@link Vote#value}
+     */
     public static void sendVote(int vote) {
         if (!SettingsEnum.RYD_ENABLED.getBoolean()) return;
 
@@ -335,11 +367,12 @@ public class ReturnYouTubeDislike {
         try {
             // Must make a local copy of videoId, since it may change between now and when the vote thread runs
             String videoIdToVoteFor = getCurrentVideoId();
-            if (videoIdToVoteFor == null || (lastVideoLoadedWasShort && !PlayerType.getCurrent().isNoneOrHidden())) {
+            if (videoIdToVoteFor == null || lastVideoLoadedWasShort != PlayerType.getCurrent().isNoneOrHidden()) {
                 // User enabled RYD after starting playback of a video.
-                // Or shorts was loaded with regular video present, then shorts was closed, and then user voted on the now visible original video
-                LogHelper.printException(() -> "Cannot send vote",
-                        null, str("revanced_ryd_failure_ryd_enabled_while_playing_video_then_user_voted"));
+                // Or shorts was loaded with regular video present, then shorts was closed,
+                // and then user voted on the now visible original video
+                // Cannot send a vote, because the loaded videoId is for the wrong video.
+                ReVancedUtils.showToastLong(str("revanced_ryd_failure_ryd_enabled_while_playing_video_then_user_voted"));
                 return;
             }
 
@@ -354,9 +387,7 @@ public class ReturnYouTubeDislike {
                 }
             });
 
-            synchronized (videoIdLockObject) {
-                replacementLikeDislikeSpan = null; // ui values need updating
-            }
+            clearCache(); // ui values need updating
 
             // update the downloaded vote data
             Future<RYDVoteData> future = getVoteFetchFuture();
@@ -402,7 +433,7 @@ public class ReturnYouTubeDislike {
     /**
      * @param isSegmentedButton if UI is using the segmented single UI component for both like and dislike
      */
-    private static Spanned createDislikeSpan(@NonNull Spanned oldSpannable, boolean isSegmentedButton, @NonNull RYDVoteData voteData) {
+    private static SpannableString createDislikeSpan(@NonNull Spanned oldSpannable, boolean isSegmentedButton, @NonNull RYDVoteData voteData) {
         if (!isSegmentedButton) {
             // simple replacement of 'dislike' with a number/percentage
             return newSpannableWithDislikes(oldSpannable, voteData);
@@ -493,14 +524,14 @@ public class ReturnYouTubeDislike {
         return false;
     }
 
-    private static Spannable newSpannableWithDislikes(@NonNull Spanned sourceStyling, @NonNull RYDVoteData voteData) {
+    private static SpannableString newSpannableWithDislikes(@NonNull Spanned sourceStyling, @NonNull RYDVoteData voteData) {
         return newSpanUsingStylingOfAnotherSpan(sourceStyling,
                 SettingsEnum.RYD_SHOW_DISLIKE_PERCENTAGE.getBoolean()
                         ? formatDislikePercentage(voteData.getDislikePercentage())
                         : formatDislikeCount(voteData.getDislikeCount()));
     }
 
-    private static Spannable newSpanUsingStylingOfAnotherSpan(@NonNull Spanned sourceStyle, @NonNull String newSpanText) {
+    private static SpannableString newSpanUsingStylingOfAnotherSpan(@NonNull Spanned sourceStyle, @NonNull String newSpanText) {
         SpannableString destination = new SpannableString(newSpanText);
         Object[] spans = sourceStyle.getSpans(0, sourceStyle.length(), Object.class);
         for (Object span : spans) {
@@ -544,33 +575,6 @@ public class ReturnYouTubeDislike {
             return dislikePercentageFormatter.format(dislikePercentage);
         }
     }
-
-
-    /**
-     * Number of times the UI was forced to wait on a network fetch to complete
-     */
-    private static volatile int numberOfTimesUIWaitedOnNetworkCalls;
-
-    /**
-     * Total time the UI waited, of all times it was forced to wait.
-     */
-    private static volatile long totalTimeUIWaitedOnNetworkCalls;
-
-    @SuppressWarnings("NonAtomicOperationOnVolatileField")
-    private static void recordTimeUISpentWaitingForNetworkCall(long timeUIWaitStarted) {
-        if (timeUIWaitStarted == 0 || !SettingsEnum.DEBUG.getBoolean()) {
-            return;
-        }
-        final long timeUIWaitingTotal = System.currentTimeMillis() - timeUIWaitStarted;
-        LogHelper.printDebug(() -> "UI thread waited for: " + timeUIWaitingTotal + "ms for vote fetch to complete");
-
-        totalTimeUIWaitedOnNetworkCalls += timeUIWaitingTotal;
-        numberOfTimesUIWaitedOnNetworkCalls++;
-        final long averageTimeForcedToWait = totalTimeUIWaitedOnNetworkCalls / numberOfTimesUIWaitedOnNetworkCalls;
-        LogHelper.printDebug(() -> "UI thread forced to wait: " + numberOfTimesUIWaitedOnNetworkCalls + " times, "
-                + "total wait time: " + totalTimeUIWaitedOnNetworkCalls + "ms, "
-                + "average wait time: " + averageTimeForcedToWait + "ms");
-    }
 }
 
 class VerticallyCenteredImageSpan extends ImageSpan {

app/src/main/java/app/revanced/integrations/settingsmenu/ReturnYouTubeDislikeSettingsFragment.java

@@ -77,7 +77,7 @@ public class ReturnYouTubeDislikeSettingsFragment extends PreferenceFragment {
         percentagePreference.setTitle(str("revanced_ryd_dislike_percentage_title"));
         percentagePreference.setOnPreferenceChangeListener((pref, newValue) -> {
             SettingsEnum.RYD_SHOW_DISLIKE_PERCENTAGE.saveValue(newValue);
-
+            ReturnYouTubeDislike.clearCache();
             updateUIState();
             return true;
         });
@@ -88,7 +88,7 @@ public class ReturnYouTubeDislikeSettingsFragment extends PreferenceFragment {
         compactLayoutPreference.setTitle(str("revanced_ryd_compact_layout_title"));
         compactLayoutPreference.setOnPreferenceChangeListener((pref, newValue) -> {
             SettingsEnum.RYD_USE_COMPACT_LAYOUT.saveValue(newValue);
-
+            ReturnYouTubeDislike.clearCache();
             updateUIState();
             return true;
         });