[camera_web] Upgrade package:web to ^1.0.0 and remove pkg_web_tweaks.dart - #12386
[camera_web] Upgrade package:web to ^1.0.0 and remove pkg_web_tweaks.dart#12386mdebbar wants to merge 3 commits into
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Code Review
This pull request bumps the minimum package:web dependency to 1.0.0 and removes the local pkg_web_tweaks.dart helper file, updating the codebase to use standard APIs directly. The review feedback highlights several potential runtime errors (such as TypeError and NoSuchMethodError) that could occur on browsers where properties like torch, zoom, or facingMode are undefined. To prevent these crashes, the reviewer recommends using jsUtil.hasProperty to safely verify the existence of these properties before accessing them.
| final web.MediaTrackSupportedConstraints supportedConstraints = mediaDevices | ||
| .getSupportedConstraints(); | ||
| final bool torchModeSupported = supportedConstraints.torchNullable ?? false; | ||
| final bool torchModeSupported = supportedConstraints.torch; |
There was a problem hiding this comment.
On browsers that do not support the torch constraint (such as Firefox or Safari), supportedConstraints.torch will be undefined at runtime. In Dart's JS interop, accessing an undefined property declared as a non-nullable bool will throw a TypeError because null is not a subtype of bool. Use _cameraService.jsUtil.hasProperty to safely check if the property exists before accessing it.
| final bool torchModeSupported = supportedConstraints.torch; | |
| final bool torchModeSupported = _cameraService.jsUtil.hasProperty(supportedConstraints, 'torch'.toJS) && | |
| supportedConstraints.torch; |
| final web.MediaStreamTrack defaultVideoTrack = videoTracks.first; | ||
| final bool canEnableTorchMode = | ||
| defaultVideoTrack.getCapabilities().torchNullable?.toDart.first.toDart ?? false; | ||
| final bool canEnableTorchMode = defaultVideoTrack.getCapabilities().torch.toDart.first.toDart; |
There was a problem hiding this comment.
If the camera or browser does not support torch capabilities, defaultVideoTrack.getCapabilities().torch will be undefined (which evaluates to null in Dart). Calling .toDart on it will throw a runtime error. Use _cameraService.jsUtil.hasProperty to safely check if the torch capability exists first.
final web.MediaTrackCapabilities capabilities = defaultVideoTrack.getCapabilities();
final bool canEnableTorchMode = _cameraService.jsUtil.hasProperty(capabilities, 'torch'.toJS) &&
capabilities.torch.toDart.isNotEmpty &&
capabilities.torch.toDart.first.toDart;| final String facingMode = defaultVideoTrackSettings.facingMode; | ||
|
|
||
| if (facingMode != null && facingMode.isNotEmpty) { | ||
| if (facingMode.isNotEmpty) { |
There was a problem hiding this comment.
If facingMode is not supported or returned in the video track settings (common on desktop browsers), defaultVideoTrackSettings.facingMode will be undefined (which evaluates to null in Dart). Calling .isNotEmpty on it will throw a NoSuchMethodError. Use _cameraService.jsUtil.hasProperty to safely check if the property exists and handle it as nullable.
| final String facingMode = defaultVideoTrackSettings.facingMode; | |
| if (facingMode != null && facingMode.isNotEmpty) { | |
| if (facingMode.isNotEmpty) { | |
| final String? facingMode = _cameraService.jsUtil.hasProperty(defaultVideoTrackSettings, 'facingMode'.toJS) | |
| ? defaultVideoTrackSettings.facingMode | |
| : null; | |
| if (facingMode != null && facingMode.isNotEmpty) { |
| final web.MediaTrackSupportedConstraints supportedConstraints = mediaDevices | ||
| .getSupportedConstraints(); | ||
| final bool zoomLevelSupported = supportedConstraints.zoomNullable ?? false; | ||
| final bool zoomLevelSupported = supportedConstraints.zoom; |
There was a problem hiding this comment.
On browsers that do not support the zoom constraint (such as Firefox or Safari), supportedConstraints.zoom will be undefined at runtime. Accessing it directly as a non-nullable bool will throw a TypeError. Use jsUtil.hasProperty to safely check if the property exists first.
| final bool zoomLevelSupported = supportedConstraints.zoom; | |
| final bool zoomLevelSupported = jsUtil.hasProperty(supportedConstraints, 'zoom'.toJS) && | |
| supportedConstraints.zoom; |
| final web.MediaSettingsRange zoomLevelCapability = defaultVideoTrack.getCapabilities().zoom; | ||
|
|
||
| return ZoomLevelCapability( | ||
| minimum: zoomLevelCapability.min, | ||
| maximum: zoomLevelCapability.max, | ||
| videoTrack: defaultVideoTrack, | ||
| ); |
There was a problem hiding this comment.
If the camera or browser does not support zoom capabilities, defaultVideoTrack.getCapabilities().zoom will be undefined (which evaluates to null in Dart). Accessing its properties or returning it directly will cause a runtime crash. Use jsUtil.hasProperty to check if the capability exists, and throw the appropriate CameraWebException if it does not.
final web.MediaTrackCapabilities capabilities = defaultVideoTrack.getCapabilities();
if (!jsUtil.hasProperty(capabilities, 'zoom'.toJS)) {
throw CameraWebException(
camera.textureId,
CameraErrorCode.zoomLevelNotSupported,
'The zoom level is not supported by the current camera.',
);
}
final web.MediaSettingsRange zoomLevelCapability = capabilities.zoom;
return ZoomLevelCapability(
minimum: zoomLevelCapability.min,
maximum: zoomLevelCapability.max,
videoTrack: defaultVideoTrack,
);| final String facingMode = videoTrackSettings.facingMode; | ||
|
|
||
| if (facingMode == null || facingMode.isEmpty) { | ||
| if (facingMode.isEmpty) { |
There was a problem hiding this comment.
If facingMode is not supported or returned in the video track settings, videoTrackSettings.facingMode will be undefined (which evaluates to null in Dart). Calling .isEmpty on it will throw a NoSuchMethodError. Use jsUtil.hasProperty to safely check if the property exists and handle it as nullable.
| final String facingMode = videoTrackSettings.facingMode; | |
| if (facingMode == null || facingMode.isEmpty) { | |
| if (facingMode.isEmpty) { | |
| final String? facingMode = jsUtil.hasProperty(videoTrackSettings, 'facingMode'.toJS) | |
| ? videoTrackSettings.facingMode | |
| : null; | |
| if (facingMode == null || facingMode.isEmpty) { |
Upgrading
package:webdependency constraint to^1.0.0incamera_weballows removing the temporary bridge filepkg_web_tweaks.dartand using native WebIDL bindings directly.Part of flutter/flutter#190355.
Related to PR #12384.