Address reviews

shell/platform/android/io/flutter/embedding/engine/systemchannels/SpellCheckChannel.java

@@ -2,7 +2,6 @@
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
-import androidx.annotation.VisibleForTesting;
 import io.flutter.Log;
 import io.flutter.embedding.engine.dart.DartExecutor;
 import io.flutter.plugin.common.JSONMethodCodec;
@@ -12,20 +11,49 @@
 import org.json.JSONArray;
 import org.json.JSONException;
 
+/**
+ * {@link SpellCheckChannel} is a platform channel that is used by Flutter to initiate spell check
+ * in the Android engine and for the Android engine to send back the results.
+ *
+ * <p>If the {@link io.flutter.plugin.editing.SpellCheckPlugin} is used to handle spell check
+ * behavior, (such is the case by default) then there is new text to be spell checked, Flutter will
+ * send a message to the engine. In response, the {@link io.flutter.plugin.editing.SpellCheckPlugin}
+ * will make a call to Android's spell check service to fetch spell check results for the specified
+ * text.
+ *
+ * <p>Once the spell check results are received by the {@link
+ * io.flutter.plugin.editing.SpellCheckPlugin}, a message will be sent back to Flutter with the
+ * results. See diagram below for overview:
+ *  -----------------------------------------------------------------------------------------------------------------------------
+ * |      From        |        To         |               Message                |                 Arguments                    |
+ * | ---------------------------------------------------------------------------------------------------------------------------|
+ * |     Flutter      |   Android Engine  |     SpellCheck.iniateSpellCheck      |  {@code String} locale, {@code String} text  |
+ * |----------------------------------------------------------------------------------------------------------------------------|
+ * |  Android Engine  |      Flutter      |  SpellCheck.updateSpellCheckResults  |        {@code ArrayList} of results          |
+ *  -----------------------------------------------------------------------------------------------------------------------------
+ *
+ * <p>By default, {@link io.flutter.plugin.editing.SpellCheckPlugin} implements {@link
+ * io.flutter.plugin.common.MethodChannel.MethodCallHandler} to initiate spell check via the Android
+ * spell check service. Implement {@link SpellCheckMethodHandler} to respond to Flutter spell check
+ * messages.
+ */
 public class SpellCheckChannel {
   private static final String TAG = "SpellCheckChannel";
 
   @NonNull public final MethodChannel channel;
   @Nullable private SpellCheckMethodHandler spellCheckMethodHandler;
 
-  @NonNull @VisibleForTesting
+  @NonNull
   final MethodChannel.MethodCallHandler parsingMethodHandler =
       new MethodChannel.MethodCallHandler() {
         @Override
         public void onMethodCall(@NonNull MethodCall call, @NonNull MethodChannel.Result result) {
           if (spellCheckMethodHandler == null) {
             // If no explicit SpellCheckMethodHandler has been registered then we don't
             // need to forward this call to an API. Return.
+            Log.v(
+                TAG,
+                "No SpellCheckeMethodHandler registered, call not forwarded to spell check API.");
             return;
           }
           String method = call.method;
@@ -51,7 +79,7 @@ public void onMethodCall(@NonNull MethodCall call, @NonNull MethodChannel.Result
       };
 
   public SpellCheckChannel(@NonNull DartExecutor dartExecutor) {
-    this.channel = new MethodChannel(dartExecutor, "flutter/spellcheck", JSONMethodCodec.INSTANCE);
+    channel = new MethodChannel(dartExecutor, "flutter/spellcheck", JSONMethodCodec.INSTANCE);
     channel.setMethodCallHandler(parsingMethodHandler);
   }
 
@@ -76,6 +104,6 @@ public interface SpellCheckMethodHandler {
      * SpellCheckChannel#setSpellCheckMethodHandler(SpellCheckMethodHandler)} once spell check
      * results are received from the native spell check service.
      */
-    void initiateSpellCheck(String locale, String text);
+    void initiateSpellCheck(@NonNull String locale, @NonNull String text);
   }
 }

shell/platform/android/io/flutter/plugin/editing/SpellCheckPlugin.java

@@ -12,31 +12,50 @@
 import android.view.textservice.TextServicesManager;
 import androidx.annotation.NonNull;
 import io.flutter.embedding.engine.systemchannels.SpellCheckChannel;
+import io.flutter.plugin.localization.LocalizationPlugin;
 import java.util.ArrayList;
 import java.util.Locale;
 
-/** Android implementation of the spell check plugin. */
-public class SpellCheckPlugin implements SpellCheckerSession.SpellCheckerSessionListener {
-
-  @NonNull private final Context mContext;
-  @NonNull private final SpellCheckChannel mSpellCheckChannel;
-  @NonNull private final TextServicesManager tsm;
+/**
+ * {@link SpellCheckPlugin} is the implementation of all functionality needed for spell check for
+ * text input.
+ *
+ * <p>The plugin handles requests for spell check sent by the {@link
+ * io.flutter.embedding.engine.systemchannels.SpellCheckChannel} via sending requests to the Android
+ * spell checker. It also receive the spell check results from the service and send them back to
+ * Flutter through the {@link io.flutter.embedding.engine.systemchannels.SpellCheckChannel}.
+ */
+public class SpellCheckPlugin
+    implements SpellCheckChannel.SpellCheckMethodHandler,
+        SpellCheckerSession.SpellCheckerSessionListener {
+
+  private final Context mContext;
+  private final SpellCheckChannel mSpellCheckChannel;
+  private final TextServicesManager mTextServicesManager;
   private SpellCheckerSession mSpellCheckerSession;
 
+  // The maximum number of suggestions that the Android spell check service is allowed to provide
+  // per word.
+  // Same number that is used by default for Android's {@code TextView}'s.
+  private static final int MAX_SPELL_CHECK_SUGGESTIONS = 5;
+
   public SpellCheckPlugin(@NonNull Context context, @NonNull SpellCheckChannel spellCheckChannel) {
     mContext = context;
     mSpellCheckChannel = spellCheckChannel;
-    tsm = (TextServicesManager) mContext.getSystemService(Context.TEXT_SERVICES_MANAGER_SERVICE);
+    mTextServicesManager =
+        (TextServicesManager) mContext.getSystemService(Context.TEXT_SERVICES_MANAGER_SERVICE);
 
-    mSpellCheckChannel.setSpellCheckMethodHandler(
-        new SpellCheckChannel.SpellCheckMethodHandler() {
-          @Override
-          public void initiateSpellCheck(String locale, String text) {
-            performSpellCheck(locale, text);
-          }
-        });
+    mSpellCheckChannel.setSpellCheckMethodHandler(this);
   }
 
+  /**
+   * Unregisters this {@code SpellCheckPlugin} as the {@code
+   * SpellCheckChannel.SpellCheckMethodHandler}, for the {@link
+   * io.flutter.embedding.engine.systemchannels.SpellCheckChannel}, and closes the most recently
+   * opened {@code SpellCheckerSessions}.
+   *
+   * <p>Do not invoke any methods on a {@code SpellCheckPlugin} after invoking this method.
+   */
   public void destroy() {
     mSpellCheckChannel.setSpellCheckMethodHandler(null);
 
@@ -45,62 +64,73 @@ public void destroy() {
     }
   }
 
+  @Override
+  public void initiateSpellCheck(@NonNull String locale, @NonNull String text) {
+    performSpellCheck(locale, text);
+  }
+
   /** Calls on the Android spell check API to spell check specified text. */
   public void performSpellCheck(String locale, String text) {
     String[] localeCodes = locale.split("-");
-    Locale parsedLocale;
-
-    if (localeCodes.length == 3) {
-      parsedLocale = new Locale(localeCodes[0], localeCodes[1], localeCodes[2]);
-    } else if (localeCodes.length == 2) {
-      parsedLocale = new Locale(localeCodes[0], localeCodes[1]);
-    } else {
-      parsedLocale = new Locale(localeCodes[0]);
-    }
+    Locale localeFromString = LocalizationPlugin.localeFromString(locale);
 
     if (mSpellCheckerSession != null) {
       mSpellCheckerSession.close();
     }
-    mSpellCheckerSession = tsm.newSpellCheckerSession(null, parsedLocale, this, true);
+    mSpellCheckerSession =
+        mTextServicesManager.newSpellCheckerSession(
+            null,
+            localeFromString,
+            this,
+            /** referToSpellCheckerLanguageSettings= */
+            true);
 
     TextInfo[] textInfos = new TextInfo[] {new TextInfo(text)};
-    mSpellCheckerSession.getSentenceSuggestions(textInfos, 3);
+    mSpellCheckerSession.getSentenceSuggestions(textInfos, MAX_SPELL_CHECK_SUGGESTIONS);
   }
 
   /**
    * Callback for Android spell check API that decomposes results and send results through the
    * {@link SpellCheckChannel}.
+   *
+   * <p>Spell check results will be encoded as a string representing the span of that result, with
+   * the format [start_index.end_index.suggestion_1,suggestion_2,suggestion_3] where there may be up
+   * to 5 suggestions.
    */
   @Override
   public void onGetSentenceSuggestions(SentenceSuggestionsInfo[] results) {
     ArrayList<String> spellCheckerSuggestionSpans = new ArrayList<String>();
 
-    for (int i = 0; i < results[0].getSuggestionsCount(); i++) {
-      SuggestionsInfo suggestionsInfo = results[0].getSuggestionsInfoAt(i);
-      int suggestionsCount = suggestionsInfo.getSuggestionsCount();
+    if (results.length > 0) {
+      SentenceSuggestionsInfo spellCheckResults = results[0];
 
-      if (suggestionsCount > 0) {
-        String spellCheckerSuggestionSpan = "";
-        int start = results[0].getOffsetAt(i);
-        int length = results[0].getLengthAt(i);
+      for (int i = 0; i < spellCheckResults.getSuggestionsCount(); i++) {
 
-        spellCheckerSuggestionSpan += (String.valueOf(start) + ".");
-        spellCheckerSuggestionSpan += (String.valueOf(start + (length - 1)) + ".");
+        SuggestionsInfo suggestionsInfo = spellCheckResults.getSuggestionsInfoAt(i);
+        int suggestionsCount = suggestionsInfo.getSuggestionsCount();
 
-        for (int j = 0; j < suggestionsCount; j++) {
-          spellCheckerSuggestionSpan += (suggestionsInfo.getSuggestionAt(j) + ",");
-        }
+        if (suggestionsCount > 0) {
+          String spellCheckerSuggestionSpan = "";
+          int start = spellCheckResults.getOffsetAt(i);
+          int length = spellCheckResults.getLengthAt(i);
+
+          spellCheckerSuggestionSpan += (String.valueOf(start) + ".");
+          spellCheckerSuggestionSpan += (String.valueOf(start + (length - 1)) + ".");
 
-        spellCheckerSuggestionSpans.add(
-            spellCheckerSuggestionSpan.substring(0, spellCheckerSuggestionSpan.length() - 1));
+          for (int j = 0; j < suggestionsCount; j++) {
+            spellCheckerSuggestionSpan += (suggestionsInfo.getSuggestionAt(j) + ",");
+          }
+
+          spellCheckerSuggestionSpans.add(
+              spellCheckerSuggestionSpan.substring(0, spellCheckerSuggestionSpan.length() - 1));
+        }
       }
     }
 
     mSpellCheckChannel.updateSpellCheckResults(spellCheckerSuggestionSpans);
   }
 
   @Override
-  @SuppressWarnings("deprecation")
   public void onGetSuggestions(SuggestionsInfo[] results) {
     // Deprecated callback for Android spell check API; will not use.
   }

shell/platform/android/test/io/flutter/plugin/editing/SpellCheckPluginTest.java

@@ -93,9 +93,10 @@ public void performSpellCheckSendsRequestToAndroidSpellCheckService() {
         .thenReturn(fakeTextServicesManager);
     SpellCheckPlugin spellCheckPlugin = new SpellCheckPlugin(fakeContext, fakeSpellCheckChannel);
     SpellCheckerSession fakeSpellCheckerSession = mock(SpellCheckerSession.class);
-    when(fakeTextServicesManager.newSpellCheckerSession(
-            null, new Locale("en", "US"), spellCheckPlugin, true))
+    Locale english_US = new Locale("en", "US");
+    when(fakeTextServicesManager.newSpellCheckerSession(null, english_US, spellCheckPlugin, true))
         .thenReturn(fakeSpellCheckerSession);
+    int maxSuggestions = 5;
 
     ArgumentCaptor<TextInfo[]> textInfosCaptor = ArgumentCaptor.forClass(TextInfo[].class);
     ArgumentCaptor<Integer> maxSuggestionsCaptor = ArgumentCaptor.forClass(Integer.class);
@@ -105,7 +106,27 @@ null, new Locale("en", "US"), spellCheckPlugin, true))
     verify(fakeSpellCheckerSession)
         .getSentenceSuggestions(textInfosCaptor.capture(), maxSuggestionsCaptor.capture());
     assertEquals("Hello, wrold!", textInfosCaptor.getValue()[0].getText());
-    assertEquals(Integer.valueOf(3), maxSuggestionsCaptor.getValue());
+    assertEquals(Integer.valueOf(maxSuggestions), maxSuggestionsCaptor.getValue());
+  }
+
+  @Test
+  public void performSpellCheckCreatesNewSpellCheckerSession() {
+    Context fakeContext = mock(Context.class);
+    SpellCheckChannel fakeSpellCheckChannel = mock(SpellCheckChannel.class);
+    TextServicesManager fakeTextServicesManager = mock(TextServicesManager.class);
+    when(fakeContext.getSystemService(Context.TEXT_SERVICES_MANAGER_SERVICE))
+        .thenReturn(fakeTextServicesManager);
+    SpellCheckPlugin spellCheckPlugin = new SpellCheckPlugin(fakeContext, fakeSpellCheckChannel);
+    SpellCheckerSession fakeSpellCheckerSession = mock(SpellCheckerSession.class);
+    Locale english_US = new Locale("en", "US");
+    when(fakeTextServicesManager.newSpellCheckerSession(null, english_US, spellCheckPlugin, true))
+        .thenReturn(fakeSpellCheckerSession);
+
+    spellCheckPlugin.performSpellCheck("en-US", "Hello, worl!");
+    spellCheckPlugin.performSpellCheck("en-US", "Hello, world!");
+
+    verify(fakeTextServicesManager, times(2))
+        .newSpellCheckerSession(null, english_US, spellCheckPlugin, true);
   }
 
   @Test