[webkit-reviews] review granted: [Bug 180005] [GTK][WPE] Add "enable-encryptedmedia" property to WebKitWebSettings : [Attachment 327550] Patch

[bugzilla-daemon at webkit.org]

Michael Catanzaro <mcatanzaro at igalia.com> has granted Yacine Bandou
<bandou.yacine at gmail.com>'s request for review:
Bug 180005: [GTK][WPE] Add "enable-encryptedmedia" property to
WebKitWebSettings
https://bugs.webkit.org/show_bug.cgi?id=180005

Attachment 327550: Patch

https://bugs.webkit.org/attachment.cgi?id=327550&action=review




--- Comment #6 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 327550
  --> https://bugs.webkit.org/attachment.cgi?id=327550
Patch

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

I was going to give this r- for the reason I mentioned above, and ask you to
change the runtime default to depend on ENABLE_ENCRYPTED_MEDIA instead... but
now that Carlos Lopez has pointed out there is precedent for having API that
does not work except in custom builds, I guess we might as well. But please do
document that it as Carlos Lopez requested.

By the way, it's interesting that you've pointed out that it is currently
impossible to enable encrypted media support. I wonder how long it has been
since anybody has tried to test this. Probably a long time.

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1305
> +	   g_param_spec_boolean("enable-encryptedmedia",

Please separate:

PROP_ENABLE_ENCRYPTED_MEDIA

"enable-encrypted-media"

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3169
> + * webkit_settings_get_enable_encryptedmedia:

encrypted_media for the function names too.

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3174
> + * Returns: %TRUE If EncryptedMedia support is enabled or %FALSE otherwise.

"if" (lowercase)