From 6e02a815018dc2217cb1c4df927c720720705352 Mon Sep 17 00:00:00 2001 From: ibaker Date: Tue, 19 May 2020 11:24:23 +0100 Subject: [PATCH] Cleanup WebvttCueParser and WebvttCssStyle - Remove scratchStyleMatches output parameter from WebvttCueParser. - Switch from String[] to Set for representing classes. - In-line WebvttCssStyle.reset() since it's not used anywhere else. PiperOrigin-RevId: 312249552 --- .../text/webvtt/WebvttCssStyle.java | 22 +++---- .../text/webvtt/WebvttCueParser.java | 63 +++++++++---------- .../exoplayer2/text/webvtt/CssParserTest.java | 53 +++++++++++----- 3 files changed, 74 insertions(+), 64 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCssStyle.java b/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCssStyle.java index bcb7e0b87c..9a00151ccb 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCssStyle.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCssStyle.java @@ -27,8 +27,8 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.Arrays; import java.util.Collections; -import java.util.List; -import org.checkerframework.checker.nullness.qual.EnsuresNonNull; +import java.util.HashSet; +import java.util.Set; /** * Style object of a Css style block in a Webvtt file. @@ -80,7 +80,7 @@ public final class WebvttCssStyle { // Selector properties. private String targetId; private String targetTag; - private List targetClasses; + private Set targetClasses; private String targetVoice; // Style properties. @@ -96,18 +96,10 @@ public final class WebvttCssStyle { @RubySpan.Position private int rubyPosition; private boolean combineUpright; - // Calling reset() is forbidden because `this` isn't initialized. This can be safely suppressed - // because reset() only assigns fields, it doesn't read any. - @SuppressWarnings("nullness:method.invocation.invalid") public WebvttCssStyle() { - reset(); - } - - @EnsuresNonNull({"targetId", "targetTag", "targetClasses", "targetVoice"}) - public void reset() { targetId = ""; targetTag = ""; - targetClasses = Collections.emptyList(); + targetClasses = Collections.emptySet(); targetVoice = ""; fontFamily = null; hasFontColor = false; @@ -129,7 +121,7 @@ public final class WebvttCssStyle { } public void setTargetClasses(String[] targetClasses) { - this.targetClasses = Arrays.asList(targetClasses); + this.targetClasses = new HashSet<>(Arrays.asList(targetClasses)); } public void setTargetVoice(String targetVoice) { @@ -155,7 +147,7 @@ public final class WebvttCssStyle { * @return The score of the match, zero if there is no match. */ public int getSpecificityScore( - @Nullable String id, @Nullable String tag, String[] classes, @Nullable String voice) { + @Nullable String id, @Nullable String tag, Set classes, @Nullable String voice) { if (targetId.isEmpty() && targetTag.isEmpty() && targetClasses.isEmpty() && targetVoice.isEmpty()) { // The selector is universal. It matches with the minimum score if and only if the given @@ -166,7 +158,7 @@ public final class WebvttCssStyle { score = updateScoreForMatch(score, targetId, id, 0x40000000); score = updateScoreForMatch(score, targetTag, tag, 2); score = updateScoreForMatch(score, targetVoice, voice, 4); - if (score == -1 || !Arrays.asList(classes).containsAll(targetClasses)) { + if (score == -1 || !classes.containsAll(targetClasses)) { return 0; } else { score += targetClasses.size() * 4; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCueParser.java b/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCueParser.java index f9220c44f7..c56fa080a0 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCueParser.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCueParser.java @@ -49,8 +49,10 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @@ -239,7 +241,6 @@ public final class WebvttCueParser { @Nullable String id, String markup, List styles) { SpannableStringBuilder spannedText = new SpannableStringBuilder(); ArrayDeque startTagStack = new ArrayDeque<>(); - List scratchStyleMatches = new ArrayList<>(); int pos = 0; List nestedElements = new ArrayList<>(); while (pos < markup.length()) { @@ -270,8 +271,7 @@ public final class WebvttCueParser { break; } startTag = startTagStack.pop(); - applySpansForTag( - id, startTag, nestedElements, spannedText, styles, scratchStyleMatches); + applySpansForTag(id, startTag, nestedElements, spannedText, styles); if (!startTagStack.isEmpty()) { nestedElements.add(new Element(startTag, spannedText.length())); } else { @@ -307,16 +307,14 @@ public final class WebvttCueParser { } // apply unclosed tags while (!startTagStack.isEmpty()) { - applySpansForTag( - id, startTagStack.pop(), nestedElements, spannedText, styles, scratchStyleMatches); + applySpansForTag(id, startTagStack.pop(), nestedElements, spannedText, styles); } applySpansForTag( id, StartTag.buildWholeCueVirtualTag(), /* nestedElements= */ Collections.emptyList(), spannedText, - styles, - scratchStyleMatches); + styles); return SpannedString.valueOf(spannedText); } @@ -534,8 +532,7 @@ public final class WebvttCueParser { StartTag startTag, List nestedElements, SpannableStringBuilder text, - List styles, - List scratchStyleMatches) { + List styles) { int start = startTag.position; int end = text.length(); @@ -565,10 +562,9 @@ public final class WebvttCueParser { return; } - scratchStyleMatches.clear(); - getApplicableStyles(styles, cueId, startTag, scratchStyleMatches); - for (int i = 0; i < scratchStyleMatches.size(); i++) { - applyStyleToText(text, scratchStyleMatches.get(i).style, start, end); + List applicableStyles = getApplicableStyles(styles, cueId, startTag); + for (int i = 0; i < applicableStyles.size(); i++) { + applyStyleToText(text, applicableStyles.get(i).style, start, end); } } @@ -616,8 +612,7 @@ public final class WebvttCueParser { @RubySpan.Position private static int getRubyPosition( List styles, @Nullable String cueId, StartTag startTag) { - List styleMatches = new ArrayList<>(); - getApplicableStyles(styles, cueId, startTag, styleMatches); + List styleMatches = getApplicableStyles(styles, cueId, startTag); for (int i = 0; i < styleMatches.size(); i++) { WebvttCssStyle style = styleMatches.get(i).style; if (style.getRubyPosition() != RubySpan.POSITION_UNKNOWN) { @@ -652,7 +647,7 @@ public final class WebvttCueParser { * colors. */ private static void applyDefaultColors( - SpannableStringBuilder text, String[] classes, int start, int end) { + SpannableStringBuilder text, Set classes, int start, int end) { for (String className : classes) { if (DEFAULT_TEXT_COLORS.containsKey(className)) { int color = DEFAULT_TEXT_COLORS.get(className); @@ -746,20 +741,18 @@ public final class WebvttCueParser { return Util.splitAtFirst(tagExpression, "[ \\.]")[0]; } - private static void getApplicableStyles( - List declaredStyles, - @Nullable String id, - StartTag tag, - List output) { - int styleCount = declaredStyles.size(); - for (int i = 0; i < styleCount; i++) { + private static List getApplicableStyles( + List declaredStyles, @Nullable String id, StartTag tag) { + List applicableStyles = new ArrayList<>(); + for (int i = 0; i < declaredStyles.size(); i++) { WebvttCssStyle style = declaredStyles.get(i); int score = style.getSpecificityScore(id, tag.name, tag.classes, tag.voice); if (score > 0) { - output.add(new StyleMatch(score, style)); + applicableStyles.add(new StyleMatch(score, style)); } } - Collections.sort(output); + Collections.sort(applicableStyles); + return applicableStyles; } private static final class WebvttCueInfoBuilder { @@ -929,14 +922,12 @@ public final class WebvttCueParser { private static final class StartTag { - private static final String[] NO_CLASSES = new String[0]; - public final String name; public final int position; public final String voice; - public final String[] classes; + public final Set classes; - private StartTag(String name, int position, String voice, String[] classes) { + private StartTag(String name, int position, String voice, Set classes) { this.position = position; this.name = name; this.voice = voice; @@ -956,17 +947,19 @@ public final class WebvttCueParser { } String[] nameAndClasses = Util.split(fullTagExpression, "\\."); String name = nameAndClasses[0]; - String[] classes; - if (nameAndClasses.length > 1) { - classes = Util.nullSafeArrayCopyOfRange(nameAndClasses, 1, nameAndClasses.length); - } else { - classes = NO_CLASSES; + Set classes = new HashSet<>(); + for (int i = 1; i < nameAndClasses.length; i++) { + classes.add(nameAndClasses[i]); } return new StartTag(name, position, voice, classes); } public static StartTag buildWholeCueVirtualTag() { - return new StartTag("", 0, "", new String[0]); + return new StartTag( + /* name= */ "", + /* position= */ 0, + /* voice= */ "", + /* classes= */ Collections.emptySet()); } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/CssParserTest.java b/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/CssParserTest.java index b77d4f14ab..d1c7ba78e9 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/CssParserTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/CssParserTest.java @@ -21,6 +21,9 @@ import static com.google.common.truth.Truth.assertThat; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.util.ParsableByteArray; import com.google.android.exoplayer2.util.Util; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; import java.util.List; import org.junit.Before; import org.junit.Test; @@ -163,32 +166,54 @@ public final class CssParserTest { public void styleScoreSystem() { WebvttCssStyle style = new WebvttCssStyle(); // Universal selector. - assertThat(style.getSpecificityScore("", "", new String[0], "")).isEqualTo(1); + assertThat(style.getSpecificityScore("", "", Collections.emptySet(), "")).isEqualTo(1); // Class match without tag match. style.setTargetClasses(new String[] { "class1", "class2"}); - assertThat(style.getSpecificityScore("", "", new String[]{"class1", "class2", "class3"}, - "")).isEqualTo(8); + assertThat( + style.getSpecificityScore( + "", "", new HashSet<>(Arrays.asList("class1", "class2", "class3")), "")) + .isEqualTo(8); // Class and tag match style.setTargetTagName("b"); - assertThat(style.getSpecificityScore("", "b", - new String[]{"class1", "class2", "class3"}, "")).isEqualTo(10); + assertThat( + style.getSpecificityScore( + "", "b", new HashSet<>(Arrays.asList("class1", "class2", "class3")), "")) + .isEqualTo(10); // Class insufficiency. - assertThat(style.getSpecificityScore("", "b", new String[]{"class1", "class"}, "")) + assertThat( + style.getSpecificityScore("", "b", new HashSet<>(Arrays.asList("class1", "class")), "")) .isEqualTo(0); // Voice, classes and tag match. style.setTargetVoice("Manuel Cráneo"); - assertThat(style.getSpecificityScore("", "b", - new String[]{"class1", "class2", "class3"}, "Manuel Cráneo")).isEqualTo(14); + assertThat( + style.getSpecificityScore( + "", + "b", + new HashSet<>(Arrays.asList("class1", "class2", "class3")), + "Manuel Cráneo")) + .isEqualTo(14); // Voice mismatch. - assertThat(style.getSpecificityScore(null, "b", - new String[]{"class1", "class2", "class3"}, "Manuel Craneo")).isEqualTo(0); + assertThat( + style.getSpecificityScore( + null, + "b", + new HashSet<>(Arrays.asList("class1", "class2", "class3")), + "Manuel Craneo")) + .isEqualTo(0); // Id, voice, classes and tag match. style.setTargetId("id"); - assertThat(style.getSpecificityScore("id", "b", - new String[]{"class1", "class2", "class3"}, "Manuel Cráneo")).isEqualTo(0x40000000 + 14); + assertThat( + style.getSpecificityScore( + "id", + "b", + new HashSet<>(Arrays.asList("class1", "class2", "class3")), + "Manuel Cráneo")) + .isEqualTo(0x40000000 + 14); // Id mismatch. - assertThat(style.getSpecificityScore("id1", "b", - new String[]{"class1", "class2", "class3"}, "")).isEqualTo(0); + assertThat( + style.getSpecificityScore( + "id1", "b", new HashSet<>(Arrays.asList("class1", "class2", "class3")), "")) + .isEqualTo(0); } // Utility methods.