Commit b61f1b9f authored by Matt Wolenetz's avatar Matt Wolenetz Committed by Commit Bot

MSE: DCHECK that MediaSourceRegistry |url| params are non-empty

Improves MediaSourceRegistry code readability by DCHECKing in its
URLRegistry overrides and in its LookupMediaSource interface that the
KURL |url| in the former, and the wtf::String |url| in the latter are
not empty (or null). The former is prevented by the FILE API
PublicUrlManager already, and the latter is now (by this change)
protected against in MediaSourceAttachment::LookupMediaSource().

This is a follow-up on a review comment in an MSE-in-Worker CL:
https://chromium-review.googlesource.com/c/chromium/src/+/2300804/7/third_party/blink/renderer/modules/mediasource/media_source_registry_impl.cc#74

BUG=878133

Change-Id: I2ef93b8d3fc894ee71b1a605acdc58df88e722d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346797
Auto-Submit: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: default avatarPhilip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797640}
parent 8810c74e
......@@ -24,7 +24,7 @@ MediaSource* MediaSourceAttachment::LookupMediaSource(const String& url) {
// The only expected caller is an HTMLMediaElement on the main thread.
DCHECK(IsMainThread());
if (!registry_)
if (!registry_ || url.IsEmpty())
return nullptr;
// This cast is safe because the only setter of |registry_| is SetRegistry().
......
......@@ -18,6 +18,10 @@ namespace blink {
// scoped_refptr.
class CORE_EXPORT MediaSourceRegistry : public URLRegistry {
public:
// Finds the attachment, if any, registered with |url| in the
// MediaSourceRegistry implementation. |url| must be non-empty. If such an
// active registration for |url| is not found, returns an unset
// scoped_refptr<MediaSourceAttachment>.
virtual scoped_refptr<MediaSourceAttachment> LookupMediaSource(
const String& url) = 0;
};
......
......@@ -47,6 +47,7 @@ void MediaSourceRegistryImpl::RegisterURL(SecurityOrigin*,
URLRegistrable* registrable) {
DCHECK(IsMainThread());
DCHECK_EQ(&registrable->Registry(), this);
DCHECK(!url.IsEmpty()); // Caller of interface should already enforce this.
DVLOG(1) << __func__ << " url=" << url;
......@@ -58,6 +59,7 @@ void MediaSourceRegistryImpl::RegisterURL(SecurityOrigin*,
void MediaSourceRegistryImpl::UnregisterURL(const KURL& url) {
DVLOG(1) << __func__ << " url=" << url;
DCHECK(IsMainThread());
DCHECK(!url.IsEmpty()); // Caller of interface should already enforce this.
auto iter = media_sources_.find(url.GetString());
if (iter == media_sources_.end())
......@@ -71,8 +73,8 @@ void MediaSourceRegistryImpl::UnregisterURL(const KURL& url) {
scoped_refptr<MediaSourceAttachment> MediaSourceRegistryImpl::LookupMediaSource(
const String& url) {
DCHECK(IsMainThread());
return url.IsNull() ? scoped_refptr<MediaSourceAttachment>()
: media_sources_.at(url);
DCHECK(!url.IsEmpty());
return media_sources_.at(url);
}
MediaSourceRegistryImpl::MediaSourceRegistryImpl() {
......
......@@ -39,7 +39,7 @@ class MediaSourceRegistryImpl final : public MediaSourceRegistry {
// MediaSourceRegistry override that finds |url| in |media_sources_| and
// returns the corresponding scoped_refptr if found. Otherwise, returns an
// unset scoped_refptr.
// unset scoped_refptr. |url| must be non-empty.
scoped_refptr<MediaSourceAttachment> LookupMediaSource(
const String& url) override;
......
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