Image Capture: add getPhotoSettings() method
This CL adds a method getPhotoSettings() to catch up with
the idl [1], and also adds tests for it.
ToT code calls the two mojo methods (getPhotoState() and SetOptions())
and distinguishes how to Resolve() the Promises via the Blink's
|service_requests_| map. Instead of extending this mechanism, this CL
binds to the OnMojo...() methods one of 4 new callbacks (WTF::Function):
- ResolveWithNothing()
- ResolveWithPhotoSettings()
- ResolveWithPhotoCapabilities()
- ResolveWithMediaTrackConstraints()
So that the different sequences to mojo and back are now:
getPhotoCapabilities --> OnMojoGetPhotoState --> ResolveWithPhotoCapabilities
getPhotoSettings --> OnMojoGetPhotoState --> ResolveWithPhotoSettings
setOptions --> OnMojoSetOptions --> OnMojoGetPhotoState --> ResolveWithNothing
SetMediaTrackConstraints --> OnMojoSetOptions --> OnMojoGetPhotoState --> ResolveWithMediaTrackConstraints
(note that the "set" versions have a cycle of setting and
a cycle of current state readback).
[1] https://github.com/w3c/mediacapture-image/issues/167
Bug: 732521
Change-Id: Id5a3ee3b58cedd289a18fa1c721be390776c6543
Reviewed-on: https://chromium-review.googlesource.com/534754
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#480447}diff --git a/content/browser/webrtc/webrtc_image_capture_browsertest.cc b/content/browser/webrtc/webrtc_image_capture_browsertest.cc
index 687c325..0cd11af 100644
--- a/content/browser/webrtc/webrtc_image_capture_browsertest.cc
+++ b/content/browser/webrtc/webrtc_image_capture_browsertest.cc
@@ -23,6 +23,7 @@
#if defined(OS_WIN)
// These tests are flaky on WebRTC Windows bots: https://crbug.com/633242.
#define MAYBE_GetPhotoCapabilities DISABLED_GetPhotoCapabilities
+#define MAYBE_GetPhotoSettings DISABLED_GetPhotoSettings
#define MAYBE_TakePhoto DISABLED_TakePhoto
#define MAYBE_GrabFrame DISABLED_GrabFrame
#define MAYBE_GetTrackCapabilities DISABLED_GetTrackCapabilities
@@ -30,6 +31,7 @@
#define MAYBE_ManipulateZoom DISABLED_ManipulateZoom
#else
#define MAYBE_GetPhotoCapabilities GetPhotoCapabilities
+#define MAYBE_GetPhotoSettings GetPhotoSettings
#define MAYBE_TakePhoto TakePhoto
#define MAYBE_GrabFrame GrabFrame
#define MAYBE_GetTrackCapabilities GetTrackCapabilities
@@ -122,6 +124,10 @@
ASSERT_TRUE(RunImageCaptureTestCase("testCreateAndGetPhotoCapabilities()"));
}
+IN_PROC_BROWSER_TEST_P(WebRtcImageCaptureBrowserTest, MAYBE_GetPhotoSettings) {
+ embedded_test_server()->StartAcceptingConnections();
+ ASSERT_TRUE(RunImageCaptureTestCase("testCreateAndGetPhotoSettings()"));
+}
IN_PROC_BROWSER_TEST_P(WebRtcImageCaptureBrowserTest, MAYBE_TakePhoto) {
embedded_test_server()->StartAcceptingConnections();
ASSERT_TRUE(RunImageCaptureTestCase("testCreateAndTakePhoto()"));
diff --git a/content/test/data/media/image_capture_test.html b/content/test/data/media/image_capture_test.html
index da8da8c8..ff7bce2 100644
--- a/content/test/data/media/image_capture_test.html
+++ b/content/test/data/media/image_capture_test.html
@@ -35,6 +35,25 @@
});
}
+// Runs an ImageCapture.getPhotoSettings().
+function testCreateAndGetPhotoSettings() {
+ navigator.mediaDevices.getUserMedia({"video" : CONSTRAINTS})
+ .then(stream => {
+ assertEquals('video', stream.getVideoTracks()[0].kind);
+ return new ImageCapture(stream.getVideoTracks()[0]);
+ })
+ .then(capturer => {
+ return capturer.getPhotoSettings();
+ })
+ .then(settings => {
+ // There's nothing to check here since |settings| vary per device.
+ reportTestSuccess();
+ })
+ .catch(err => {
+ return failTest(err.toString());
+ });
+}
+
// Runs an ImageCapture.takePhoto().
function testCreateAndTakePhoto() {
navigator.mediaDevices.getUserMedia({"video" : CONSTRAINTS})
diff --git a/third_party/WebKit/LayoutTests/imagecapture/getPhotoSettings.html b/third_party/WebKit/LayoutTests/imagecapture/getPhotoSettings.html
new file mode 100644
index 0000000..a6935363
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/imagecapture/getPhotoSettings.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<script src="../resources/mojo-helpers.js"></script>
+<script src="resources/mock-imagecapture.js"></script>
+<body>
+<canvas id='canvas' width=10 height=10/>
+</body>
+<script>
+
+const fillLightModeNames = ["off", "auto", "flash"];
+
+// This test verifies that ImageCapture can call getPhotoSettings(), with a
+// mock Mojo interface implementation.
+async_test(function(t) {
+ var canvas = document.getElementById('canvas');
+ var context = canvas.getContext("2d");
+ context.fillStyle = "red";
+ context.fillRect(0, 0, 10, 10);
+ var stream = canvas.captureStream();
+
+ var mock_state;
+
+ mockImageCaptureReady
+ .then(mock => {
+ mock_state = mock.state();
+ return new ImageCapture(stream.getVideoTracks()[0]);
+ },
+ error => {
+ assert_unreached("Error creating MockImageCapture: " + error);
+ })
+ .then(capturer => {
+ return capturer.getPhotoSettings();
+ })
+ .then(settings => {
+ assert_equals(settings.imageWidth, mock_state.width.current, 'width');
+ assert_equals(settings.imageHeight, mock_state.height.current, 'height');
+ // TODO(mcasas): check the remaining two entries https://crbug.com/732521.
+
+ t.done();
+ })
+ .catch(error => {
+ assert_unreached("Error during getPhotoSettings(): " + error.message);
+ });
+}, 'exercises ImageCapture.getPhotoSettings()');
+
+</script>
diff --git a/third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt b/third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt
index b743349..30a219a 100644
--- a/third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt
+++ b/third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt
@@ -3157,6 +3157,7 @@
getter track
method constructor
method getPhotoCapabilities
+ method getPhotoSettings
method grabFrame
method setOptions
method takePhoto
diff --git a/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt b/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
index ff81a8a..720f354 100644
--- a/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
+++ b/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
@@ -3086,6 +3086,7 @@
getter track
method constructor
method getPhotoCapabilities
+ method getPhotoSettings
method grabFrame
method setOptions
method takePhoto
diff --git a/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt b/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt
index 9b7e867..44f56e6 100644
--- a/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt
+++ b/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt
@@ -3819,6 +3819,7 @@
getter track
method constructor
method getPhotoCapabilities
+ method getPhotoSettings
method grabFrame
method setOptions
method takePhoto
diff --git a/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
index f1136b1..ae8e3a87 100644
--- a/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
+++ b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
@@ -3819,6 +3819,7 @@
getter track
method constructor
method getPhotoCapabilities
+ method getPhotoSettings
method grabFrame
method setOptions
method takePhoto
diff --git a/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
index 1fabf58..fa0cd82 100644
--- a/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
+++ b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
@@ -13,7 +13,6 @@
#include "modules/EventTargetModules.h"
#include "modules/imagecapture/MediaSettingsRange.h"
#include "modules/imagecapture/PhotoCapabilities.h"
-#include "modules/imagecapture/PhotoSettings.h"
#include "modules/mediastream/MediaStreamTrack.h"
#include "modules/mediastream/MediaTrackCapabilities.h"
#include "modules/mediastream/MediaTrackConstraints.h"
@@ -128,7 +127,10 @@
resolver->Reject(DOMException::Create(kNotFoundError, kNoServiceError));
return promise;
}
- service_requests_.insert(resolver, HeapVector<MediaTrackConstraintSet>());
+ service_requests_.insert(resolver);
+
+ auto resolver_cb = WTF::Bind(&ImageCapture::ResolveWithPhotoCapabilities,
+ WrapPersistent(this));
// m_streamTrack->component()->source()->id() is the renderer "name" of the
// camera;
@@ -136,9 +138,36 @@
// scriptState->getExecutionContext()->getSecurityOrigin()->toString()
service_->GetPhotoState(
stream_track_->Component()->Source()->Id(),
- ConvertToBaseCallback(
- WTF::Bind(&ImageCapture::OnMojoGetPhotoState, WrapPersistent(this),
- WrapPersistent(resolver), false /* trigger_take_photo */)));
+ ConvertToBaseCallback(WTF::Bind(
+ &ImageCapture::OnMojoGetPhotoState, WrapPersistent(this),
+ WrapPersistent(resolver), WTF::Passed(std::move(resolver_cb)),
+ false /* trigger_take_photo */)));
+ return promise;
+}
+
+ScriptPromise ImageCapture::getPhotoSettings(ScriptState* script_state) {
+ ScriptPromiseResolver* resolver = ScriptPromiseResolver::Create(script_state);
+ ScriptPromise promise = resolver->Promise();
+
+ if (!service_) {
+ resolver->Reject(DOMException::Create(kNotFoundError, kNoServiceError));
+ return promise;
+ }
+ service_requests_.insert(resolver);
+
+ auto resolver_cb =
+ WTF::Bind(&ImageCapture::ResolveWithPhotoSettings, WrapPersistent(this));
+
+ // m_streamTrack->component()->source()->id() is the renderer "name" of the
+ // camera;
+ // TODO(mcasas) consider sending the security origin as well:
+ // scriptState->getExecutionContext()->getSecurityOrigin()->toString()
+ service_->GetPhotoState(
+ stream_track_->Component()->Source()->Id(),
+ ConvertToBaseCallback(WTF::Bind(
+ &ImageCapture::OnMojoGetPhotoState, WrapPersistent(this),
+ WrapPersistent(resolver), WTF::Passed(std::move(resolver_cb)),
+ false /* trigger_take_photo */)));
return promise;
}
@@ -158,7 +187,7 @@
resolver->Reject(DOMException::Create(kNotFoundError, kNoServiceError));
return promise;
}
- service_requests_.insert(resolver, HeapVector<MediaTrackConstraintSet>());
+ service_requests_.insert(resolver);
// TODO(mcasas): should be using a mojo::StructTraits instead.
auto settings = media::mojom::blink::PhotoSettings::New();
@@ -211,11 +240,15 @@
settings->fill_light_mode = ParseFillLightMode(fill_light_mode);
}
+ auto resolver_cb =
+ WTF::Bind(&ImageCapture::ResolveWithNothing, WrapPersistent(this));
+
service_->SetOptions(
stream_track_->Component()->Source()->Id(), std::move(settings),
ConvertToBaseCallback(
WTF::Bind(&ImageCapture::OnMojoSetOptions, WrapPersistent(this),
- WrapPersistent(resolver), trigger_take_photo)));
+ WrapPersistent(resolver),
+ WTF::Passed(std::move(resolver_cb)), trigger_take_photo)));
return promise;
}
@@ -233,7 +266,7 @@
return promise;
}
- service_requests_.insert(resolver, HeapVector<MediaTrackConstraintSet>());
+ service_requests_.insert(resolver);
// m_streamTrack->component()->source()->id() is the renderer "name" of the
// camera;
@@ -499,13 +532,19 @@
current_constraints_ = temp_constraints;
- service_requests_.insert(resolver, constraints_vector);
+ service_requests_.insert(resolver);
+
+ MediaTrackConstraints resolver_constraints;
+ resolver_constraints.setAdvanced(constraints_vector);
+ auto resolver_cb = WTF::Bind(&ImageCapture::ResolveWithMediaTrackConstraints,
+ WrapPersistent(this), resolver_constraints);
service_->SetOptions(
stream_track_->Component()->Source()->Id(), std::move(settings),
- ConvertToBaseCallback(
- WTF::Bind(&ImageCapture::OnMojoSetOptions, WrapPersistent(this),
- WrapPersistent(resolver), false /* trigger_take_photo */)));
+ ConvertToBaseCallback(WTF::Bind(
+ &ImageCapture::OnMojoSetOptions, WrapPersistent(this),
+ WrapPersistent(resolver), WTF::Passed(std::move(resolver_cb)),
+ false /* trigger_take_photo */)));
}
const MediaTrackConstraintSet& ImageCapture::GetMediaTrackConstraints() const {
@@ -599,10 +638,10 @@
void ImageCapture::OnMojoGetPhotoState(
ScriptPromiseResolver* resolver,
+ PromiseResolverFunction resolve_function,
bool trigger_take_photo,
media::mojom::blink::PhotoStatePtr photo_state) {
- if (!service_requests_.Contains(resolver))
- return;
+ DCHECK(service_requests_.Contains(resolver));
if (photo_state.is_null()) {
resolver->Reject(DOMException::Create(kUnknownError, "platform error"));
@@ -610,6 +649,11 @@
return;
}
+ photo_settings_ = PhotoSettings();
+ photo_settings_.setImageHeight(photo_state->height->current);
+ photo_settings_.setImageWidth(photo_state->width->current);
+ // TODO(mcasas): collect the remaining two entries https://crbug.com/732521.
+
photo_capabilities_ = PhotoCapabilities::Create();
photo_capabilities_->SetRedEyeReduction(photo_state->red_eye_reduction);
// TODO(mcasas): Remove the explicit MediaSettingsRange::create() when
@@ -637,27 +681,15 @@
return;
}
- // If this is a response to a SetMediaTrackConstraints() request, it will have
- // the original constraints to apply: Resolve() with those; Resolve() with the
- // |photo_capabilities_| otherwise.
- const HeapVector<MediaTrackConstraintSet>& originalConstraints =
- service_requests_.at(resolver);
- if (originalConstraints.IsEmpty()) {
- resolver->Resolve(photo_capabilities_);
- } else {
- MediaTrackConstraints constraints;
- constraints.setAdvanced(originalConstraints);
- resolver->Resolve(constraints);
- }
-
+ (*resolve_function)(resolver);
service_requests_.erase(resolver);
}
void ImageCapture::OnMojoSetOptions(ScriptPromiseResolver* resolver,
+ PromiseResolverFunction resolve_function,
bool trigger_take_photo,
bool result) {
- if (!service_requests_.Contains(resolver))
- return;
+ DCHECK(service_requests_.Contains(resolver));
if (!result) {
resolver->Reject(DOMException::Create(kUnknownError, "setOptions failed"));
@@ -668,15 +700,15 @@
// Retrieve the current device status after setting the options.
service_->GetPhotoState(
stream_track_->Component()->Source()->Id(),
- ConvertToBaseCallback(
- WTF::Bind(&ImageCapture::OnMojoGetPhotoState, WrapPersistent(this),
- WrapPersistent(resolver), trigger_take_photo)));
+ ConvertToBaseCallback(WTF::Bind(
+ &ImageCapture::OnMojoGetPhotoState, WrapPersistent(this),
+ WrapPersistent(resolver), WTF::Passed(std::move(resolve_function)),
+ trigger_take_photo)));
}
void ImageCapture::OnMojoTakePhoto(ScriptPromiseResolver* resolver,
media::mojom::blink::BlobPtr blob) {
- if (!service_requests_.Contains(resolver))
- return;
+ DCHECK(service_requests_.Contains(resolver));
// TODO(mcasas): Should be using a mojo::StructTraits.
if (blob->data.IsEmpty()) {
@@ -790,11 +822,34 @@
void ImageCapture::OnServiceConnectionError() {
service_.reset();
- for (const auto& resolver : service_requests_)
- resolver.key->Reject(DOMException::Create(kNotFoundError, kNoServiceError));
+ for (ScriptPromiseResolver* resolver : service_requests_)
+ resolver->Reject(DOMException::Create(kNotFoundError, kNoServiceError));
service_requests_.clear();
}
+void ImageCapture::ResolveWithNothing(ScriptPromiseResolver* resolver) {
+ DCHECK(resolver);
+ resolver->Resolve();
+}
+
+void ImageCapture::ResolveWithPhotoSettings(ScriptPromiseResolver* resolver) {
+ DCHECK(resolver);
+ resolver->Resolve(photo_settings_);
+}
+
+void ImageCapture::ResolveWithPhotoCapabilities(
+ ScriptPromiseResolver* resolver) {
+ DCHECK(resolver);
+ resolver->Resolve(photo_capabilities_);
+}
+
+void ImageCapture::ResolveWithMediaTrackConstraints(
+ MediaTrackConstraints constraints,
+ ScriptPromiseResolver* resolver) {
+ DCHECK(resolver);
+ resolver->Resolve(constraints);
+}
+
DEFINE_TRACE(ImageCapture) {
visitor->Trace(stream_track_);
visitor->Trace(capabilities_);
diff --git a/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
index 1c096df1..600144c8 100644
--- a/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
+++ b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h
@@ -12,6 +12,7 @@
#include "media/capture/mojo/image_capture.mojom-blink.h"
#include "modules/EventTargetModules.h"
#include "modules/ModulesExport.h"
+#include "modules/imagecapture/PhotoSettings.h"
#include "modules/mediastream/MediaTrackCapabilities.h"
#include "modules/mediastream/MediaTrackConstraintSet.h"
#include "modules/mediastream/MediaTrackSettings.h"
@@ -24,11 +25,10 @@
class MediaStreamTrack;
class MediaTrackConstraints;
class PhotoCapabilities;
-class PhotoSettings;
class ScriptPromiseResolver;
class WebImageCaptureFrameGrabber;
-// TODO(mcasas): Consideradding a LayoutTest checking that this class is not
+// TODO(mcasas): Consider adding a LayoutTest checking that this class is not
// garbage collected while it has event listeners.
class MODULES_EXPORT ImageCapture final
: public EventTargetWithInlineData,
@@ -56,6 +56,7 @@
MediaStreamTrack* videoStreamTrack() const { return stream_track_.Get(); }
ScriptPromise getPhotoCapabilities(ScriptState*);
+ ScriptPromise getPhotoSettings(ScriptState*);
ScriptPromise setOptions(ScriptState*,
const PhotoSettings&,
@@ -79,12 +80,17 @@
DECLARE_VIRTUAL_TRACE();
private:
+ using PromiseResolverFunction =
+ std::unique_ptr<Function<void(ScriptPromiseResolver*)>>;
+
ImageCapture(ExecutionContext*, MediaStreamTrack*);
void OnMojoGetPhotoState(ScriptPromiseResolver*,
+ PromiseResolverFunction,
bool trigger_take_photo,
media::mojom::blink::PhotoStatePtr);
void OnMojoSetOptions(ScriptPromiseResolver*,
+ PromiseResolverFunction,
bool trigger_take_photo,
bool result);
void OnMojoTakePhoto(ScriptPromiseResolver*, media::mojom::blink::BlobPtr);
@@ -92,6 +98,12 @@
void UpdateMediaTrackCapabilities(media::mojom::blink::PhotoStatePtr);
void OnServiceConnectionError();
+ void ResolveWithNothing(ScriptPromiseResolver*);
+ void ResolveWithPhotoSettings(ScriptPromiseResolver*);
+ void ResolveWithPhotoCapabilities(ScriptPromiseResolver*);
+ void ResolveWithMediaTrackConstraints(MediaTrackConstraints,
+ ScriptPromiseResolver*);
+
Member<MediaStreamTrack> stream_track_;
std::unique_ptr<WebImageCaptureFrameGrabber> frame_grabber_;
media::mojom::blink::ImageCapturePtr service_;
@@ -99,12 +111,11 @@
MediaTrackCapabilities capabilities_;
MediaTrackSettings settings_;
MediaTrackConstraintSet current_constraints_;
+ PhotoSettings photo_settings_;
Member<PhotoCapabilities> photo_capabilities_;
- HeapHashMap<Member<ScriptPromiseResolver>,
- HeapVector<MediaTrackConstraintSet>>
- service_requests_;
+ HeapHashSet<Member<ScriptPromiseResolver>> service_requests_;
};
} // namespace blink
diff --git a/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl
index c9b84af..e819110 100644
--- a/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl
+++ b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl
@@ -16,6 +16,7 @@
[ImplementedAs=videoStreamTrack] readonly attribute MediaStreamTrack track;
[CallWith=ScriptState] Promise<PhotoCapabilities> getPhotoCapabilities();
+ [CallWith=ScriptState] Promise<PhotoSettings> getPhotoSettings();
[CallWith=ScriptState] Promise<void> setOptions(PhotoSettings photoSettings);
[CallWith=ScriptState] Promise<Blob> takePhoto(optional PhotoSettings photoSettings);
[CallWith=ScriptState] Promise<ImageBitmap> grabFrame();
