Minor refactoring of SelectSettings results for consistency.

content/renderer/media/media_stream_constraints_util_video_device.h

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_RENDERER_MEDIA_MEDIA_STREAM_CONSTRAINTS_UTIL_VIDEO_DEVICE_H_
 #define CONTENT_RENDERER_MEDIA_MEDIA_STREAM_CONSTRAINTS_UTIL_VIDEO_DEVICE_H_
 
 #include <string>
 #include <vector>
 
+#include "base/logging.h"
 #include "content/common/content_export.h"
 #include "content/common/media/media_devices.mojom.h"
 #include "media/capture/video_capture_types.h"
 #include "third_party/webrtc/base/optional.h"
 
 namespace blink {
 class WebString;
 class WebMediaConstraints;
 }
 
@@ skipping 10 matching lines @@
   ~VideoDeviceCaptureCapabilities();
   VideoDeviceCaptureCapabilities& operator=(
       VideoDeviceCaptureCapabilities&& other);
 
   // Each field is independent of each other.
   std::vector<::mojom::VideoInputDeviceCapabilitiesPtr> device_capabilities;
   std::vector<media::PowerLineFrequency> power_line_capabilities;
   std::vector<rtc::Optional<bool>> noise_reduction_capabilities;
 };
 
-struct CONTENT_EXPORT VideoDeviceCaptureSourceSelectionResult {
+class CONTENT_EXPORT VideoDeviceCaptureSourceSelectionResult {
+ public:
+  // Creates a result without value and with an empty failed constraint name.
   VideoDeviceCaptureSourceSelectionResult();

[hta - Chromium, 2017/03/15 11:55:08]

Can we remove this constructor, since it creates an object in an inconsistent
state, and we have removed the ability to change it?

[Guido Urdaneta, 2017/03/15 19:16:47]

My original intention was to remove it, but we need it because
PostTaskAndReplyWithResult requires it to have a default constructor.
It is used here:
https://cs.chromium.org/chromium/src/content/renderer/media/user_media_client...

Also, the state is technically not inconsistent because the HasValue() method
returns false, so the other fields do not have any meaningful values and should
not be read.

+
+  // Creates a result without value and with the given |failed_constraint_name|.
+  // Does not take ownership of |failed_constraint_name|, so it must be null or
+  // point to a string that remains accessible.
+  explicit VideoDeviceCaptureSourceSelectionResult(
+      const char* failed_constraint_name);
+
+  // Creates a result with the given values. |device_id| is moved to an internal
+  // field.
   VideoDeviceCaptureSourceSelectionResult(
+      const std::string& device_id,
+      ::mojom::FacingMode facing_mode_,
+      media::VideoCaptureParams capture_params_,
+      rtc::Optional<bool> noise_reduction_);
+
+  VideoDeviceCaptureSourceSelectionResult(
+      const VideoDeviceCaptureSourceSelectionResult& other);
+  VideoDeviceCaptureSourceSelectionResult& operator=(
       const VideoDeviceCaptureSourceSelectionResult& other);
   VideoDeviceCaptureSourceSelectionResult(
       VideoDeviceCaptureSourceSelectionResult&& other);
-  ~VideoDeviceCaptureSourceSelectionResult();
-  VideoDeviceCaptureSourceSelectionResult& operator=(
-      const VideoDeviceCaptureSourceSelectionResult& other);
   VideoDeviceCaptureSourceSelectionResult& operator=(
       VideoDeviceCaptureSourceSelectionResult&& other);
+  ~VideoDeviceCaptureSourceSelectionResult();
 
-  bool HasValue() const { return failed_constraint_name == nullptr; }
+  bool HasValue() const { return failed_constraint_name_ == nullptr; }
 
   // Convenience accessors for fields embedded in |capture_params|.
   const media::VideoCaptureFormat& Format() const {
-    return capture_params.requested_format;
+    return capture_params_.requested_format;
   }
   int Width() const {
-    return capture_params.requested_format.frame_size.width();
+    DCHECK(HasValue());
+    return capture_params_.requested_format.frame_size.width();
   }
   int Height() const {
-    return capture_params.requested_format.frame_size.height();
+    DCHECK(HasValue());
+    return capture_params_.requested_format.frame_size.height();
   }
-  float FrameRate() const { return capture_params.requested_format.frame_rate; }
+  float FrameRate() const {
+    DCHECK(HasValue());
+    return capture_params_.requested_format.frame_rate;
+  }
   media::PowerLineFrequency PowerLineFrequency() const {
-    return capture_params.power_line_frequency;
+    DCHECK(HasValue());
+    return capture_params_.power_line_frequency;
   }
 
-  const char* failed_constraint_name;
-  std::string device_id;
-  ::mojom::FacingMode facing_mode;
-  media::VideoCaptureParams capture_params;
-  rtc::Optional<bool> noise_reduction;
+  // Other accessors.
+  const char* failed_constraint_name() const { return failed_constraint_name_; }
+
+  const std::string& device_id() const {
+    DCHECK(HasValue());
+    return device_id_;
+  }
+
+  ::mojom::FacingMode facing_mode() const {
+    DCHECK(HasValue());
+    return facing_mode_;
+  }
+
+  const media::VideoCaptureParams& capture_params() const {
+    DCHECK(HasValue());
+    return capture_params_;
+  }
+
+  const rtc::Optional<bool>& noise_reduction() const {
+    DCHECK(HasValue());
+    return noise_reduction_;
+  }
+
+ private:
+  const char* failed_constraint_name_;
+  std::string device_id_;
+  ::mojom::FacingMode facing_mode_;
+  media::VideoCaptureParams capture_params_;
+  rtc::Optional<bool> noise_reduction_;
 };
 
 // This function performs source and source-settings selection based on
 // the given |capabilities| and |constraints|.
 // Chromium performs constraint resolution in two steps. First, a source and its
 // settings are selected; then a track is created, connected to the source, and
 // finally the track settings are selected. This function implements an
 // algorithm for the first step. Sources are not a user-visible concept, so the
 // spec only specifies an algorithm for track settings.
 // This algorithm for sources is compatible with the spec algorithm for tracks,
@@ skipping 52 matching lines @@
 //    reduction, resolution, and frame rate, in that order. Note that there is
 //    no default facing mode or aspect ratio.
 VideoDeviceCaptureSourceSelectionResult CONTENT_EXPORT
 SelectVideoDeviceCaptureSourceSettings(
     const VideoDeviceCaptureCapabilities& capabilities,
     const blink::WebMediaConstraints& constraints);
 
 }  // namespace content
 
 #endif  // CONTENT_RENDERER_MEDIA_MEDIA_STREAM_CONSTRAINTS_UTIL_VIDEO_DEVICE_H_