Commit 31c6ef91 authored by mlamouri's avatar mlamouri Committed by Commit bot

Media Session: use dictionary instead of an interface for MediaImage.

Using an interface makes the usage of MediaImage needlessly more
complicated than needed. A dictionary should keep the feature working
roughly the same way and improve developer's ergonomics.

Spec change: https://github.com/WICG/mediasession/pull/162

BUG=676995
R=zqzhang@chromium.org

Review-Url: https://codereview.chromium.org/2612003002
Cr-Commit-Position: refs/heads/master@{#441962}
parent 1c53fd3c
<!DOCTYPE html>
<title>MediaImage interface</title>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script>
test(function() {
var image = new MediaImage({});
assert_not_equals(image, null);
var exception = false;
try {
image = new MediaImage("foobar");
} catch (e) {
exception = true;
}
assert_true(exception);
exception = false;
try {
image = new MediaImage(42);
} catch (e) {
exception = true;
}
assert_true(exception);
}, 'Test that MediaImage is constructed using a dictionary');
test (function() {
var image = new MediaImage({
src: 'foo', sizes: 'bar', type: 'plop'});
assert_greater_than(image.src.indexOf('foo'), -1);
assert_equals(image.sizes, 'bar');
assert_equals(image.type, 'plop');
}, 'Test the different values allowed in MediaImage init dictionary');
test (function() {
var image = new MediaImage({});
assert_equals(image.src, document.URL);
assert_equals(image.sizes, '');
assert_equals(image.type, '');
}, 'Test the default values for MediaImage');
test (function() {
var image = new MediaMetadata({ randomValueThatWillNotBeAdded: '... hopefully ;)' });
assert_equals(image.randomValueThatWillNotBeAdded, undefined);
}, 'Test that passing unknown values to the dictionary is a no-op');
</script>
......@@ -18,17 +18,16 @@
}, "Test that MediaMetadata constructor can take no parameter");
test(function() {
var image1 = { src: 'http://example.com/1', sizes: 'sizes1', type: 'type1' };
var image2 = { src: 'http://example.com/2', sizes: 'sizes2', type: 'type2' };
var metadata = new MediaMetadata({
title: 'foo', album: 'bar', artist: 'plop',
artwork: [ { src: 'src1', sizes: 'sizes1', type: 'type1'},
{ src: 'src2', sizes: 'sizes2', type: 'type2'} ] });
title: 'foo', album: 'bar', artist: 'plop', artwork: [ image1, image2 ]
});
assert_equals(metadata.title, 'foo');
assert_equals(metadata.album, 'bar');
assert_equals(metadata.artist, 'plop');
assert_equals(2, metadata.artwork.length);
var image1 = new MediaImage({ src: 'src1', sizes: 'sizes1', type: 'type1'});
var image2 = new MediaImage({ src: 'src2', sizes: 'sizes2', type: 'type2'});
assert_equals(metadata.artwork.length, 2);
assert_equals(metadata.artwork[0].src, image1.src);
assert_equals(metadata.artwork[0].sizes, image1.sizes);
assert_equals(metadata.artwork[0].type, image1.type);
......@@ -59,10 +58,11 @@
}, 'Test that passing unknown values to the dictionary is a no-op');
test(function() {
var image1 = { src: 'http://example.com/1', sizes: 'sizes1', type: 'type1' };
var image2 = { src: 'http://example.com/2', sizes: 'sizes2', type: 'type2' };
var metadata = new MediaMetadata({
title: 'foo', album: 'bar', artist: 'plop',
artwork: [ { src: 'src1', sizes: 'sizes1', type: 'type1'},
{ src: 'src2', sizes: 'sizes2', type: 'type2'} ] });
title: 'foo', album: 'bar', artist: 'plop', artwork: [ image1, image2 ]
});
metadata.title = 'something else';
assert_equals(metadata.title, 'something else');
......@@ -73,7 +73,7 @@
metadata.artist = 'someone else';
assert_equals(metadata.artist, 'someone else');
var image = new MediaImage({ src: 'http://example.com/', sizes: '40x40', type: 'image/png' });
var image = { src: 'http://example.com/', sizes: '40x40', type: 'image/png' };
metadata.artwork = [ image ];
assert_equals(metadata.artwork.length, 1);
assert_equals(metadata.artwork[0].src, 'http://example.com/');
......@@ -81,4 +81,92 @@
assert_equals(metadata.artwork[0].type, 'image/png');
}, "Test that MediaMetadata is read/write");
test(function() {
var metadata = new MediaMetadata({ artwork: [ { src: 'http://foo.com/' } ] });
assert_throws(new TypeError(), _ => {
metadata.artwork.push({
src: 'http://example.com/', sizes: '40x40', type: 'image/png',
});
});
metadata.artwork[0].src = 'bar';
assert_equals(metadata.artwork[0].src, 'http://foo.com/');
}, "Test that MediaMetadat.artwork can't be modified");
test(function() {
var metadata = new MediaMetadata({ artwork: [{
src: 'http://example.com/', sizes: '40x40', type: 'image/png',
some_other_value: 'foo',
}]});
assert_equals(metadata.artwork[0].src, 'http://example.com/');
assert_equals(metadata.artwork[0].sizes, '40x40');
assert_equals(metadata.artwork[0].type, 'image/png');
assert_false('some_other_value' in metadata.artwork[0]);
metadata.artwork[0].something_else = 'bar';
assert_false('something_else' in metadata.artwork[0]);
}, "Test that MediaMetadata.artwork will not expose unknown properties");
test(function() {
var metadata = new MediaMetadata({ artwork: [
{ src: 'http://example.com/1', sizes: '40x40', type: 'image/png' },
{ src: 'http://example.com/2', sizes: '40x40', type: 'image/png' },
]});
assert_true(Object.isFrozen(metadata.artwork));
for (var i = 0; i < metadata.artwork.length; ++i)
assert_true(Object.isFrozen(metadata.artwork[i]));
}, "Test that MediaMetadata.artwork is Frozen");
test(function() {
var metadata = new MediaMetadata({ artwork: [
{ src: 'http://example.com', sizes: '40x40', type: 'image/png' },
{ src: '../foo', sizes: '40x40', type: 'image/png' },
{ src: '/foo/bar', sizes: '40x40', type: 'image/png' },
]});
assert_equals(metadata.artwork[0].src, new URL('http://example.com', document.URL).href)
assert_equals(metadata.artwork[1].src, new URL('../foo', document.URL).href)
assert_equals(metadata.artwork[2].src, new URL('/foo/bar', document.URL).href)
}, "Test that MediaMetadata.artwork returns parsed urls");
test(function() {
var metadata = 42;
assert_throws(new TypeError(), _ => {
metadata
new MediaMetadata({ artwork: [ { src: 'http://[example.com]' }] });
});
assert_equals(metadata, 42);
metadata = new MediaMetadata();
assert_throws(new TypeError(), _ => {
metadata.artwork = [
// Valid url.
{ src: 'http://example.com' },
// Invalid url.
{ src: 'http://example.com:demo' },
];
});
assert_equals(metadata.artwork.length, 0);
}, "Test that MediaMetadata throws when setting an invalid url");
test(function() {
var metadata = new MediaMetadata({ artwork: [ { src: 'foo.jpg' } ] });
assert_equals(metadata.artwork[0].type, '');
assert_equals(metadata.artwork[0].sizes, '');
}, "Test MediaImage default values");
test(function() {
assert_throws(new TypeError(), _ => {
new MediaMetadata({ artwork: [ {} ] });
});
var metadata = new MediaMetadata();
assert_throws(new TypeError(), _ => {
metadata.artwork = [ { type: 'image/png', sizes: '40x40' } ];
});
}, "Test that MediaImage.src is required")
</script>
......@@ -54,9 +54,9 @@ async_test(t => {
window.navigator.mediaSession.metadata.title = 'new title';
window.navigator.mediaSession.metadata.album = 'new album';
window.navigator.mediaSession.metadata.artist = 'new artist';
var image = new MediaImage(
{ src: 'http://example.com/', sizes: '40x40', type: 'image/png' });
window.navigator.mediaSession.metadata.artwork = [ image ];
window.navigator.mediaSession.metadata.artwork = [
{ src: 'http://example.com/', sizes: '40x40', type: 'image/png' }
];
// This two last changes are made asynchronously and will go in different
// mojo calls.
......
......@@ -3818,12 +3818,6 @@ interface MediaError
attribute MEDIA_ERR_SRC_NOT_SUPPORTED
getter code
method constructor
interface MediaImage
attribute @@toStringTag
getter sizes
getter src
getter type
method constructor
interface MediaKeyMessageEvent : Event
attribute @@toStringTag
getter message
......
......@@ -6,8 +6,6 @@ import("//third_party/WebKit/Source/modules/modules.gni")
blink_modules_sources("mediasession") {
sources = [
"MediaImage.cpp",
"MediaImage.h",
"MediaMetadata.cpp",
"MediaMetadata.h",
"MediaMetadataSanitizer.cpp",
......
// Copyright 2016 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 "modules/mediasession/MediaImage.h"
#include "core/dom/ExecutionContext.h"
#include "core/inspector/ConsoleMessage.h"
#include "modules/mediasession/MediaImageInit.h"
#include "platform/weborigin/KURL.h"
#include "wtf/text/StringOperators.h"
namespace blink {
// static
MediaImage* MediaImage::create(ExecutionContext* context,
const MediaImageInit& image) {
return new MediaImage(context, image);
}
MediaImage::MediaImage(ExecutionContext* context, const MediaImageInit& image) {
m_src = context->completeURL(image.src());
if (!KURL(ParsedURLString, m_src).isValid()) {
context->addConsoleMessage(
ConsoleMessage::create(JSMessageSource, WarningMessageLevel,
"MediaImage src is invalid: " + image.src()));
}
m_sizes = image.sizes();
m_type = image.type();
}
String MediaImage::src() const {
return m_src;
}
String MediaImage::sizes() const {
return m_sizes;
}
String MediaImage::type() const {
return m_type;
}
} // namespace blink
// Copyright 2016 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.
#ifndef MediaImage_h
#define MediaImage_h
#include "bindings/core/v8/ScriptWrappable.h"
#include "modules/ModulesExport.h"
#include "platform/heap/Handle.h"
#include "wtf/text/WTFString.h"
namespace blink {
class ExecutionContext;
class MediaImageInit;
class MODULES_EXPORT MediaImage final
: public GarbageCollectedFinalized<MediaImage>,
public ScriptWrappable {
DEFINE_WRAPPERTYPEINFO();
public:
static MediaImage* create(ExecutionContext*, const MediaImageInit&);
String src() const;
String sizes() const;
String type() const;
DEFINE_INLINE_TRACE() {}
private:
MediaImage(ExecutionContext*, const MediaImageInit&);
String m_src;
String m_sizes;
String m_type;
};
} // namespace blink
#endif // MediaImage_h
......@@ -2,14 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// https://wicg.github.io/mediasession/#the-mediaimage-interface
// https://wicg.github.io/mediasession/#dictdef-mediaimage
[
ConstructorCallWith=ExecutionContext,
Constructor(MediaImageInit image),
RuntimeEnabled=MediaSession,
] interface MediaImage {
readonly attribute USVString src;
readonly attribute DOMString sizes;
readonly attribute DOMString type;
dictionary MediaImage {
required USVString src;
DOMString sizes = "";
DOMString type = "";
};
// Copyright 2016 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.
// https://wicg.github.io/mediasession/#dictdef-mediaimageinit
dictionary MediaImageInit {
USVString src = "";
DOMString sizes = "";
DOMString type = "";
};
......@@ -4,7 +4,9 @@
#include "modules/mediasession/MediaMetadata.h"
#include "core/dom/ExecutionContext.h"
#include "bindings/core/v8/ExceptionState.h"
#include "bindings/core/v8/ScriptState.h"
#include "bindings/core/v8/ToV8.h"
#include "modules/mediasession/MediaImage.h"
#include "modules/mediasession/MediaMetadataInit.h"
#include "modules/mediasession/MediaSession.h"
......@@ -12,19 +14,20 @@
namespace blink {
// static
MediaMetadata* MediaMetadata::create(ExecutionContext* context,
const MediaMetadataInit& metadata) {
return new MediaMetadata(context, metadata);
MediaMetadata* MediaMetadata::create(ScriptState* scriptState,
const MediaMetadataInit& metadata,
ExceptionState& exceptionState) {
return new MediaMetadata(scriptState, metadata, exceptionState);
}
MediaMetadata::MediaMetadata(ExecutionContext* context,
const MediaMetadataInit& metadata)
MediaMetadata::MediaMetadata(ScriptState* scriptState,
const MediaMetadataInit& metadata,
ExceptionState& exceptionState)
: m_notifySessionTimer(this, &MediaMetadata::notifySessionTimerFired) {
m_title = metadata.title();
m_artist = metadata.artist();
m_album = metadata.album();
for (const auto& image : metadata.artwork())
m_artwork.push_back(MediaImage::create(context, image));
setArtworkInternal(scriptState, metadata.artwork(), exceptionState);
}
String MediaMetadata::title() const {
......@@ -39,10 +42,22 @@ String MediaMetadata::album() const {
return m_album;
}
const HeapVector<Member<MediaImage>>& MediaMetadata::artwork() const {
const HeapVector<MediaImage>& MediaMetadata::artwork() const {
return m_artwork;
}
Vector<v8::Local<v8::Value>> MediaMetadata::artwork(
ScriptState* scriptState) const {
Vector<v8::Local<v8::Value>> result(m_artwork.size());
for (size_t i = 0; i < m_artwork.size(); ++i) {
result[i] =
freezeV8Object(ToV8(m_artwork[i], scriptState), scriptState->isolate());
}
return result;
}
void MediaMetadata::setTitle(const String& title) {
m_title = title;
notifySessionAsync();
......@@ -58,8 +73,10 @@ void MediaMetadata::setAlbum(const String& album) {
notifySessionAsync();
}
void MediaMetadata::setArtwork(const HeapVector<Member<MediaImage>>& artwork) {
m_artwork = artwork;
void MediaMetadata::setArtwork(ScriptState* scriptState,
const HeapVector<MediaImage>& artwork,
ExceptionState& exceptionState) {
setArtworkInternal(scriptState, artwork, exceptionState);
notifySessionAsync();
}
......@@ -79,6 +96,25 @@ void MediaMetadata::notifySessionTimerFired(TimerBase*) {
m_session->onMetadataChanged();
}
void MediaMetadata::setArtworkInternal(ScriptState* scriptState,
const HeapVector<MediaImage>& artwork,
ExceptionState& exceptionState) {
HeapVector<MediaImage> processedArtwork(artwork);
for (MediaImage& image : processedArtwork) {
KURL url = scriptState->getExecutionContext()->completeURL(image.src());
if (!url.isValid()) {
exceptionState.throwTypeError("'" + image.src() +
"' can't be resolved to a valid URL.");
return;
}
image.setSrc(url);
}
DCHECK(!exceptionState.hadException());
m_artwork.swap(processedArtwork);
}
DEFINE_TRACE(MediaMetadata) {
visitor->trace(m_artwork);
visitor->trace(m_session);
......
......@@ -13,10 +13,11 @@
namespace blink {
class ExecutionContext;
class ExceptionState;
class MediaImage;
class MediaMetadataInit;
class MediaSession;
class ScriptState;
// Implementation of MediaMetadata interface from the Media Session API.
// The MediaMetadata object is linked to a MediaSession that owns it. When one
......@@ -30,17 +31,23 @@ class MODULES_EXPORT MediaMetadata final
DEFINE_WRAPPERTYPEINFO();
public:
static MediaMetadata* create(ExecutionContext*, const MediaMetadataInit&);
static MediaMetadata* create(ScriptState*,
const MediaMetadataInit&,
ExceptionState&);
String title() const;
String artist() const;
String album() const;
const HeapVector<Member<MediaImage>>& artwork() const;
Vector<v8::Local<v8::Value>> artwork(ScriptState*) const;
// Internal use only, returns a reference to m_artwork instead of a Frozen
// copy of a MediaImage array.
const HeapVector<MediaImage>& artwork() const;
void setTitle(const String&);
void setArtist(const String&);
void setAlbum(const String&);
void setArtwork(const HeapVector<Member<MediaImage>>&);
void setArtwork(ScriptState*, const HeapVector<MediaImage>&, ExceptionState&);
// Called by MediaSession to associate or de-associate itself.
void setSession(MediaSession*);
......@@ -48,7 +55,7 @@ class MODULES_EXPORT MediaMetadata final
DECLARE_VIRTUAL_TRACE();
private:
MediaMetadata(ExecutionContext*, const MediaMetadataInit&);
MediaMetadata(ScriptState*, const MediaMetadataInit&, ExceptionState&);
// Called when one of the metadata fields is updated from script. It will
// notify the session asynchronously in order to bundle multiple call in one
......@@ -59,10 +66,16 @@ class MODULES_EXPORT MediaMetadata final
// modified.
void notifySessionTimerFired(TimerBase*);
// Make an internal copy of the MediaImage vector with some internal steps
// such as parsing of the src property.
void setArtworkInternal(ScriptState*,
const HeapVector<MediaImage>&,
ExceptionState&);
String m_title;
String m_artist;
String m_album;
HeapVector<Member<MediaImage>> m_artwork;
HeapVector<MediaImage> m_artwork;
Member<MediaSession> m_session;
Timer<MediaMetadata> m_notifySessionTimer;
......
......@@ -5,12 +5,13 @@
// https://wicg.github.io/mediasession/#the-mediametadata-interface
[
ConstructorCallWith=ExecutionContext,
ConstructorCallWith=ScriptState,
Constructor(optional MediaMetadataInit metadata),
RaisesException=Constructor,
RuntimeEnabled=MediaSession,
] interface MediaMetadata {
attribute DOMString title;
attribute DOMString artist;
attribute DOMString album;
attribute FrozenArray<MediaImage> artwork;
[CallWith=ScriptState, RaisesException=Setter] attribute FrozenArray<MediaImage> artwork;
};
......@@ -8,5 +8,5 @@ dictionary MediaMetadataInit {
DOMString title = "";
DOMString artist = "";
DOMString album = "";
sequence<MediaImageInit> artwork = [];
sequence<MediaImage> artwork = [];
};
......@@ -34,9 +34,8 @@ const size_t kMaxNumberOfMediaImages = 10;
const size_t kMaxNumberOfImageSizes = 10;
bool checkMediaImageSrcSanity(const KURL& src, ExecutionContext* context) {
// Console warning for invalid src is printed upon MediaImage creation.
if (!src.isValid())
return false;
// Invalid URLs will be rejected early on.
DCHECK(src.isValid());
if (!src.protocolIs(url::kHttpScheme) && !src.protocolIs(url::kHttpsScheme) &&
!src.protocolIs(url::kDataScheme) && !src.protocolIs(url::kBlobScheme)) {
......@@ -46,6 +45,7 @@ bool checkMediaImageSrcSanity(const KURL& src, ExecutionContext* context) {
src.getString()));
return false;
}
DCHECK(src.getString().is8Bit());
if (src.getString().length() > url::kMaxURLChars) {
context->addConsoleMessage(ConsoleMessage::create(
......@@ -59,21 +59,19 @@ bool checkMediaImageSrcSanity(const KURL& src, ExecutionContext* context) {
// Sanitize MediaImage and do mojo serialization. Returns null when
// |image.src()| is bad.
blink::mojom::blink::MediaImagePtr sanitizeMediaImageAndConvertToMojo(
const MediaImage* image,
const MediaImage& image,
ExecutionContext* context) {
DCHECK(image);
blink::mojom::blink::MediaImagePtr mojoImage;
KURL url = KURL(ParsedURLString, image->src());
KURL url = KURL(ParsedURLString, image.src());
if (!checkMediaImageSrcSanity(url, context))
return mojoImage;
mojoImage = blink::mojom::blink::MediaImage::New();
mojoImage->src = url;
mojoImage->type = image->type().left(kMaxImageTypeLength);
mojoImage->type = image.type().left(kMaxImageTypeLength);
for (const auto& webSize :
WebIconSizesParser::parseIconSizes(image->sizes())) {
WebIconSizesParser::parseIconSizes(image.sizes())) {
mojoImage->sizes.push_back(webSize);
if (mojoImage->sizes.size() == kMaxNumberOfImageSizes) {
context->addConsoleMessage(ConsoleMessage::create(
......@@ -101,9 +99,9 @@ MediaMetadataSanitizer::sanitizeAndConvertToMojo(const MediaMetadata* metadata,
mojoMetadata->artist = metadata->artist().left(kMaxStringLength);
mojoMetadata->album = metadata->album().left(kMaxStringLength);
for (const auto image : metadata->artwork()) {
for (const MediaImage& image : metadata->artwork()) {
blink::mojom::blink::MediaImagePtr mojoImage =
sanitizeMediaImageAndConvertToMojo(image.get(), context);
sanitizeMediaImageAndConvertToMojo(image, context);
if (!mojoImage.is_null())
mojoMetadata->artwork.push_back(std::move(mojoImage));
if (mojoMetadata->artwork.size() == kMaxNumberOfMediaImages) {
......
......@@ -144,7 +144,6 @@ modules_idl_files =
"mediacapturefromelement/CanvasCaptureMediaStreamTrack.idl",
"mediarecorder/BlobEvent.idl",
"mediarecorder/MediaRecorder.idl",
"mediasession/MediaImage.idl",
"mediasession/MediaMetadata.idl",
"mediasession/MediaSession.idl",
"mediasource/MediaSource.idl",
......@@ -422,7 +421,7 @@ modules_dictionary_idl_files =
"indexeddb/IDBVersionChangeEventInit.idl",
"mediarecorder/BlobEventInit.idl",
"mediarecorder/MediaRecorderOptions.idl",
"mediasession/MediaImageInit.idl",
"mediasession/MediaImage.idl",
"mediasession/MediaMetadataInit.idl",
"mediastream/ConstrainBooleanParameters.idl",
"mediastream/ConstrainDOMStringParameters.idl",
......@@ -731,8 +730,8 @@ generated_modules_dictionary_files = [
"$blink_modules_output_dir/mediarecorder/BlobEventInit.h",
"$blink_modules_output_dir/mediarecorder/MediaRecorderOptions.cpp",
"$blink_modules_output_dir/mediarecorder/MediaRecorderOptions.h",
"$blink_modules_output_dir/mediasession/MediaImageInit.cpp",
"$blink_modules_output_dir/mediasession/MediaImageInit.h",
"$blink_modules_output_dir/mediasession/MediaImage.cpp",
"$blink_modules_output_dir/mediasession/MediaImage.h",
"$blink_modules_output_dir/mediasession/MediaMetadataInit.cpp",
"$blink_modules_output_dir/mediasession/MediaMetadataInit.h",
"$blink_modules_output_dir/mediastream/ConstrainBooleanParameters.cpp",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment