From 79efa4342852a3e9271a56e3a0fb7c15be664e9a Mon Sep 17 00:00:00 2001 From: Nikita Kraev Date: Tue, 25 Feb 2020 05:43:28 -0800 Subject: [PATCH] Update ImageEditingManager to use internal storage before falling back to external for cache Summary: Changelog: [Android] [Changed] - Internal storage now will be preferred for caching images from ImageEditor. Now we try to write to internal cache directory first. If that fails (due to no memory error or other IOException), we attempt to write to external storage (if that is allowed). I introduced additional configuration flag, `allowExternalStorage` which is true by default. And i updated documentation for new behaviour - see `ImageEditor.cropImage(..)` comment. Reviewed By: makovkastar Differential Revision: D19878158 fbshipit-source-id: 7e338ce68f535f74c62b5eecd5a94af7e7396f8b --- .../FBReactNativeSpec/FBReactNativeSpec.h | 6 ++ .../modules/camera/ImageEditingManager.java | 62 +++++++++++-------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h b/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h index eb2b0c3416c422..754ea84d77b2a7 100644 --- a/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h +++ b/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h @@ -1416,6 +1416,7 @@ namespace JS { JS::NativeImageEditor::OptionsSize size() const; folly::Optional displaySize() const; NSString *resizeMode() const; + folly::Optional allowExternalStorage() const; Options(NSDictionary *const v) : _v(v) {} private: @@ -3490,6 +3491,11 @@ inline NSString *JS::NativeImageEditor::Options::resizeMode() const id const p = _v[@"resizeMode"]; return RCTBridgingToString(p); } +inline folly::Optional JS::NativeImageEditor::Options::allowExternalStorage() const +{ + id const p = _v[@"allowExternalStorage"]; + return RCTBridgingToOptionalBool(p); +} inline bool JS::NativeImagePickerIOS::SpecOpenCameraDialogConfig::unmirrorFrontFacingCamera() const { id const p = _v[@"unmirrorFrontFacingCamera"]; diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/camera/ImageEditingManager.java b/ReactAndroid/src/main/java/com/facebook/react/modules/camera/ImageEditingManager.java index e1ef2238ede520..105d4d3c153fa6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/camera/ImageEditingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/camera/ImageEditingManager.java @@ -158,6 +158,9 @@ public void cropImage( String uri, ReadableMap options, final Callback success, final Callback error) { ReadableMap offset = options.hasKey("offset") ? options.getMap("offset") : null; ReadableMap size = options.hasKey("size") ? options.getMap("size") : null; + boolean allowExternalStorage = + options.hasKey("allowExternalStorage") ? options.getBoolean("allowExternalStorage") : true; + if (offset == null || size == null || !offset.hasKey("x") @@ -178,6 +181,7 @@ public void cropImage( (int) offset.getDouble("y"), (int) size.getDouble("width"), (int) size.getDouble("height"), + allowExternalStorage, success, error); if (options.hasKey("displaySize")) { @@ -195,6 +199,7 @@ private static class CropTask extends GuardedAsyncTask { final int mY; final int mWidth; final int mHeight; + final boolean mAllowExternalStorage; int mTargetWidth = 0; int mTargetHeight = 0; final Callback mSuccess; @@ -207,6 +212,7 @@ private CropTask( int y, int width, int height, + boolean allowExternalStorage, Callback success, Callback error) { super(context); @@ -220,6 +226,7 @@ private CropTask( mY = y; mWidth = width; mHeight = height; + mAllowExternalStorage = allowExternalStorage; mSuccess = success; mError = error; } @@ -267,8 +274,17 @@ protected void doInBackgroundGuarded(Void... params) { throw new IOException("Could not determine MIME type"); } - File tempFile = createTempFile(mContext, mimeType); - writeCompressedBitmapToFile(cropped, mimeType, tempFile); + File tempFile; + try { + tempFile = writeBitmapToInternalCache(mContext, cropped, mimeType); + } catch (Exception e) { + if (mAllowExternalStorage) { + tempFile = writeBitmapToExternalCache(mContext, cropped, mimeType); + } else { + throw new SecurityException( + "We couldn't create file in internal cache and external cache is disabled. Did you forget to pass allowExternalStorage=true?"); + } + } if (mimeType.equals("image/jpeg")) { copyExif(mContext, Uri.parse(mUri), tempFile); @@ -455,42 +471,36 @@ private static Bitmap.CompressFormat getCompressFormatForType(String type) { return Bitmap.CompressFormat.JPEG; } + private static File writeBitmapToInternalCache(Context context, Bitmap cropped, String mimeType) + throws IOException { + File tempFile = createTempFile(context.getCacheDir(), mimeType); + writeCompressedBitmapToFile(cropped, mimeType, tempFile); + return tempFile; + } + + private static File writeBitmapToExternalCache(Context context, Bitmap cropped, String mimeType) + throws IOException { + File tempFile = createTempFile(context.getExternalCacheDir(), mimeType); + writeCompressedBitmapToFile(cropped, mimeType, tempFile); + return tempFile; + } + private static void writeCompressedBitmapToFile(Bitmap cropped, String mimeType, File tempFile) throws IOException { OutputStream out = new FileOutputStream(tempFile); - try { - cropped.compress(getCompressFormatForType(mimeType), COMPRESS_QUALITY, out); - } finally { - if (out != null) { - out.close(); - } - } + cropped.compress(getCompressFormatForType(mimeType), COMPRESS_QUALITY, out); } /** - * Create a temporary file in the cache directory on either internal or external storage, - * whichever is available and has more free space. + * Create a temporary file in internal / external storage to use for image scaling and caching. * * @param mimeType the MIME type of the file to create (image/*) */ - private static File createTempFile(Context context, @Nullable String mimeType) + private static File createTempFile(@Nullable File cacheDir, @Nullable String mimeType) throws IOException { - File externalCacheDir = context.getExternalCacheDir(); - File internalCacheDir = context.getCacheDir(); - File cacheDir; - if (externalCacheDir == null && internalCacheDir == null) { + if (cacheDir == null) { throw new IOException("No cache directory available"); } - if (externalCacheDir == null) { - cacheDir = internalCacheDir; - } else if (internalCacheDir == null) { - cacheDir = externalCacheDir; - } else { - cacheDir = - externalCacheDir.getFreeSpace() > internalCacheDir.getFreeSpace() - ? externalCacheDir - : internalCacheDir; - } return File.createTempFile(TEMP_FILE_PREFIX, getFileExtensionForType(mimeType), cacheDir); }