Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/camera/camera_android_camerax/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.6.24+4

* Allows for video recording without audio when permission RECORD_AUDIO is denied.

## 0.6.24+3

* Bumps com.android.tools.build:gradle from 8.12.1 to 8.13.1.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,17 @@ public PendingRecording asPersistentRecording(PendingRecording pigeonInstance) {
@NonNull
@Override
public PendingRecording withAudioEnabled(PendingRecording pigeonInstance, boolean initialMuted) {
if (!initialMuted
&& ContextCompat.checkSelfPermission(
boolean hasPermission =
ContextCompat.checkSelfPermission(
getPigeonRegistrar().getContext(), Manifest.permission.RECORD_AUDIO)
== PackageManager.PERMISSION_GRANTED) {
return pigeonInstance.withAudioEnabled(false);
== PackageManager.PERMISSION_GRANTED;

if (hasPermission) {
return pigeonInstance.withAudioEnabled(initialMuted);
}

return pigeonInstance.withAudioEnabled(true);
// By default, the recording will not contain audio.
return pigeonInstance;
}

@NonNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -52,7 +53,6 @@ public void withAudioEnabled_doesNotEnableAudioWhenRequestedAndPermissionNotGran
final PigeonApiPendingRecording api =
new TestProxyApiRegistrar().getPigeonApiPendingRecording();
final PendingRecording instance = mock(PendingRecording.class);
final PendingRecording newInstance = mock(PendingRecording.class);

try (MockedStatic<ContextCompat> mockedContextCompat =
Mockito.mockStatic(ContextCompat.class)) {
Expand All @@ -63,10 +63,32 @@ public void withAudioEnabled_doesNotEnableAudioWhenRequestedAndPermissionNotGran
any(Context.class), eq(Manifest.permission.RECORD_AUDIO)))
.thenAnswer((Answer<Integer>) invocation -> PackageManager.PERMISSION_DENIED);

when(instance.withAudioEnabled(true)).thenReturn(newInstance);
when(instance.withAudioEnabled(false)).thenReturn(instance);

assertEquals(api.withAudioEnabled(instance, false), newInstance);
verify(instance).withAudioEnabled(true);
assertEquals(api.withAudioEnabled(instance, false), instance);
verify(instance, never()).withAudioEnabled(false);
}
}

@Test
public void withAudioEnabled_doesNotEnableAudioWhenNotRequestedAndPermissionNotGranted() {
final PigeonApiPendingRecording api =
new TestProxyApiRegistrar().getPigeonApiPendingRecording();
final PendingRecording instance = mock(PendingRecording.class);

try (MockedStatic<ContextCompat> mockedContextCompat =
Mockito.mockStatic(ContextCompat.class)) {
mockedContextCompat
.when(
() ->
ContextCompat.checkSelfPermission(
any(Context.class), eq(Manifest.permission.RECORD_AUDIO)))
.thenAnswer((Answer<Integer>) invocation -> PackageManager.PERMISSION_DENIED);

when(instance.withAudioEnabled(true)).thenReturn(instance);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make a separate test with these changes? This (withAudioEnabled_doesNotEnableAudioWhenRequestedAndPermissionNotGranted) is supposed to test when audio IS requested but permission is not granted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 i might be missing something. i'm falling to see the issue. could you help?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the original test was testing an invalid assumption. withAudioEnabled_doesNotEnableAudioWhenRequestedAndPermissionNotGranted is testing when audio IS requested with permission denied.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe so. withAudioEnabled takes a boolean initialMuted, which should be false to request audio. I'm asking if you can separate this out into two tests--one withAudioEnabled_doesNotEnableAudioWhenRequestedAndPermissionNotGranted (this one without changes) and another (for example) withAudioEnabled_doesNotEnableAudioWhenNotRequestedAndPermissionNotGranted (one that tests the case that this PR addresses).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.


assertEquals(api.withAudioEnabled(instance, true), instance);
verify(instance, never()).withAudioEnabled(true);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera_android_camerax/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: camera_android_camerax
description: Android implementation of the camera plugin using the CameraX library.
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_android_camerax
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.6.24+3
version: 0.6.24+4

environment:
sdk: ^3.9.0
Expand Down