[webkit-reviews] review granted: [Bug 208923] [EME] Issue an "encrypted" event when a new encrypted initialization segment is encountered : [Attachment 393266] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 208923: [EME] Issue an "encrypted" event when a new encrypted
initialization segment is encountered
https://bugs.webkit.org/show_bug.cgi?id=208923

Attachment 393266: Patch

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




--- Comment #3 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 393266
  --> https://bugs.webkit.org/attachment.cgi?id=393266
Patch

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

r=me once the bots are happy

> Source/WTF/wtf/LoggerHelper.h:64
> +#define ALWAYS_LOG_IF_POSSIBLE(...)	   if (logger())
logger()->logAlways(logChannel(), __VA_ARGS__)
> +#define ERROR_LOG_IF_POSSIBLE(...)	   if (logger())
logger()->error(logChannel(), __VA_ARGS__)
> +#define WARNING_LOG_IF_POSSIBLE(...)    if (logger())
logger()->warning(logChannel(), __VA_ARGS__)
> +#define INFO_LOG_IF_POSSIBLE(...)	   if (logger())
logger()->info(logChannel(), __VA_ARGS__)
> +#define DEBUG_LOG_IF_POSSIBLE(...)	   if (logger())
logger()->debug(logChannel(), __VA_ARGS__)
> +#define WILL_LOG_IF_POSSIBLE(_level_)   if (logger())
logger()->willLog(logChannel(), _level_)

It might be a good idea to use a method with a different name, since currently
'logger()' always return a ref. Maybe loggerPtr()?

    if (loggerPtr()) loggerPtr()->log...

> Source/WebCore/Modules/encryptedmedia/MediaKeySystemAccess.cpp:66
> +    m_taskQueue.enqueueTask([this, document = makeRef(document), promise =
WTFMove(promise)] () mutable {

Do you need to keep the document alive, can't you use makeWeakPtr()?