getUserMedia: handle the device starting status report.

content/renderer/media/video_capture_impl.cc

Index: content/renderer/media/video_capture_impl.cc
diff --git a/content/renderer/media/video_capture_impl.cc b/content/renderer/media/video_capture_impl.cc
index 089fcbc8f8209a7c4dde23c17406fe30ad92787e..4f5a053a9a883087f4e5d73c78425dd01020b138 100644
--- a/content/renderer/media/video_capture_impl.cc
+++ b/content/renderer/media/video_capture_impl.cc
@@ -70,6 +70,7 @@ VideoCaptureImpl::VideoCaptureImpl(media::VideoCaptureSessionId session_id)
       video_capture_host_for_testing_(nullptr),
       observer_binding_(this),
       state_(VIDEO_CAPTURE_STATE_STOPPED),
+      should_request_refresh_frame_on_started_(false),
       weak_factory_(this) {
   io_thread_checker_.DetachFromThread();
 
@@ -113,9 +114,6 @@ void VideoCaptureImpl::StartCapture(
              clients_.count(client_id)) {
     DLOG(FATAL) << __func__ << " This client has already started.";
   } else {
-    // Note: |state_| might not be started at this point. But we tell
-    // client that we have started.
-    state_update_cb.Run(VIDEO_CAPTURE_STATE_STARTED);

[miu, 2017/03/06 22:37:33]

Per my earlier comment, how about keeping this, but calling it with the new
VIDEO_CAPTURE_STATE_STARTING state?

[braveyao, 2017/03/08 22:02:52]

This does no good and will cause troubles to the unittests. So I still don't
want to keep it.

     if (state_ == VIDEO_CAPTURE_STATE_STARTED) {
       clients_[client_id] = client_info;
       // TODO(sheu): Allowing resolution change will require that all
@@ -188,8 +186,13 @@ void VideoCaptureImpl::OnStateChanged(mojom::VideoCaptureState state) {
 
   switch (state) {
     case mojom::VideoCaptureState::STARTED:
-      // Capture has started in the browser process. Since we have already
-      // told all clients that we have started there's nothing to do.
+      state_ = VIDEO_CAPTURE_STATE_STARTED;
+      for (const auto& client : clients_)
+        client.second.state_update_cb.Run(VIDEO_CAPTURE_STATE_STARTED);
+      if (should_request_refresh_frame_on_started_) {

[miu, 2017/03/06 22:37:33]

Suggestion: Instead of this logic, what if VideoCaptureImpl just unconditionally
requests a refresh frame once reaching the STARTED state? IMHO, it makes sense
to provide the client with a first frame as soon as possible. Also, it wouldn't
be horribly redundant because capture devices can ignore the request if they
know they are going to provide a new frame soon anyway.

[braveyao, 2017/03/08 22:02:52]

Done.
Good to know!
But it will cause many GMock Warnings. And most cases don't care the calling to
RequestRefreshFrames. So I didn't add EXPECT_CALL there. Is it really good?

[miu, 2017/03/10 23:56:56]

Yes, this should be fine. It's up to you, but it would probably be reasonable to
add the EXPECT_CALL() to reduce log spam; and because it's an acceptable
behavior change.

[braveyao, 2017/03/13 19:04:44]

Acknowledged.

+        RequestRefreshFrame();
+        should_request_refresh_frame_on_started_ = false;
+      }
       break;
     case mojom::VideoCaptureState::STOPPED:
       state_ = VIDEO_CAPTURE_STATE_STOPPED;
@@ -228,9 +231,6 @@ void VideoCaptureImpl::OnBufferCreated(int32_t buffer_id,
   DCHECK(io_thread_checker_.CalledOnValidThread());
   DCHECK(handle.is_valid());
 
-  if (state_ != VIDEO_CAPTURE_STATE_STARTED)
-    return;
-
   base::SharedMemoryHandle memory_handle;
   size_t memory_size = 0;
   bool read_only_flag = false;
@@ -260,6 +260,8 @@ void VideoCaptureImpl::OnBufferReady(int32_t buffer_id,
   DCHECK(io_thread_checker_.CalledOnValidThread());
 
   bool consume_buffer = state_ == VIDEO_CAPTURE_STATE_STARTED;
+  if (!consume_buffer && first_frame_ref_time_.is_null())
+    should_request_refresh_frame_on_started_ = true;
   if ((info->pixel_format != media::PIXEL_FORMAT_I420 &&
        info->pixel_format != media::PIXEL_FORMAT_Y16) ||
       info->storage_type != media::PIXEL_STORAGE_CPU) {
@@ -382,7 +384,6 @@ void VideoCaptureImpl::StartCaptureInternal() {
 
   GetVideoCaptureHost()->Start(device_id_, session_id_, params_,
                                observer_binding_.CreateInterfacePtrAndBind());
-  state_ = VIDEO_CAPTURE_STATE_STARTED;

[miu, 2017/03/06 22:37:33]

Retain this, but it should be changed to:

  state_ = VIDEO_CAPTURE_STATE_STARTING;

[braveyao, 2017/03/08 22:02:52]

Done.

 }
 
 void VideoCaptureImpl::OnDeviceSupportedFormats(