Fix new lint from android 14 upgrade, and remove it from the baseline (…

…flutter#47817) See flutter#47609 (comment) for context. It isn't clear to me what the file descriptor here is actually doing, so I'm not actually too sure about this fix. Can it just be deleted? Update: it seems to me that the motivation here is that `ClipData.Item.coerceToText` [consumes a `SecurityException` here](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/content/ClipData.java;l=411;bpv=0;bpt=1), with just a log line that isn't particularly descriptive. And basically we want to run into that same exception so we can [provide a more helpful log line](https://github.com/flutter/engine/blob/00db306f6f7b42b5664d604f33a0de426edf7109/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java#L525), so we do the same thing that the underlying `coerceToText` method does? [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
mnordine · Nov 29, 2023 · 0a005c4 · 0a005c4
1 parent 60b9639
commit 0a005c4
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 14 deletions.
diff --git a/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java b/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java
@@ -11,6 +11,7 @@
 import android.content.ClipDescription;
 import android.content.ClipboardManager;
 import android.content.Context;
+import android.content.res.AssetFileDescriptor;
 import android.os.Build;
 import android.view.HapticFeedbackConstants;
 import android.view.SoundEffectConstants;
@@ -25,6 +26,7 @@
 import io.flutter.Log;
 import io.flutter.embedding.engine.systemchannels.PlatformChannel;
 import java.io.FileNotFoundException;
+import java.io.IOException;
 import java.util.List;
 
 /** Android implementation of the platform plugin. */
@@ -512,14 +514,21 @@ private CharSequence getClipboardData(PlatformChannel.ClipboardContentFormat for
 
     if (!clipboard.hasPrimaryClip()) return null;
 
+    CharSequence charSequence = null;
     try {
       ClipData clip = clipboard.getPrimaryClip();
       if (clip == null) return null;
       if (format == null || format == PlatformChannel.ClipboardContentFormat.PLAIN_TEXT) {
         ClipData.Item item = clip.getItemAt(0);
+        AssetFileDescriptor assetFileDescriptor = null;
         if (item.getUri() != null)
-          activity.getContentResolver().openTypedAssetFileDescriptor(item.getUri(), "text/*", null);
-        return item.coerceToText(activity);
+          assetFileDescriptor =
+              activity
+                  .getContentResolver()
+                  .openTypedAssetFileDescriptor(item.getUri(), "text/*", null);
+        charSequence = item.coerceToText(activity);
+        if (assetFileDescriptor != null) assetFileDescriptor.close();
+        return charSequence;
       }
     } catch (SecurityException e) {
       Log.w(
@@ -531,6 +540,9 @@ private CharSequence getClipboardData(PlatformChannel.ClipboardContentFormat for
       return null;
     } catch (FileNotFoundException e) {
       return null;
+    } catch (IOException e) {
+      Log.w(TAG, "Failed to close AssetFileDescriptor while accessing clipboard data.", e);
+      return charSequence;
     }
 
     return null;

diff --git a/shell/platform/android/test/io/flutter/plugin/platform/PlatformPluginTest.java b/shell/platform/android/test/io/flutter/plugin/platform/PlatformPluginTest.java
@@ -11,6 +11,10 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.anyBoolean;
+import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
@@ -20,9 +24,11 @@
 
 import android.app.Activity;
 import android.content.ClipData;
+import android.content.ClipDescription;
 import android.content.ClipboardManager;
 import android.content.ContentResolver;
 import android.content.Context;
+import android.content.res.AssetFileDescriptor;
 import android.net.Uri;
 import android.os.Build;
 import android.view.View;
@@ -84,18 +90,39 @@ public void platformPlugin_getClipboardData() throws IOException {
     PlatformChannel fakePlatformChannel = mock(PlatformChannel.class);
     PlatformPlugin platformPlugin = new PlatformPlugin(fakeActivity, fakePlatformChannel);
 
+    // Successfully get the contents of the primary clip when they contain text.
     ClipboardContentFormat clipboardFormat = ClipboardContentFormat.PLAIN_TEXT;
     assertNull(platformPlugin.mPlatformMessageHandler.getClipboardData(clipboardFormat));
     ClipData clip = ClipData.newPlainText("label", "Text");
     clipboardManager.setPrimaryClip(clip);
     assertNotNull(platformPlugin.mPlatformMessageHandler.getClipboardData(clipboardFormat));
 
-    ContentResolver contentResolver = ctx.getContentResolver();
+    // Return null when the primary clip contains non-text media.
+    ContentResolver contentResolver = spy(ctx.getContentResolver());
     when(fakeActivity.getContentResolver()).thenReturn(contentResolver);
     Uri uri = Uri.parse("content://media/external_primary/images/media/");
     clip = ClipData.newUri(contentResolver, "URI", uri);
     clipboardManager.setPrimaryClip(clip);
     assertNull(platformPlugin.mPlatformMessageHandler.getClipboardData(clipboardFormat));
+
+    // Still return text when the AssetFileDescriptor throws an IOException.
+    when(fakeActivity.getContentResolver()).thenReturn(contentResolver);
+    ClipDescription clipDescription =
+        new ClipDescription(
+            "label",
+            new String[] {
+              ClipDescription.MIMETYPE_TEXT_PLAIN, ClipDescription.MIMETYPE_TEXT_URILIST
+            });
+    ClipData.Item clipDataItem = new ClipData.Item("Text", null, uri);
+    ClipData clipData = new ClipData(clipDescription, clipDataItem);
+    clipboardManager.setPrimaryClip(clipData);
+    AssetFileDescriptor fakeAssetFileDescriptor = mock(AssetFileDescriptor.class);
+    doReturn(fakeAssetFileDescriptor)
+        .when(contentResolver)
+        .openTypedAssetFileDescriptor(eq(uri), anyString(), eq(null));
+    doThrow(new IOException()).when(fakeAssetFileDescriptor).close();
+    assertNotNull(platformPlugin.mPlatformMessageHandler.getClipboardData(clipboardFormat));
+    verify(fakeAssetFileDescriptor).close();
   }
 
   @SuppressWarnings("deprecation")

diff --git a/tools/android_lint/baseline.xml b/tools/android_lint/baseline.xml
@@ -67,17 +67,6 @@
             column="82"/>
     </issue>
 
-    <issue
-        id="Recycle"
-        message="This `AssetFileDescriptor` should be freed up after use with `#close()`"
-        errorLine1="          activity.getContentResolver().openTypedAssetFileDescriptor(item.getUri(), &quot;text/*&quot;, null);"
-        errorLine2="                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
-        <location
-            file="../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformPlugin.java"
-            line="522"
-            column="41"/>
-    </issue>
-
     <issue
         id="ClickableViewAccessibility"
         message="Custom view `FlutterView` overrides `onTouchEvent` but not `performClick`"