Bug 122301 – URLMediaSource.idl and URLMediaStream.idl are wrong

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Chris Dumez]

URLMediaSource.idl and URLMediaStream.idl are partial interfaces that extend the URL interface. However, this has no impact at the moment and it causes the bindings generator to generate a warning because the URL interface does not exist. The interface is called DOMURL in WebKit.

I assume this code was imported from Blink where the IDL interfaces' name matches their JS name (I tried to do the same in WebKit but Darin thought the IDL interfaces' name should be consistent with the implementation name instead).

You should update those partial interfaces as follows:
- partial interface URL {
+ partial interface DOMURL {

so that is actually does something and affects the behavior on JS side.

Jer Noble, could you please look into this since I believe you are working on this?

--
Bindings generator warning:
Use of uninitialized value $targetIdlFile in hash element at Source/WebCore/bindings/scripts/preprocess-idls.pl line 128.

We should probably have the script die instead in this case and print a useful error message to let the developer know its partial interface (or IDL implements statement is wrong) is wrong.

[Darin Adler]

(In reply to comment #0)
> (I tried to do the same in WebKit but Darin thought the IDL interfaces' name should be consistent with the implementation name instead).

That’s incorrect. My comment was about the IDL file names, not about the IDL interface names inside the files.

[Chris Dumez]

(In reply to comment #1)
> (In reply to comment #0)
> > (I tried to do the same in WebKit but Darin thought the IDL interfaces' name should be consistent with the implementation name instead).
> 
> That’s incorrect. My comment was about the IDL file names, not about the IDL interface names inside the files.

Ok, then I might have misunderstood you at the time. In any case, it makes the generator's work easier if the interface's name matches the file name (at I believe the generators relies on that).

Those files should probably be renamed to DOMURLMediaSource.idl / DOMURLMediaStream.idl then, for consistency.

[Jer Noble]

(In reply to comment #0)
> Jer Noble, could you please look into this since I believe you are working on this?

Sure thing.

[Philippe Normand]

See bug 122443

[Jer Noble]

Created attachment 213590 [details]
Patch

[Jer Noble]

Created attachment 213591 [details]
Patch

Here's a version of the patch with rename detection enabled.

[Jer Noble]

Bug 122444 was committed while I was writing this patch, so I need to incorporate it and rebaseline.

[Philippe Normand]

Comment on attachment 213591 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        by a MEDIA_SOURCE conditional.

I guess you mean MEDIA_STREAM here. Sorry about the collision with bug 122444

[Jer Noble]

Created attachment 213594 [details]
Patch

[Philippe Normand]

Comment on attachment 213594 [details]
Patch

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

> Source/WebCore/Modules/mediastream/DOMURLMediaStream.idl:31
> +    Conditional=MEDIA_SOURCE

MEDIA_STREAM :)

[Jer Noble]

ack!(In reply to comment #10)
> (From update of attachment 213594 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=213594&action=review
> 
> > Source/WebCore/Modules/mediastream/DOMURLMediaStream.idl:31
> > +    Conditional=MEDIA_SOURCE
> 
> MEDIA_STREAM :)

Whoops. :)

[Jer Noble]

Created attachment 213596 [details]
Patch

MEDIA_SOURCE -> MEDIA_STREAM

[Philippe Normand]

Comment on attachment 213596 [details]
Patch

Ok, thanks!

[EFL EWS Bot]

Comment on attachment 213596 [details]
Patch

Attachment 213596 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/3649002

[Philippe Normand]

/mnt/eflews/webkit/WebKit/WebKitBuild/Release/DerivedSources/WebCore/JSDOMURL.cpp:178:23: error: 'JSC::EncodedJSValue WebCore::jsDOMURLConstructorFunctionCreateObjectURL1(JSC::ExecState*)' defined but not used [-Werror=unused-function]
cc1plus: all warnings being treated as errors

I think this patch should depend on the one in bug 122443

[EFL EWS Bot]

Comment on attachment 213596 [details]
Patch

Attachment 213596 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/3634020

[Jer Noble]

Created attachment 213607 [details]
Patch for committing.

[Jer Noble]

Committed r157054: <http://trac.webkit.org/changeset/157054>