Remove VideoCaptureDeviceFactory::EnumerateDeviceDescriptors()

Remove VideoCaptureDeviceFactory::EnumerateDeviceDescriptors()

The method VideoCaptureDeviceFactory::EnumerateDeviceDescriptors()
is the asynchronous equivalent of method VideoCaptureDeviceFactory
::GetDeviceDescriptors(). Its current implementation delegates to
GetDeviceDescriptors() and invokes the callback synchronously.

This CL removes EnumerateDeviceDescriptors() and lets all clients
call GetDeviceDescriptors() directly.

BUG=708233
Review-Url: https://codereview.chromium.org/2805863002
Cr-Commit-Position: refs/heads/master@{#463346}
Committed: https://chromium.googlesource.com/chromium/src/+/71618f5681772f3d76c8c9c1f7f24490074db4b3

[Chandan, 2017-04-07 11:20:21]

c.padhi@samsung.com changed reviewers:
+ chfremer@chromium.org, mcasas@chromium.org

[Chandan, 2017-04-07 11:20:22]

PTAL.Thank you.

[chfremer, 2017-04-07 16:34:21]

Overall looking good.
Unfortunately my CL https://codereview.chromium.org/2805533002/ was still in the
process of landing and this will cause merge conflicts. Assuming it does not get
reverted again, could you please rebase after it has landed and then
merge/reapply your changes?

https://codereview.chromium.org/2805863002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/video_capture_manager.cc (right):

https://codereview.chromium.org/2805863002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/video_capture_manager.cc:232:
base::Bind(&VideoCaptureManager::ConsolidateDevicesInfoOnDeviceThread,
As a heads up, I am sorry about the extra work this means, but there is a CL
still in flight and about to land that will conflict with this, see
https://codereview.chromium.org/2805533002/. There, the call to
EnumerateDeviceDescriptors has moved to a new class VideoCaptureSystem.
The changes you made here will probably have to be applied there instead.

https://codereview.chromium.org/2805863002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/video_capture_manager.cc:767: void
VideoCaptureManager::ConsolidateDevicesInfoOnDeviceThread(
Now that this method includes the logic for pulling new device descriptors from
the factory, we may want to rename it. Maybe "UpdateDeviceInfosOnDeviceThread"?

https://codereview.chromium.org/2805863002/diff/1/media/capture/video/fake_vi...
File media/capture/video/fake_video_capture_device_unittest.cc (right):

https://codereview.chromium.org/2805863002/diff/1/media/capture/video/fake_vi...
media/capture/video/fake_video_capture_device_unittest.cc:228:
std::unique_ptr<VideoCaptureDeviceDescriptors> descriptors_;
Does this need to be a std::unique_ptr<> or can we use
VideoCaptureDeviceDescriptors directly?

https://codereview.chromium.org/2805863002/diff/1/media/capture/video/fake_vi...
media/capture/video/fake_video_capture_device_unittest.cc:254:
descriptors_->clear();
I believe that the gtest framework will create a new instance of the test class
for each test run, so this should not be needed.

[Chandan, 2017-04-10 09:18:23]

c.padhi@samsung.com changed reviewers:
+ a.suchit@chromium.org

[Chandan, 2017-04-10 09:18:24]

Uploaded patchset 2. Please review. Thank you.

https://codereview.chromium.org/2805863002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/video_capture_manager.cc (right):

https://codereview.chromium.org/2805863002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/video_capture_manager.cc:232:
base::Bind(&VideoCaptureManager::ConsolidateDevicesInfoOnDeviceThread,
On 2017/04/07 16:34:21, chfremer wrote:
> As a heads up, I am sorry about the extra work this means, but there is a CL
> still in flight and about to land that will conflict with this, see
> https://codereview.chromium.org/2805533002/. There, the call to
> EnumerateDeviceDescriptors has moved to a new class VideoCaptureSystem.
> The changes you made here will probably have to be applied there instead.

Done.

https://codereview.chromium.org/2805863002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/video_capture_manager.cc:767: void
VideoCaptureManager::ConsolidateDevicesInfoOnDeviceThread(
On 2017/04/07 16:34:21, chfremer wrote:
> Now that this method includes the logic for pulling new device descriptors
from
> the factory, we may want to rename it. Maybe
"UpdateDeviceInfosOnDeviceThread"?

This method no longer exists in this class.

https://codereview.chromium.org/2805863002/diff/1/media/capture/video/fake_vi...
File media/capture/video/fake_video_capture_device_unittest.cc (right):

https://codereview.chromium.org/2805863002/diff/1/media/capture/video/fake_vi...
media/capture/video/fake_video_capture_device_unittest.cc:228:
std::unique_ptr<VideoCaptureDeviceDescriptors> descriptors_;
On 2017/04/07 16:34:21, chfremer wrote:
> Does this need to be a std::unique_ptr<> or can we use
> VideoCaptureDeviceDescriptors directly?

VideoCaptureDeviceDescriptors descriptors_ was unused.
std::unique_ptr<VideoCaptureDeviceDescriptors> descriptors(EnumerateDevices());
was used in each test run. Hence, changed descriptors_ to std::unique_ptr<> to
avoid '->' to '.' changes in test runs.

https://codereview.chromium.org/2805863002/diff/1/media/capture/video/fake_vi...
media/capture/video/fake_video_capture_device_unittest.cc:254:
descriptors_->clear();
On 2017/04/07 16:34:21, chfremer wrote:
> I believe that the gtest framework will create a new instance of the test
class
> for each test run, so this should not be needed.

Done.

[chfremer, 2017-04-10 16:55:08]

Patch Set 2 lgtm. Bots please.

[Chandan, 2017-04-10 17:03:34]

The CQ bit was checked by c.padhi@samsung.com

[commit-bot: I haz the power, 2017-04-10 17:04:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-10 19:00:05]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491843814126120,
"parent_rev": "6aad314383abc65e773b58a5aa697a84bf42e176", "commit_rev":
"71618f5681772f3d76c8c9c1f7f24490074db4b3"}

[commit-bot: I haz the power, 2017-04-10 19:01:03]

Description was changed from

==========
Remove VideoCaptureDeviceFactory::EnumerateDeviceDescriptors()

The method VideoCaptureDeviceFactory::EnumerateDeviceDescriptors()
is the asynchronous equivalent of method VideoCaptureDeviceFactory
::GetDeviceDescriptors(). Its current implementation delegates to
GetDeviceDescriptors() and invokes the callback synchronously.

This CL removes EnumerateDeviceDescriptors() and lets all clients
call GetDeviceDescriptors() directly.

BUG=708233
==========

to

==========
Remove VideoCaptureDeviceFactory::EnumerateDeviceDescriptors()

The method VideoCaptureDeviceFactory::EnumerateDeviceDescriptors()
is the asynchronous equivalent of method VideoCaptureDeviceFactory
::GetDeviceDescriptors(). Its current implementation delegates to
GetDeviceDescriptors() and invokes the callback synchronously.

This CL removes EnumerateDeviceDescriptors() and lets all clients
call GetDeviceDescriptors() directly.

BUG=708233

Review-Url: https://codereview.chromium.org/2805863002
Cr-Commit-Position: refs/heads/master@{#463346}
Committed:
https://chromium.googlesource.com/chromium/src/+/71618f5681772f3d76c8c9c1f7f2...
==========

[commit-bot: I haz the power, 2017-04-10 19:01:04]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/71618f5681772f3d76c8c9c1f7f2...