Bug 148527 - Media Session: MediaSession constructor 'kind' argument optional
Summary: Media Session: MediaSession constructor 'kind' argument optional
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-08-27 08:33 PDT by Eric Carlson
Modified: 2017-06-19 09:03 PDT (History)
4 users (show)

See Also:


Attachments
Proposed patch (19.34 KB, patch)
2015-08-27 09:03 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing. (19.33 KB, patch)
2015-08-27 09:34 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing. (19.33 KB, patch)
2015-08-27 09:56 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
YAPFL (19.33 KB, patch)
2015-08-27 10:08 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2015-08-27 08:33:34 PDT
The MediaSession constructor's "kind" argument is optional and had a default value.
Comment 1 Radar WebKit Bug Importer 2015-08-27 08:34:08 PDT
<rdar://problem/22456091>
Comment 2 Eric Carlson 2015-08-27 09:03:43 PDT
Created attachment 260058 [details]
Proposed patch
Comment 3 WebKit Commit Bot 2015-08-27 09:05:52 PDT
Attachment 260058 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSMediaSessionCustom.cpp:30:  You should not add a blank line before implementation file's own header.  [build/include_order] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Jer Noble 2015-08-27 09:17:49 PDT
Comment on attachment 260058 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260058&action=review

> Source/WebCore/Modules/mediasession/MediaSession.cpp:49
> +    // 2. Set media sessionâs current media session type to the corresponding media session type of media session category.

Unicode quotation mark? (By the way, this has to be the worst spec language since "boxes in boxes".)

> Source/WebCore/bindings/js/JSMediaSessionCustom.cpp:62
> +    String kind;
> +    if (exec->argumentCount() > 0) {
> +        JSString* kindString = exec->argument(0).toString(exec);
> +        if (UNLIKELY(exec->hadException()))
> +            return JSValue::encode(jsUndefined());
> +        kind = kindString->value(exec);
> +        if (kind != "content" && kind != "transient" && kind != "transient-solo" && kind != "ambient")
> +            return throwArgumentMustBeEnumError(*exec, 0, "kind", "MediaSession", nullptr, "\"content\", \"transient\", \"transient-solo\", \"ambient\"");
> +    } else
> +        kind = "content";
> +
> +    RefPtr<MediaSession> object = MediaSession::create(*context, kind);
> +    return JSValue::encode(asObject(toJS(exec, castedThis->globalObject(), object.get())));

Ugh.  I guess there's no way to do this in bindings?
Comment 5 Eric Carlson 2015-08-27 09:28:27 PDT
(In reply to comment #4)
> Comment on attachment 260058 [details]
> Proposed patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=260058&action=review
> 
> > Source/WebCore/Modules/mediasession/MediaSession.cpp:49
> > +    // 2. Set media sessionâs current media session type to the corresponding media session type of media session category.
> 
> Unicode quotation mark? (By the way, this has to be the worst spec language
> since "boxes in boxes".)
> 

Oops!

> > Source/WebCore/bindings/js/JSMediaSessionCustom.cpp:62
> > +    String kind;
> > +    if (exec->argumentCount() > 0) {
> > +        JSString* kindString = exec->argument(0).toString(exec);
> > +        if (UNLIKELY(exec->hadException()))
> > +            return JSValue::encode(jsUndefined());
> > +        kind = kindString->value(exec);
> > +        if (kind != "content" && kind != "transient" && kind != "transient-solo" && kind != "ambient")
> > +            return throwArgumentMustBeEnumError(*exec, 0, "kind", "MediaSession", nullptr, "\"content\", \"transient\", \"transient-solo\", \"ambient\"");
> > +    } else
> > +        kind = "content";
> > +
> > +    RefPtr<MediaSession> object = MediaSession::create(*context, kind);
> > +    return JSValue::encode(asObject(toJS(exec, castedThis->globalObject(), object.get())));
> 
> Ugh.  I guess there's no way to do this in bindings?

Not for enum arguments, only "[Default=Undefined] optional" or "[Default=NullString] optional"
Comment 6 Eric Carlson 2015-08-27 09:34:33 PDT
Created attachment 260059 [details]
Patch for landing.
Comment 7 WebKit Commit Bot 2015-08-27 09:51:26 PDT
Attachment 260059 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSMediaSessionCustom.cpp:30:  You should add a blank line after implementation file's own header.  [build/include_order] [4]
ERROR: Source/WebCore/bindings/js/JSMediaSessionCustom.cpp:30:  You should not add a blank line before implementation file's own header.  [build/include_order] [4]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Eric Carlson 2015-08-27 09:56:30 PDT
Created attachment 260061 [details]
Patch for landing.
Comment 9 WebKit Commit Bot 2015-08-27 10:01:16 PDT
Attachment 260061 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSMediaSessionCustom.cpp:30:  You should not add a blank line before implementation file's own header.  [build/include_order] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Eric Carlson 2015-08-27 10:08:21 PDT
Created attachment 260063 [details]
YAPFL
Comment 11 WebKit Commit Bot 2015-08-27 10:34:05 PDT
Comment on attachment 260063 [details]
YAPFL

Clearing flags on attachment: 260063

Committed r189031: <http://trac.webkit.org/changeset/189031>