[PTZ] Resolve getUserMedia when ImageCapture has been initialized

This CL makes sure video track capabilities and settings are available
after getUserMedia promise resolves. This was not the case and web
developers had to introduce an arbitrary timeout before accessing
track capabilities and settings.

Bug: 711524, 934063
Change-Id: I3063cc66c52189a34c58e465385be82c8cff955b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2270142Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#783846}

chrome/test/data/webrtc/webrtc_pan_tilt_zoom_test.html

@@ -45,23 +45,13 @@
     }
   }
 
-  async function getTrackSetting(settingName) {
-    // |videoTrack|s settings retrieval is a process kicked right after
-    // creation, we introduce a small delay to allow for those to be collected.
-    // TODO(mcasas): this shouldn't be needed, https://crbug.com/711524.
-    await waitForImageCapture;
-
+  function getTrackSetting(settingName) {
     const videoTrack = stream.getVideoTracks()[0];
     const settings = videoTrack.getSettings();
     returnToTest(settings[settingName]);
   }
 
-  async function checkConstraints(expectedConstraints) {
-    // |videoTrack|s constraints retrieval is a process kicked right after
-    // creation, we introduce a small delay to allow for those to be collected.
-    // TODO(mcasas): this shouldn't be needed, https://crbug.com/711524.
-    await waitForImageCapture;
-
+  function checkConstraints(expectedConstraints) {
     const videoTrack = stream.getVideoTracks()[0];
     const constraints = videoTrack.getConstraints();
 

content/test/data/media/image_capture_test.html

@@ -114,11 +114,6 @@ function testCreateAndGetTrackCapabilities() {
       .then(stream => {
         assertEquals('video', stream.getVideoTracks()[0].kind);
         imageCapturer = new ImageCapture(stream.getVideoTracks()[0]);
-        // TODO(mcasas): Before accesing synchronous track APIs we need a delay,
-        // use instead a round trip of capabilities: https://crbug.com/711524.
-        return imageCapturer.getPhotoCapabilities();
-      })
-      .then(capabilities => {
         imageCapturer.track.getCapabilities();
         // There's nothing to check here since |capabilities| vary per device.
         reportTestSuccess();
@@ -135,11 +130,6 @@ function testCreateAndGetTrackSettings() {
       .then(stream => {
         assertEquals('video', stream.getVideoTracks()[0].kind);
         imageCapturer = new ImageCapture(stream.getVideoTracks()[0]);
-        // TODO(mcasas): Before accesing synchronous track APIs we need a delay,
-        // use instead a round trip of capabilities: https://crbug.com/711524.
-        return imageCapturer.getPhotoCapabilities();
-      })
-      .then(capabilities => {
         imageCapturer.track.getSettings();
         // There's nothing to check here since |settings| vary per device.
         reportTestSuccess();
@@ -158,11 +148,6 @@ function testManipulatePan() {
       .then(stream => {
         assertEquals('video', stream.getVideoTracks()[0].kind);
         imageCapturer = new ImageCapture(stream.getVideoTracks()[0]);
-        // TODO(mcasas): Before accesing synchronous track APIs we need a delay,
-        // use instead a round trip of capabilities: https://crbug.com/711524.
-        return imageCapturer.getPhotoCapabilities();
-      })
-      .then(capabilities => {
         const trackCapabilities = imageCapturer.track.getCapabilities();
         if (trackCapabilities.pan === undefined) {
           console.log('pan not supported, skipping test');
@@ -199,11 +184,6 @@ function testManipulateTilt() {
       .then(stream => {
         assertEquals('video', stream.getVideoTracks()[0].kind);
         imageCapturer = new ImageCapture(stream.getVideoTracks()[0]);
-        // TODO(mcasas): Before accesing synchronous track APIs we need a delay,
-        // use instead a round trip of capabilities: https://crbug.com/711524.
-        return imageCapturer.getPhotoCapabilities();
-      })
-      .then(capabilities => {
         const trackCapabilities = imageCapturer.track.getCapabilities();
         if (trackCapabilities.tilt === undefined) {
           console.log('tilt not supported, skipping test');
@@ -240,11 +220,6 @@ function testManipulateZoom() {
       .then(stream => {
         assertEquals('video', stream.getVideoTracks()[0].kind);
         imageCapturer = new ImageCapture(stream.getVideoTracks()[0]);
-        // TODO(mcasas): Before accesing synchronous track APIs we need a delay,
-        // use instead a round trip of capabilities: https://crbug.com/711524.
-        return imageCapturer.getPhotoCapabilities();
-      })
-      .then(capabilities => {
         const trackCapabilities = imageCapturer.track.getCapabilities();
         if (trackCapabilities.zoom === undefined) {
           console.log('zoom not supported, skipping test');
@@ -281,11 +256,6 @@ function testManipulateExposureTime() {
         .then(stream => {
             assertEquals('video', stream.getVideoTracks()[0].kind);
             imageCapturer = new ImageCapture(stream.getVideoTracks()[0]);
-            // TODO(mcasas): Before accesing synchronous track APIs we need a delay,
-            // use instead a round trip of capabilities: https://crbug.com/711524.
-            return imageCapturer.getPhotoCapabilities();
-        })
-        .then(capabilities => {
             const trackCapabilities = imageCapturer.track.getCapabilities();
             if (trackCapabilities.exposureTime === undefined) {
                 console.log('exposureTime not supported, skipping test');
@@ -340,11 +310,6 @@ function testManipulateFocusDistance() {
         .then(stream => {
             assertEquals('video', stream.getVideoTracks()[0].kind);
             imageCapturer = new ImageCapture(stream.getVideoTracks()[0]);
-            // TODO(mcasas): Before accesing synchronous track APIs we need a delay,
-            // use instead a round trip of capabilities: https://crbug.com/711524.
-            return imageCapturer.getPhotoCapabilities();
-        })
-        .then(capabilities => {
             const trackCapabilities = imageCapturer.track.getCapabilities();
             if (trackCapabilities.focusDistance === undefined) {
                 console.log('focusDistance not supported, skipping test');

third_party/blink/renderer/modules/imagecapture/image_capture.cc

@@ -105,8 +105,8 @@ ImageCapture* ImageCapture::Create(ExecutionContext* context,
       (track->GetImageCapture() &&
        track->GetImageCapture()->HasPanTiltZoomPermissionGranted());
 
-  return MakeGarbageCollected<ImageCapture>(context, track,
-                                            pan_tilt_zoom_allowed);
+  return MakeGarbageCollected<ImageCapture>(
+      context, track, pan_tilt_zoom_allowed, base::DoNothing());
 }
 
 ImageCapture::~ImageCapture() {
@@ -697,8 +697,9 @@ void ImageCapture::SetMediaTrackConstraints(
 }
 
 void ImageCapture::SetPanTiltZoomSettingsFromTrack(
+    base::OnceClosure initialized_callback,
     media::mojom::blink::PhotoStatePtr photo_state) {
-  UpdateMediaTrackCapabilities(std::move(photo_state));
+  UpdateMediaTrackCapabilities(base::DoNothing(), std::move(photo_state));
 
   MediaStreamVideoTrack* video_track = MediaStreamVideoTrack::GetVideoTrack(
       WebMediaStreamTrack(stream_track_->Component()));
@@ -708,8 +709,16 @@ void ImageCapture::SetPanTiltZoomSettingsFromTrack(
   base::Optional<double> tilt = video_track->tilt();
   base::Optional<double> zoom = video_track->zoom();
 
-  if (!pan.has_value() && !tilt.has_value() && !zoom.has_value())
+  const bool ptz_requested =
+      pan.has_value() || tilt.has_value() || zoom.has_value();
+  const bool ptz_supported = capabilities_->hasPan() ||
+                             capabilities_->hasTilt() ||
+                             capabilities_->hasZoom();
+  if (!ptz_supported || !ptz_requested || !HasPanTiltZoomPermissionGranted() ||
+      !service_.is_bound()) {
+    std::move(initialized_callback).Run();
     return;
+  }
 
   ExecutionContext* context = GetExecutionContext();
   if (pan.has_value())
@@ -719,17 +728,6 @@ void ImageCapture::SetPanTiltZoomSettingsFromTrack(
   if (zoom.has_value())
     UseCounter::Count(context, WebFeature::kImageCaptureZoom);
 
-  if (!HasPanTiltZoomPermissionGranted())
-    return;
-
-  if (!capabilities_->hasPan() && !capabilities_->hasTilt() &&
-      !capabilities_->hasZoom()) {
-    return;
-  }
-
-  if (!service_.is_bound())
-    return;
-
   auto settings = media::mojom::blink::PhotoSettings::New();
 
   if (capabilities_->hasPan() && pan.has_value() &&
@@ -754,13 +752,16 @@ void ImageCapture::SetPanTiltZoomSettingsFromTrack(
   service_->SetOptions(
       stream_track_->Component()->Source()->Id(), std::move(settings),
       WTF::Bind(&ImageCapture::OnSetPanTiltZoomSettingsFromTrack,
-                WrapPersistent(this)));
+                WrapPersistent(this), std::move(initialized_callback)));
 }
 
-void ImageCapture::OnSetPanTiltZoomSettingsFromTrack(bool result) {
-  service_->GetPhotoState(stream_track_->Component()->Source()->Id(),
-                          WTF::Bind(&ImageCapture::UpdateMediaTrackCapabilities,
-                                    WrapPersistent(this)));
+void ImageCapture::OnSetPanTiltZoomSettingsFromTrack(
+    base::OnceClosure done_callback,
+    bool result) {
+  service_->GetPhotoState(
+      stream_track_->Component()->Source()->Id(),
+      WTF::Bind(&ImageCapture::UpdateMediaTrackCapabilities,
+                WrapPersistent(this), std::move(done_callback)));
 }
 
 const MediaTrackConstraintSet* ImageCapture::GetMediaTrackConstraints() const {
@@ -828,7 +829,8 @@ void ImageCapture::GetMediaTrackSettings(MediaTrackSettings* settings) const {
 
 ImageCapture::ImageCapture(ExecutionContext* context,
                            MediaStreamTrack* track,
-                           bool pan_tilt_zoom_allowed)
+                           bool pan_tilt_zoom_allowed,
+                           base::OnceClosure initialized_callback)
     : ExecutionContextLifecycleObserver(context),
       stream_track_(track),
       service_(context),
@@ -861,7 +863,7 @@ ImageCapture::ImageCapture(ExecutionContext* context,
   service_->GetPhotoState(
       stream_track_->Component()->Source()->Id(),
       WTF::Bind(&ImageCapture::SetPanTiltZoomSettingsFromTrack,
-                WrapPersistent(this)));
+                WrapPersistent(this), std::move(initialized_callback)));
 
   ConnectToPermissionService(
       context, permission_service_.BindNewPipeAndPassReceiver(
@@ -924,7 +926,7 @@ void ImageCapture::OnMojoGetPhotoState(
     photo_capabilities_->SetFillLightMode(photo_state->fill_light_mode);
 
   // Update the local track photo_state cache.
-  UpdateMediaTrackCapabilities(std::move(photo_state));
+  UpdateMediaTrackCapabilities(base::DoNothing(), std::move(photo_state));
 
   if (trigger_take_photo) {
     service_->TakePhoto(
@@ -983,9 +985,12 @@ void ImageCapture::OnMojoTakePhoto(ScriptPromiseResolver* resolver,
 }
 
 void ImageCapture::UpdateMediaTrackCapabilities(
+    base::OnceClosure initialized_callback,
     media::mojom::blink::PhotoStatePtr photo_state) {
-  if (!photo_state)
+  if (!photo_state) {
+    std::move(initialized_callback).Run();
     return;
+  }
 
   WTF::Vector<WTF::String> supported_white_balance_modes;
   supported_white_balance_modes.ReserveInitialCapacity(
@@ -1104,6 +1109,8 @@ void ImageCapture::UpdateMediaTrackCapabilities(
     capabilities_->setTorch(photo_state->supports_torch);
   if (photo_state->supports_torch)
     settings_->setTorch(photo_state->torch);
+
+  std::move(initialized_callback).Run();
 }
 
 void ImageCapture::OnServiceConnectionError() {

third_party/blink/renderer/modules/imagecapture/image_capture.h

@@ -45,9 +45,12 @@ class MODULES_EXPORT ImageCapture final
                               MediaStreamTrack*,
                               ExceptionState&);
 
+  // |initialized_callback| is called when settings and capabilities are
+  // retrieved.
   ImageCapture(ExecutionContext*,
                MediaStreamTrack*,
-               bool pan_tilt_zoom_allowed);
+               bool pan_tilt_zoom_allowed,
+               base::OnceClosure initialized_callback);
   ~ImageCapture() override;
 
   // EventTarget implementation.
@@ -105,10 +108,20 @@ class MODULES_EXPORT ImageCapture final
   // If getUserMedia contains either pan, tilt, or zoom constraints, the
   // corresponding settings will be set when image capture is created.
   void SetPanTiltZoomSettingsFromTrack(
+      base::OnceClosure callback,
+      media::mojom::blink::PhotoStatePtr photo_state);
+  // Update local track settings and capabilities once pan, tilt, and zoom
+  // settings have been set. |done_callback| will be called when settings and
+  // capabilities are retrieved.
+  void OnSetPanTiltZoomSettingsFromTrack(base::OnceClosure done_callback,
+                                         bool result);
+  // Update local track settings and capabilities and call
+  // |initialized_callback| to indicate settings and capabilities have been
+  // retrieved.
+  void UpdateMediaTrackCapabilities(
+      base::OnceClosure initialized_callback,
       media::mojom::blink::PhotoStatePtr photo_state);
-  void OnSetPanTiltZoomSettingsFromTrack(bool result);
 
-  void UpdateMediaTrackCapabilities(media::mojom::blink::PhotoStatePtr);
   void OnServiceConnectionError();
 
   void ResolveWithNothing(ScriptPromiseResolver*);

third_party/blink/renderer/modules/mediastream/media_stream.cc

@@ -92,7 +92,15 @@ MediaStream* MediaStream::Create(ExecutionContext* context,
 
 MediaStream* MediaStream::Create(ExecutionContext* context,
                                  MediaStreamDescriptor* stream_descriptor) {
-  return MakeGarbageCollected<MediaStream>(context, stream_descriptor);
+  return MakeGarbageCollected<MediaStream>(context, stream_descriptor,
+                                           /*callback=*/base::DoNothing());
+}
+
+void MediaStream::Create(ExecutionContext* context,
+                         MediaStreamDescriptor* stream_descriptor,
+                         base::OnceCallback<void(MediaStream*)> callback) {
+  MakeGarbageCollected<MediaStream>(context, stream_descriptor,
+                                    std::move(callback));
 }
 
 MediaStream* MediaStream::Create(ExecutionContext* context,
@@ -104,9 +112,11 @@ MediaStream* MediaStream::Create(ExecutionContext* context,
 }
 
 MediaStream::MediaStream(ExecutionContext* context,
-                         MediaStreamDescriptor* stream_descriptor)
+                         MediaStreamDescriptor* stream_descriptor,
+                         base::OnceCallback<void(MediaStream*)> callback)
     : ExecutionContextClient(context),
       descriptor_(stream_descriptor),
+      media_stream_initialized_callback_(std::move(callback)),
       scheduled_event_timer_(
           context->GetTaskRunner(TaskType::kMediaElementEvent),
           this,
@@ -126,7 +136,9 @@ MediaStream::MediaStream(ExecutionContext* context,
   video_tracks_.ReserveCapacity(number_of_video_tracks);
   for (uint32_t i = 0; i < number_of_video_tracks; i++) {
     auto* new_track = MakeGarbageCollected<MediaStreamTrack>(
-        context, descriptor_->VideoComponent(i));
+        context, descriptor_->VideoComponent(i),
+        WTF::Bind(&MediaStream::OnMediaStreamTrackInitialized,
+                  WrapPersistent(this)));
     new_track->RegisterMediaStream(this);
     video_tracks_.push_back(new_track);
   }
@@ -134,6 +146,17 @@ MediaStream::MediaStream(ExecutionContext* context,
   if (EmptyOrOnlyEndedTracks()) {
     descriptor_->SetActive(false);
   }
+
+  if (number_of_video_tracks == 0) {
+    std::move(media_stream_initialized_callback_).Run(this);
+  }
+}
+
+void MediaStream::OnMediaStreamTrackInitialized() {
+  if (++number_of_video_tracks_initialized_ ==
+      descriptor_->NumberOfVideoComponents()) {
+    std::move(media_stream_initialized_callback_).Run(this);
+  }
 }
 
 MediaStream::MediaStream(ExecutionContext* context,

third_party/blink/renderer/modules/mediastream/media_stream.h

@@ -67,6 +67,12 @@ class MODULES_EXPORT MediaStream final
   // Creates a MediaStream matching the MediaStreamDescriptor. MediaStreamTracks
   // are created for any MediaStreamComponents attached to the descriptor.
   static MediaStream* Create(ExecutionContext*, MediaStreamDescriptor*);
+  // Creates a MediaStream matching the MediaStreamDescriptor. MediaStreamTracks
+  // are created for any MediaStreamComponents attached to the descriptor. It
+  // returns the stream via callback.
+  static void Create(ExecutionContext*,
+                     MediaStreamDescriptor*,
+                     base::OnceCallback<void(MediaStream*)> callback);
   // Creates a MediaStream with the specified MediaStreamDescriptor and
   // MediaStreamTracks. The tracks must match the MediaStreamComponents attached
   // to the descriptor (or else a DCHECK fails). This allows you to create
@@ -81,7 +87,9 @@ class MODULES_EXPORT MediaStream final
                              const MediaStreamTrackVector& audio_tracks,
                              const MediaStreamTrackVector& video_tracks);
 
-  MediaStream(ExecutionContext*, MediaStreamDescriptor*);
+  MediaStream(ExecutionContext*,
+              MediaStreamDescriptor*,
+              base::OnceCallback<void(MediaStream*)> callback);
   MediaStream(ExecutionContext*,
               MediaStreamDescriptor*,
               const MediaStreamTrackVector& audio_tracks,
@@ -156,14 +164,22 @@ class MODULES_EXPORT MediaStream final
   void ScheduleDispatchEvent(Event*);
   void ScheduledEventTimerFired(TimerBase*);
 
+  void OnMediaStreamTrackInitialized();
+
   MediaStreamTrackVector audio_tracks_;
   MediaStreamTrackVector video_tracks_;
   Member<MediaStreamDescriptor> descriptor_;
   // Observers are informed when |addTrack| and |removeTrack| are called.
   HeapHashSet<WeakMember<MediaStreamObserver>> observers_;
 
+  // The callback to be called when the media stream is fully initialized,
+  // including image capture for video tracks.
+  base::OnceCallback<void(MediaStream*)> media_stream_initialized_callback_;
+
   TaskRunnerTimer<MediaStream> scheduled_event_timer_;
   HeapVector<Member<Event>> scheduled_events_;
+
+  uint32_t number_of_video_tracks_initialized_ = 0;
 };
 
 using MediaStreamVector = HeapVector<Member<MediaStream>>;

third_party/blink/renderer/modules/mediastream/media_stream_track.cc

@@ -222,11 +222,21 @@ MediaStreamTrack::MediaStreamTrack(ExecutionContext* context,
                                    MediaStreamComponent* component)
     : MediaStreamTrack(context,
                        component,
-                       component->Source()->GetReadyState()) {}
+                       component->Source()->GetReadyState(),
+                       /*callback=*/base::DoNothing()) {}
 
 MediaStreamTrack::MediaStreamTrack(ExecutionContext* context,
                                    MediaStreamComponent* component,
-                                   MediaStreamSource::ReadyState ready_state)
+                                   base::OnceClosure callback)
+    : MediaStreamTrack(context,
+                       component,
+                       component->Source()->GetReadyState(),
+                       std::move(callback)) {}
+
+MediaStreamTrack::MediaStreamTrack(ExecutionContext* context,
+                                   MediaStreamComponent* component,
+                                   MediaStreamSource::ReadyState ready_state,
+                                   base::OnceClosure callback)
     : ready_state_(ready_state),
       component_(component),
       execution_context_(context) {
@@ -244,8 +254,10 @@ MediaStreamTrack::MediaStreamTrack(ExecutionContext* context,
     bool pan_tilt_zoom_allowed =
         image_capture_ ? image_capture_->HasPanTiltZoomPermissionGranted()
                        : video_track->pan_tilt_zoom_allowed();
-    image_capture_ = MakeGarbageCollected<ImageCapture>(context, this,
-                                                        pan_tilt_zoom_allowed);
+    image_capture_ = MakeGarbageCollected<ImageCapture>(
+        context, this, pan_tilt_zoom_allowed, std::move(callback));
+  } else {
+    std::move(callback).Run();
   }
 }
 
@@ -401,7 +413,8 @@ void MediaStreamTrack::stopTrack(ExecutionContext* execution_context) {
 MediaStreamTrack* MediaStreamTrack::clone(ScriptState* script_state) {
   MediaStreamComponent* cloned_component = Component()->Clone();
   MediaStreamTrack* cloned_track = MakeGarbageCollected<MediaStreamTrack>(
-      ExecutionContext::From(script_state), cloned_component, ready_state_);
+      ExecutionContext::From(script_state), cloned_component, ready_state_,
+      base::DoNothing());
   DidCloneMediaStreamTrack(Component(), cloned_component);
   return cloned_track;
 }

third_party/blink/renderer/modules/mediastream/media_stream_track.h

@@ -57,7 +57,11 @@ class MODULES_EXPORT MediaStreamTrack
   MediaStreamTrack(ExecutionContext*, MediaStreamComponent*);
   MediaStreamTrack(ExecutionContext*,
                    MediaStreamComponent*,
-                   MediaStreamSource::ReadyState);
+                   base::OnceClosure callback);
+  MediaStreamTrack(ExecutionContext*,
+                   MediaStreamComponent*,
+                   MediaStreamSource::ReadyState,
+                   base::OnceClosure callback);
   ~MediaStreamTrack() override;
 
   String kind() const;

third_party/blink/renderer/modules/mediastream/user_media_request.cc

@@ -517,20 +517,21 @@ void UserMediaRequest::Succeed(MediaStreamDescriptor* stream_descriptor) {
   if (!GetExecutionContext())
     return;
 
-  MediaStream* stream =
-      MediaStream::Create(GetExecutionContext(), stream_descriptor);
+  MediaStream::Create(GetExecutionContext(), stream_descriptor,
+                      WTF::Bind(&UserMediaRequest::OnMediaStreamInitialized,
+                                WrapPersistent(this)));
+}
+
+void UserMediaRequest::OnMediaStreamInitialized(MediaStream* stream) {
+  DCHECK(!is_resolved_);
 
   MediaStreamTrackVector audio_tracks = stream->getAudioTracks();
-  for (MediaStreamTrackVector::iterator iter = audio_tracks.begin();
-       iter != audio_tracks.end(); ++iter) {
-    (*iter)->SetConstraints(audio_);
-  }
+  for (const auto& audio_track : audio_tracks)
+    audio_track->SetConstraints(audio_);
 
   MediaStreamTrackVector video_tracks = stream->getVideoTracks();
-  for (MediaStreamTrackVector::iterator iter = video_tracks.begin();
-       iter != video_tracks.end(); ++iter) {
-    (*iter)->SetConstraints(video_);
-  }
+  for (const auto& video_track : video_tracks)
+    video_track->SetConstraints(video_);
 
   callbacks_->OnSuccess(nullptr, stream);
   is_resolved_ = true;

third_party/blink/renderer/modules/mediastream/user_media_request.h

@@ -119,6 +119,7 @@ class MODULES_EXPORT UserMediaRequest final
   void Start();
 
   void Succeed(MediaStreamDescriptor*);
+  void OnMediaStreamInitialized(MediaStream* stream);
   void FailConstraint(const String& constraint_name, const String& message);
   void Fail(Error name, const String& message);