[webkit-reviews] review granted: [Bug 232693] Ignore BroadcastChannel::postMessage from detached iframe / closing worker contexts : [Attachment 443635] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 232693: Ignore BroadcastChannel::postMessage from detached iframe / closing
worker contexts
https://bugs.webkit.org/show_bug.cgi?id=232693

Attachment 443635: Patch

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




--- Comment #5 from Darin Adler <darin at apple.com> ---
Comment on attachment 443635
  --> https://bugs.webkit.org/attachment.cgi?id=443635
Patch

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

> Source/WebCore/dom/BroadcastChannel.cpp:187
> +    if (!isEligibleForMessaging())
> +	   return { };
> +
>      if (m_isClosed)
>	   return Exception { InvalidStateError, "This BroadcastChannel is
closed" };

So while closing we silently do nothing, and once closed we throw an exception.
To me this seems like a peculiar design.

> Source/WebCore/dom/BroadcastChannel.cpp:235
> +    if (!isEligibleForMessaging())
> +	   return;
> +
>      if (m_isClosed)
>	   return;

If it wasn’t for the unusual case above, I would suggest we merge the m_closed
check into isEligibleForMessaging.