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

This commit is contained in:
LisoUseInAIKyrios 2023-04-12 21:52:45 +04:00 committed by oSumAtrIX
parent b959c8ef98
commit 212e4f2ce4
No known key found for this signature in database
GPG Key ID: A9B3094ACDB604B4
3 changed files with 69 additions and 65 deletions

View File

@ -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 {

View File

@ -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 {

View File

@ -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;
});