[webkit-reviews] review denied: [Bug 203389] Unused arguments in MESSAGE_CHECK_CONTEXTID() macros : [Attachment 381907] Patch v2

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 203389: Unused arguments in MESSAGE_CHECK_CONTEXTID() macros
https://bugs.webkit.org/show_bug.cgi?id=203389

Attachment 381907: Patch v2

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




--- Comment #7 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 381907
  --> https://bugs.webkit.org/attachment.cgi?id=381907
Patch v2

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

> Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.mm:36
> +#define MESSAGE_CHECK_CONTEXTID(identifier)
MESSAGE_CHECK_BASE(m_contextMap.isValidKey(identifier),
m_page->process().connection())

Although you correct the macro here, it actually creates compile errors because
of mistakes later in the code:

For example:
void PlaybackSessionManagerProxy::rateChanged(uint64_t contextId, bool
isPlaying, double rate)
{
    MESSAGE_CHECK_CONTEXTID(contextID);
    ensureModel(contextId).rateChanged(isPlaying, rate);
}

Here, the argument 'contextID' used to be macro-expanded to this:

void PlaybackSessionManagerProxy::rateChanged(uint64_t contextId, bool
isPlaying, double rate)
{
    MESSAGE_CHECK_BASE(m_contextMap.isValidKey(contextId),
m_page->process().connection())
    ensureModel(contextId).rateChanged(isPlaying, rate);
}

... and the 'contextId' from the macro expansion matched the 'contextId' label
in the parameter.

We need to fix the call-sites, too, so that the parameter of
MESSAGE_CHECK_CONTEXTID matches an actual variable name at the call site.