From 212e4f2ce43360776fe20467c4142c9936b22d42 Mon Sep 17 00:00:00 2001 From: LisoUseInAIKyrios <118716522+LisoUseInAIKyrios@users.noreply.github.com> Date: Wed, 12 Apr 2023 21:52:45 +0400 Subject: [PATCH] fix(youtube/return-youtube-dislike): stale dislike data shown after opening / closing the app during shorts playback (#356) --- .../patches/ReturnYouTubeDislikePatch.java | 2 +- .../ReturnYouTubeDislike.java | 128 +++++++++--------- .../ReturnYouTubeDislikeSettingsFragment.java | 4 +- 3 files changed, 69 insertions(+), 65 deletions(-) diff --git a/app/src/main/java/app/revanced/integrations/patches/ReturnYouTubeDislikePatch.java b/app/src/main/java/app/revanced/integrations/patches/ReturnYouTubeDislikePatch.java index 69a5def1..dd1d6e37 100644 --- a/app/src/main/java/app/revanced/integrations/patches/ReturnYouTubeDislikePatch.java +++ b/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 { diff --git a/app/src/main/java/app/revanced/integrations/returnyoutubedislike/ReturnYouTubeDislike.java b/app/src/main/java/app/revanced/integrations/returnyoutubedislike/ReturnYouTubeDislike.java index d9a41374..d441c2b1 100644 --- a/app/src/main/java/app/revanced/integrations/returnyoutubedislike/ReturnYouTubeDislike.java +++ b/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 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 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 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 { diff --git a/app/src/main/java/app/revanced/integrations/settingsmenu/ReturnYouTubeDislikeSettingsFragment.java b/app/src/main/java/app/revanced/integrations/settingsmenu/ReturnYouTubeDislikeSettingsFragment.java index 6664861f..3175675e 100644 --- a/app/src/main/java/app/revanced/integrations/settingsmenu/ReturnYouTubeDislikeSettingsFragment.java +++ b/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; });