Minor refactoring of SelectSettings results for consistency.

content/renderer/media/media_stream_constraints_util_video_device.h

Index: content/renderer/media/media_stream_constraints_util_video_device.h
diff --git a/content/renderer/media/media_stream_constraints_util_video_device.h b/content/renderer/media/media_stream_constraints_util_video_device.h
index 71a19254db745f9e1984e52c4df6f49a0aae41c5..8b422694dede812ab8e7ab1c865fdccb745d0e06 100644
--- a/content/renderer/media/media_stream_constraints_util_video_device.h
+++ b/content/renderer/media/media_stream_constraints_util_video_device.h
@@ -8,6 +8,7 @@
 #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"
@@ -38,40 +39,87 @@ struct CONTENT_EXPORT VideoDeviceCaptureCapabilities {
   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 VideoDeviceCaptureSourceSelectionResult& other);
+      const std::string& device_id,
+      ::mojom::FacingMode facing_mode_,
+      media::VideoCaptureParams capture_params_,
+      rtc::Optional<bool> noise_reduction_);
+
   VideoDeviceCaptureSourceSelectionResult(
-      VideoDeviceCaptureSourceSelectionResult&& other);
-  ~VideoDeviceCaptureSourceSelectionResult();
+      const VideoDeviceCaptureSourceSelectionResult& other);
   VideoDeviceCaptureSourceSelectionResult& operator=(
       const VideoDeviceCaptureSourceSelectionResult& other);
+  VideoDeviceCaptureSourceSelectionResult(
+      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 {
+    DCHECK(HasValue());
+    return capture_params_.requested_format.frame_rate;
   }
-  float FrameRate() const { 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;
+  }
+
+  // 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_;
   }
 
-  const char* failed_constraint_name;
-  std::string device_id;
-  ::mojom::FacingMode facing_mode;
-  media::VideoCaptureParams capture_params;
-  rtc::Optional<bool> 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