Change class VideoCaptureDevice::Name to struct VideoCaptureDeviceDescriptor

content/browser/media/media_internals_unittest.cc

 // Copyright (c) 2011 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.
 
 #include "content/browser/media/media_internals.h"
 
 #include <stddef.h>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 94 matching lines @@
   }
 
   ~MediaInternalsVideoCaptureDeviceTest() override {
     media_internals_->RemoveUpdateCallback(update_cb_);
   }
 
  protected:
   MediaInternals::UpdateCallback update_cb_;
 };
 
+// TODO(chfremer): This test does not seem to be very useful. All it does it
+// to provide a duplicate implementation of the functionality under test.
+// https://crbug.com/630694

[mcasas, 2016/07/26 23:52:03]

What's the TODO? :)

The original meaning of the test was to make sure,
by breaking, taht developers would update media_internals
code when they updated other parts of media/capture,
e.g. [1]. nisse@ had a go at it but couldn't do much.


[1]
https://cs.chromium.org/chromium/src/content/browser/media/media_internals.cc...

[chfremer, 2016/07/27 23:10:17]

The TODO is to remove the test (see also https://crbug.com/630694) unless we can
convince ourselves that there is value in keeping it. I am currently not quite
convinced of its value.

I don't understand what to look at at the code you referenced in [1].
I see there is an invocation of GetCaptureApiTypeString(), but I don't see the
connection to this test. Could you please elaborate?

[1]
https://cs.chromium.org/chromium/src/content/browser/media/media_internals.cc...

[mcasas, 2016/07/28 00:05:42]

Then add some action to the TODO :) like "consider removing...".

[chfremer, 2016/07/29 18:09:50]

Done.

 #if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_LINUX) || \
     defined(OS_ANDROID)
 TEST_F(MediaInternalsVideoCaptureDeviceTest,
        AllCaptureApiTypesHaveProperStringRepresentation) {
-  typedef media::VideoCaptureDevice::Name VideoCaptureDeviceName;
-  typedef std::map<VideoCaptureDeviceName::CaptureApiType, std::string>
-      CaptureApiTypeStringMap;
+  using VideoCaptureApiType = media::VideoCaptureApiType;
+  typedef std::map<VideoCaptureApiType, std::string> CaptureApiTypeStringMap;
   CaptureApiTypeStringMap m;
-#if defined(OS_LINUX)
-  m[VideoCaptureDeviceName::V4L2_SINGLE_PLANE] = "V4L2 SPLANE";
-#elif defined(OS_WIN)
-  m[VideoCaptureDeviceName::MEDIA_FOUNDATION] = "Media Foundation";
-  m[VideoCaptureDeviceName::DIRECT_SHOW] = "Direct Show";
-#elif defined(OS_MACOSX)
-  m[VideoCaptureDeviceName::AVFOUNDATION] = "AV Foundation";
-  m[VideoCaptureDeviceName::DECKLINK] = "DeckLink";
-#elif defined(OS_ANDROID)
-  m[VideoCaptureDeviceName::API1] = "Camera API1";
-  m[VideoCaptureDeviceName::API2_LEGACY] = "Camera API2 Legacy";
-  m[VideoCaptureDeviceName::API2_FULL] = "Camera API2 Full";
-  m[VideoCaptureDeviceName::API2_LIMITED] = "Camera API2 Limited";
-  m[VideoCaptureDeviceName::TANGO] = "Tango API";
-#endif
-  EXPECT_EQ(media::VideoCaptureDevice::Name::API_TYPE_UNKNOWN, m.size());
+  m[VideoCaptureApiType::LINUX_V4L2_SINGLE_PLANE] = "V4L2 SPLANE";
+  m[VideoCaptureApiType::WINDOWS_MEDIA_FOUNDATION] = "Media Foundation";
+  m[VideoCaptureApiType::WINDOWS_DIRECT_SHOW] = "Direct Show";
+  m[VideoCaptureApiType::MACOSX_AVFOUNDATION] = "AV Foundation";
+  m[VideoCaptureApiType::MACOSX_DECKLINK] = "DeckLink";
+  m[VideoCaptureApiType::ANDROID_API1] = "Camera API1";
+  m[VideoCaptureApiType::ANDROID_API2_LEGACY] = "Camera API2 Legacy";
+  m[VideoCaptureApiType::ANDROID_API2_FULL] = "Camera API2 Full";
+  m[VideoCaptureApiType::ANDROID_API2_LIMITED] = "Camera API2 Limited";
+  m[VideoCaptureApiType::ANDROID_TANGO] = "Tango API";
+  EXPECT_EQ(static_cast<size_t>(VideoCaptureApiType::API_TYPE_UNKNOWN),
+            m.size());
   for (CaptureApiTypeStringMap::iterator it = m.begin(); it != m.end(); ++it) {
-    const VideoCaptureDeviceName device_name("dummy", "dummy", it->first);
-    EXPECT_EQ(it->second, device_name.GetCaptureApiTypeString());
+    media::VideoCaptureDeviceDescriptor descriptor;
+    descriptor.capture_api = it->first;
+    EXPECT_EQ(it->second, descriptor.GetCaptureApiTypeString());
   }
 }
 #endif
 
 TEST_F(MediaInternalsVideoCaptureDeviceTest,
        VideoCaptureFormatStringIsInExpectedFormat) {
   // Since media internals will send video capture capabilities to JavaScript in
   // an expected format and there are no public methods for accessing the
   // resolutions, frame rates or pixel formats, this test checks that the format
   // has not changed. If the test fails because of the changed format, it should
@@ skipping 18 matching lines @@
        NotifyVideoCaptureDeviceCapabilitiesEnumerated) {
   const int kWidth = 1280;
   const int kHeight = 720;
   const float kFrameRate = 30.0f;
   const media::VideoPixelFormat kPixelFormat =
       media::PIXEL_FORMAT_I420;
   const media::VideoCaptureFormat format_hd({kWidth, kHeight},
       kFrameRate, kPixelFormat);
   media::VideoCaptureFormats formats{};
   formats.push_back(format_hd);
-  const media::VideoCaptureDeviceInfo device_info(
+  media::VideoCaptureDeviceInfo device_info;
+  device_info.supported_formats = formats;
+  device_info.descriptor.device_id = "dummy";
+  device_info.descriptor.friendly_name = "dummy";
 #if defined(OS_MACOSX)
-      media::VideoCaptureDevice::Name(
-          "dummy", "dummy", media::VideoCaptureDevice::Name::AVFOUNDATION),
+  device_info.descriptor.capture_api =
+      media::VideoCaptureApiType::MACOSX_AVFOUNDATION;
 #elif defined(OS_WIN)
-      media::VideoCaptureDevice::Name("dummy", "dummy",
-          media::VideoCaptureDevice::Name::DIRECT_SHOW),
+  device_info.descriptor.capture_api =
+      media::VideoCaptureApiType::WINDOWS_DIRECT_SHOW;
 #elif defined(OS_LINUX)
-      media::VideoCaptureDevice::Name(
-          "dummy", "/dev/dummy",
-          media::VideoCaptureDevice::Name::V4L2_SINGLE_PLANE),
+  device_info.descriptor.device_id = "/dev/dummy";
+  device_info.descriptor.capture_api =
+      media::VideoCaptureApiType::LINUX_V4L2_SINGLE_PLANE;
 #elif defined(OS_ANDROID)
-      media::VideoCaptureDevice::Name("dummy", "dummy",
-          media::VideoCaptureDevice::Name::API2_LEGACY),
-#else
-      media::VideoCaptureDevice::Name("dummy", "dummy"),
+  device_info.descriptor.capture_api =
+      media::VideoCaptureApiType::ANDROID_API2_LEGACY;
 #endif
-      formats);
   media::VideoCaptureDeviceInfos device_infos{};
   device_infos.push_back(device_info);
 
   // When updating video capture capabilities, the update will serialize
   // a JSON array of objects to string. So here, the |UpdateCallbackImpl| will
   // deserialize the first object in the array. This means we have to have
   // exactly one device_info in the |device_infos|.
   media_internals_->UpdateVideoCaptureDeviceCapabilities(device_infos);
 
 #if defined(OS_LINUX)
@@ skipping 98 matching lines @@
   ExpectStatus("closed");
 }
 
 INSTANTIATE_TEST_CASE_P(
     MediaInternalsAudioLogTest, MediaInternalsAudioLogTest, testing::Values(
         media::AudioLogFactory::AUDIO_INPUT_CONTROLLER,
         media::AudioLogFactory::AUDIO_OUTPUT_CONTROLLER,
         media::AudioLogFactory::AUDIO_OUTPUT_STREAM));
 
 }  // namespace content