0

Don't use photo picker for video capture

It is strange shouldUsePhotoPicker() checks whether this is a image capture, but doesn't do the same for video capture.
This causes an issue that when selectFile() is called, it will skip all
the permission checks and go directly to call
launchSelectedFileIntent(). And launchSelectFileIntent() only invokes
camera Intent if Chrome has camera permission.

This CL will disable photopicker if the file selection is video capture.
So we will always go through permission check before calling
launchSelectFileIntent().

Bug: 374606647
Change-Id: I39d382d611a256bd22fdbcb34548ff338702617e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5963378
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1374657}
This commit is contained in:
Min Qin
2024-10-28 16:17:06 +00:00
committed by Chromium LUCI CQ
parent 89bce857de
commit 341386faa0
5 changed files with 75 additions and 5 deletions

@ -690,14 +690,18 @@ public class SelectFileDialog implements WindowAndroid.IntentCallback, PhotoPick
/**
* Determines whether the photo picker should be used for this select file request. To be
* applicable for the photo picker, the following must be true:
* 1.) Only media types were requested in the file request
* 2.) The file request did not explicitly ask to capture camera directly.
* 3.) The photo picker is supported by the embedder (i.e. Chrome).
* 4.) There is a valid Android Activity associated with the file request.
* applicable for the photo picker, the following must be true: 1.) Only media types were
* requested in the file request 2.) The file request did not explicitly ask to capture camera
* directly. 3.) The photo picker is supported by the embedder (i.e. Chrome). 4.) There is a
* valid Android Activity associated with the file request.
*/
private boolean shouldUsePhotoPicker() {
boolean isSupportedVideoType =
!UiAndroidFeatureMap.isEnabled(
UiAndroidFeatures.DISABLE_PHOTO_PICKER_FOR_VIDEO_CAPTURE)
|| !captureVideo();
return !captureImage()
&& isSupportedVideoType
&& isSupportedPhotoPickerTypes(mMimeTypes)
&& shouldShowPhotoPicker()
&& mWindowAndroid.getActivity().get() != null;

@ -63,6 +63,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
/** Tests logic in the SelectFileDialog class. */
@RunWith(BaseRobolectricTestRunner.class)
@ -71,6 +72,7 @@ import java.util.List;
UiAndroidFeatures.DEPRECATED_EXTERNAL_PICKER_FUNCTION,
UiAndroidFeatures.SELECT_FILE_OPEN_DOCUMENT
})
@EnableFeatures({UiAndroidFeatures.DISABLE_PHOTO_PICKER_FOR_VIDEO_CAPTURE})
@LooperMode(LooperMode.Mode.PAUSED)
public class SelectFileDialogTest {
// A callback that fires when the file selection pipeline shuts down as a result of an action.
@ -683,6 +685,62 @@ public class SelectFileDialogTest {
selectFileDialog.resetFileSelectionAttempts();
}
@Test
public void testVideoCaptureRequestPermissionSuccess() throws Exception {
TestSelectFileDialog selectFileDialog = new TestSelectFileDialog(0);
WindowAndroid windowAndroid = Mockito.mock(WindowAndroid.class);
when(windowAndroid.hasPermission(Manifest.permission.CAMERA))
.thenReturn(false)
.thenReturn(true);
IntentArgumentMatcher videoCaptureIntentArgumentMatcher =
new IntentArgumentMatcher(new Intent(MediaStore.ACTION_VIDEO_CAPTURE));
when(windowAndroid.canResolveActivity(
ArgumentMatchers.argThat(videoCaptureIntentArgumentMatcher)))
.thenReturn(true);
// Setup the request callback to simulate an interrupted permission flow.
Mockito.doAnswer(
(invocation) -> {
PermissionCallback callback =
(PermissionCallback) invocation.getArguments()[1];
callback.onRequestPermissionsResult(
new String[] {Manifest.permission.CAMERA},
new int[] {PackageManager.PERMISSION_GRANTED});
return null;
})
.when(windowAndroid)
.requestPermissions(
aryEq(new String[] {Manifest.permission.CAMERA}),
(PermissionCallback) any());
AtomicBoolean cameraIntentShow = new AtomicBoolean(false);
IntentArgumentMatcher chooserIntentArgumentMatcher =
new IntentArgumentMatcher(new Intent(MediaStore.ACTION_VIDEO_CAPTURE));
Mockito.doAnswer(
(invocation) -> {
cameraIntentShow.set(true);
return true;
})
.when(windowAndroid)
.showIntent(
ArgumentMatchers.argThat(chooserIntentArgumentMatcher),
(WindowAndroid.IntentCallback) any(),
anyInt());
// Ensure permission request in selectFile can handle interrupted permission flow.
selectFileDialog.selectFile(
Intent.ACTION_GET_CONTENT,
new String[] {"video/*"},
/* capture= */ true,
/* multiple= */ false,
windowAndroid);
assertTrue(cameraIntentShow.get());
selectFileDialog.resetFileSelectionAttempts();
}
/** Returns the determined scope for the accepted |fileTypes|. */
private int scopeForFileTypes(String... fileTypes) {
SelectFileDialog instance = SelectFileDialog.create((long) /* nativeSelectFileDialog= */ 0);

@ -26,6 +26,7 @@ const base::Feature* const kFeaturesExposedToJava[] = {
&ui::kRequireLeadingInTextViewWithLeading,
&ui::kSelectFileOpenDocument,
&ui::kCheckIntentCallerPermission,
&ui::kDisablePhotoPickerForVideoCapture,
};
// static

@ -44,4 +44,8 @@ BASE_FEATURE(kSendTouchMovesToEventForwarderObservers,
BASE_FEATURE(kCheckIntentCallerPermission,
"CheckIntentCallerPermission",
base::FEATURE_ENABLED_BY_DEFAULT);
BASE_FEATURE(kDisablePhotoPickerForVideoCapture,
"DisablePhotoPickerForVideoCapture",
base::FEATURE_ENABLED_BY_DEFAULT);
} // namespace ui

@ -52,6 +52,9 @@ UI_ANDROID_EXPORT BASE_DECLARE_FEATURE(
// When launching an intent, check whether the caller has the permission to
// access a URI before returning the result.
UI_ANDROID_EXPORT BASE_DECLARE_FEATURE(kCheckIntentCallerPermission);
// Whether photo picker should be disabled for video capture.
UI_ANDROID_EXPORT BASE_DECLARE_FEATURE(kDisablePhotoPickerForVideoCapture);
} // namespace ui
#endif // UI_ANDROID_UI_ANDROID_FEATURES_H_