[go: up one dir, main page]

Fix CCA user facing camera is not used as default bug.

This CL removed the undefined label of FRONT_CAMERA_LABEL used to judge
front camera. And allowed camera facing be distinguished on HALv1 device
with facing configuration so as to be correcly collected by user metrics.

Bug: 969628
Test: On HALv3 and HALv1 device with facing configuration the facing can be
correctly be set and collected by user metrics. On HALv1 device without
facing configuration, the facing is set to empty string as intended.

(cherry picked from commit 2b1f2b73e58abd9080b2dc2148c510ba5130515d)

Change-Id: I5931d11009089790d78a6f4ace284f37beb139aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1644526
Reviewed-by: Sheng-hao Tsao <shenghao@chromium.org>
Commit-Queue: Kuo Jen Wei <inker@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#667513}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1652891
Reviewed-by: Kuo Jen Wei <inker@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#246}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/chrome/browser/resources/chromeos/camera/src/js/metrics.js b/chrome/browser/resources/chromeos/camera/src/js/metrics.js
index 61919b61..4f956f2 100644
--- a/chrome/browser/resources/chromeos/camera/src/js/metrics.js
+++ b/chrome/browser/resources/chromeos/camera/src/js/metrics.js
@@ -93,7 +93,7 @@
 
 /**
  * Returns event builder for the metrics type: capture.
- * @param {string} facingMode Camera facing-mode of the capture.
+ * @param {?string} facingMode Camera facing-mode of the capture.
  * @param {number=} length Length of 1 minute buckets for captured video.
  * @param {number} width The width of the capture resolution.
  * @param {number} height The height of the capture resolution.
@@ -115,7 +115,7 @@
   return cca.metrics.base_.category('capture')
       .action(/^(\w*)/.exec(condState(
           ['video-mode', 'photo-mode', 'square-mode', 'portrait-mode']))[0])
-      .label(facingMode)
+      .label(facingMode || '(not set)')
       .dimen(3, condState(['sound']))
       .dimen(4, condState(['mirror']))
       .dimen(5, condState(['_3x3', '_4x4', 'golden'], 'grid'))
diff --git a/chrome/browser/resources/chromeos/camera/src/js/views/camera.js b/chrome/browser/resources/chromeos/camera/src/js/views/camera.js
index 51e53325..663b952 100644
--- a/chrome/browser/resources/chromeos/camera/src/js/views/camera.js
+++ b/chrome/browser/resources/chromeos/camera/src/js/views/camera.js
@@ -99,10 +99,10 @@
       });
 
   /**
-   * @type {string}
+   * @type {?string}
    * @private
    */
-  this.facingMode_ = '';
+  this.facingMode_ = null;
 
   /**
    * @type {boolean}
@@ -264,13 +264,17 @@
 /**
  * Try start stream reconfiguration with specified device id.
  * @async
- * @param {string} deviceId
+ * @param {?string} deviceId
  * @return {boolean} If found suitable stream and reconfigure successfully.
  */
 cca.views.Camera.prototype.startWithDevice_ = async function(deviceId) {
   let supportedModes = null;
   for (const mode of this.modes_.getModeCandidates()) {
     try {
+      if (!deviceId) {
+        // Null for requesting default camera on HALv1.
+        throw new Error;
+      }
       const previewRs = (await this.options_.getDeviceResolutions(deviceId))[1];
       var resolCandidates =
           this.modes_.getResolutionCandidates(mode, deviceId, previewRs);
@@ -291,7 +295,8 @@
             }
           }
           await this.preview_.start(stream);
-          this.facingMode_ = this.options_.updateValues(constraints, stream);
+          this.facingMode_ =
+              await this.options_.updateValues(constraints, stream);
           await this.modes_.updateModeSelectionUI(supportedModes);
           await this.modes_.updateMode(
               mode, stream, deviceId, captureResolution);
diff --git a/chrome/browser/resources/chromeos/camera/src/js/views/camera/modes.js b/chrome/browser/resources/chromeos/camera/src/js/views/camera/modes.js
index 03fe9e9b..291fd7d4 100644
--- a/chrome/browser/resources/chromeos/camera/src/js/views/camera/modes.js
+++ b/chrome/browser/resources/chromeos/camera/src/js/views/camera/modes.js
@@ -79,7 +79,7 @@
           new cca.views.camera.Video(this.stream_, this.doSavePicture_),
       isSupported: async () => true,
       resolutionConfig: videoResolPreferrer,
-      v1Config: cca.views.camera.Modes.videoConstraits,
+      v1Config: cca.views.camera.Modes.getV1Constraints.bind(this, true),
       nextMode: 'photo-mode',
     },
     'photo-mode': {
@@ -87,7 +87,7 @@
           this.stream_, this.doSavePicture_, this.captureResolution_),
       isSupported: async () => true,
       resolutionConfig: photoResolPreferrer,
-      v1Config: cca.views.camera.Modes.photoConstraits,
+      v1Config: cca.views.camera.Modes.getV1Constraints.bind(this, false),
       nextMode: 'square-mode',
     },
     'square-mode': {
@@ -95,7 +95,7 @@
           this.stream_, this.doSavePicture_, this.captureResolution_),
       isSupported: async () => true,
       resolutionConfig: photoResolPreferrer,
-      v1Config: cca.views.camera.Modes.photoConstraits,
+      v1Config: cca.views.camera.Modes.getV1Constraints.bind(this, false),
       nextMode: 'portrait-mode',
     },
     'portrait-mode': {
@@ -117,7 +117,7 @@
         }
       },
       resolutionConfig: photoResolPreferrer,
-      v1Config: cca.views.camera.Modes.photoConstraits,
+      v1Config: cca.views.camera.Modes.getV1Constraints.bind(this, false),
       nextMode: 'video-mode',
     },
   };
@@ -162,14 +162,15 @@
 };
 
 /**
- * Returns a set of available video constraints for HALv1 device.
+ * Returns a set of available constraints for HALv1 device.
+ * @param {boolean} videoMode Is getting constraints for video mode.
  * @param {?string} deviceId Id of video device.
  * @return {Array<Object>} Result of constraints-candidates.
  */
-cca.views.camera.Modes.videoConstraits = function(deviceId) {
+cca.views.camera.Modes.getV1Constraints = function(videoMode, deviceId) {
   return [
     {
-      aspectRatio: {ideal: 1.7777777778},
+      aspectRatio: {ideal: videoMode ? 1.7777777778 : 1.3333333333},
       width: {min: 1280},
       frameRate: {min: 24},
     },
@@ -178,30 +179,14 @@
       frameRate: {min: 24},
     },
   ].map((constraint) => {
-    constraint.deviceId = {exact: deviceId};
-    return {audio: true, video: constraint};
-  });
-};
-
-/**
- * Returns a set of available photo constraints for HALv1 device.
- * @param {?string} deviceId Id of video device.
- * @return {Array<Object>} Result of constraints-candidates.
- */
-cca.views.camera.Modes.photoConstraits = function(deviceId) {
-  return [
-    {
-      aspectRatio: {ideal: 1.3333333333},
-      width: {min: 1280},
-      frameRate: {min: 24},
-    },
-    {
-      width: {min: 640},
-      frameRate: {min: 24},
-    },
-  ].map((constraint) => {
-    constraint.deviceId = {exact: deviceId};
-    return {audio: false, video: constraint};
+    if (deviceId) {
+      constraint.deviceId = {exact: deviceId};
+    } else {
+      // HALv1 devices are unable to know facing before stream configuration,
+      // deviceId is set to null for requesting user facing camera as default.
+      constraint.facingMode = {exact: 'user'};
+    }
+    return {audio: videoMode, video: constraint};
   });
 };
 
@@ -255,7 +240,7 @@
  * Gets capture resolution and its corresponding preview constraints for the
  * given mode on camera HALv1 device.
  * @param {string} mode
- * @param {string} deviceId
+ * @param {?string} deviceId
  * @return {Array<[?[number, number], Array<Object>]>} Result capture resolution
  *     width, height and constraints-candidates for its preview.
  */
@@ -299,7 +284,7 @@
  * @async
  * @param {string} mode Classname of mode to be updated.
  * @param {MediaStream} stream Stream of the new switching mode.
- * @param {string} deviceId Device id of currently working video device.
+ * @param {?string} deviceId Device id of currently working video device.
  * @param {?[number, number]} captureResolution Capturing resolution width and
  *     height.
  */
@@ -312,7 +297,7 @@
   this.stream_ = stream;
   this.captureResolution_ = captureResolution;
   this.current = this.allModes_[mode].captureFactory();
-  if (this.captureResolution_) {
+  if (deviceId && this.captureResolution_) {
     this.allModes_[mode].resolutionConfig.updateCurrentResolution(
         deviceId, ...this.captureResolution_);
   }
diff --git a/chrome/browser/resources/chromeos/camera/src/js/views/camera/options.js b/chrome/browser/resources/chromeos/camera/src/js/views/camera/options.js
index 74f20d49..63a1129 100644
--- a/chrome/browser/resources/chromeos/camera/src/js/views/camera/options.js
+++ b/chrome/browser/resources/chromeos/camera/src/js/views/camera/options.js
@@ -80,13 +80,24 @@
   this.videoDevices_ = null;
 
   /**
-   * List of available video devices and width, height of its supported video
-   * resolutions and photo resolutions.
+   * MediaDeviceInfo and additional information queried from private mojo api of
+   * all available video device. The additional fields in the array entry
+   * represent camera facing, supported photo resolutions, supported video
+   * resolutions. On HALv1 device, the promise will throw exception for its
+   * incapability of building mojo api connection.
    * @type {Promise<!Array<[MediaDeviceInfo, cros.mojom.CameraFacing, ResolList,
    *     ResolList]>>}
    * @private
    */
-  this.deviceResolutions_ = null;
+  this.devicePrivateInfo_ = null;
+
+  /**
+   * Whether the current device is HALv1 and lacks facing configuration.
+   * get facing information.
+   * @type {?boolean}
+   * private
+   */
+  this.isV1NoFacingConfig_ = null;
 
   /**
    * Mirroring set per device.
@@ -170,13 +181,23 @@
  * Updates the options' values for the current constraints and stream.
  * @param {Object} constraints Current stream constraints in use.
  * @param {MediaStream} stream Current Stream in use.
- * @return {string} Facing-mode in use.
+ * @return {?string} Facing-mode in use.
  */
-cca.views.camera.Options.prototype.updateValues = function(
-    constraints, stream) {
+cca.views.camera.Options.prototype.updateValues =
+    async function(constraints, stream) {
   var track = stream.getVideoTracks()[0];
   var trackSettings = track.getSettings && track.getSettings();
-  var facingMode = trackSettings && trackSettings.facingMode;
+  let facingMode = trackSettings && trackSettings.facingMode;
+  if (this.isV1NoFacingConfig_ === null) {
+    // Because the facing mode of external camera will be set to undefined on
+    // all devices, to distinguish HALv1 device without facing configuration,
+    // assume the first opened camera is built-in camera. Device without facing
+    // configuration won't set facing of built-in cameras. Also if HALv1 device
+    // with facing configuration opened external camera first after CCA launched
+    // the logic here may misjudge it as this category.
+    this.isV1NoFacingConfig_ = facingMode === undefined;
+  }
+  facingMode = this.isV1NoFacingConfig_ ? null : facingMode || 'external';
   this.updateVideoDeviceId_(constraints, trackSettings);
   this.updateMirroring_(facingMode);
   this.audioTrack_ = stream.getAudioTracks()[0];
@@ -265,7 +286,7 @@
     this.refreshingVideoDeviceIds_ = false;
   });
 
-  this.deviceResolutions_ =
+  this.devicePrivateInfo_ =
       this.videoDevices_
           .then((devices) => {
             return Promise.all(devices.map((d) => Promise.all([
@@ -285,14 +306,14 @@
 
   (async () => {
     try {
-      var deviceResolutions = await this.deviceResolutions_;
+      var devicePrivateInfo = await this.devicePrivateInfo_;
     } catch (e) {
       return;
     }
     let frontSetting = null;
     let backSetting = null;
     let externalSettings = [];
-    deviceResolutions.forEach(([{deviceId}, facing, photoRs, videoRs]) => {
+    devicePrivateInfo.forEach(([{deviceId}, facing, photoRs, videoRs]) => {
       const setting = [deviceId, photoRs, videoRs];
       switch (facing) {
         case cros.mojom.CameraFacing.CAMERA_FACING_FRONT:
@@ -321,28 +342,45 @@
 
 /**
  * Gets the video device ids sorted by preference.
- * @return {!Promise<!Array<string>>}
+ * @async
+ * @return {Array<?string>} May contain null for user facing camera on HALv1
+ *     devices.
+ * @throws {Error} Throws exception for no available video devices.
  */
-cca.views.camera.Options.prototype.videoDeviceIds = function() {
-  return this.videoDevices_.then((devices) => {
-    if (devices.length == 0) {
-      throw new Error('Device list empty.');
+cca.views.camera.Options.prototype.videoDeviceIds = async function() {
+  const devices = await this.videoDevices_;
+  if (devices.length == 0) {
+    throw new Error('Device list empty.');
+  }
+  try {
+    var facings = (await this.devicePrivateInfo_)
+                      .reduce(
+                          (facings, [d, facing]) =>
+                              Object.assign(facings, {[d.deviceId]: facing}),
+                          {});
+    this.isV1NoFacingConfig_ = false;
+  } catch (e) {
+    facings = null;
+  }
+  // Put the selected video device id first.
+  var sorted = devices.map((device) => device.deviceId).sort((a, b) => {
+    if (a == b) {
+      return 0;
     }
-    // Put the selected video device id first.
-    return devices
-        .sort((a, b) => {
-          if (a.deviceId == b.deviceId) {
-            return 0;
-          }
-          if (this.videoDeviceId_ ?
-                  (a.deviceId == this.videoDeviceId_) :
-                  (a.label == cca.views.camera.Options.FRONT_CAMERA_LABEL)) {
-            return -1;
-          }
-          return 1;
-        })
-        .map(({deviceId}) => deviceId);
+    if (this.videoDeviceId_ ?
+            a === this.videoDeviceId_ :
+            (facings &&
+             facings[a] === cros.mojom.CameraFacing.CAMERA_FACING_FRONT)) {
+      return -1;
+    }
+    return 1;
   });
+  // Prepended 'null' deviceId means the system default camera on HALv1
+  // device. Add it only when the app is launched (no video-device-id set).
+  if (!facings && this.videoDeviceId_ === null) {
+    sorted.unshift(null);
+  }
+  return sorted;
 };
 
 /**
@@ -355,8 +393,9 @@
  */
 cca.views.camera.Options.prototype.getDeviceResolutions =
     async function(deviceId) {
-  const deviceResolutions = await this.deviceResolutions_;
+  // HALv1 device will thrown from here.
+  const devicePrivateInfo = await this.devicePrivateInfo_;
   const [, , photoRs, videoRs] =
-      deviceResolutions.find(([d]) => d.deviceId == deviceId);
+      devicePrivateInfo.find(([d]) => d.deviceId === deviceId);
   return [photoRs, videoRs];
 };