[webkit-reviews] review granted: [Bug 235107] Improve computation of service worker FetchEvent.resultingClientId : [Attachment 449390] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 235107: Improve computation of service worker FetchEvent.resultingClientId
https://bugs.webkit.org/show_bug.cgi?id=235107

Attachment 449390: Patch

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




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

The code should use ScriptExecutionContextIdentifier, not const
std::optional<ScriptExecutionIdentifier>&. These identifiers are not
particularly huge structures and don’t need to be passed by reference. And
also, there is already a null value for identifiers, so we don’t need to wrap
them in std::optional just to have a null meaning nothing is specified. We can
create the null value with the default constructor, and detect whether it’s
null using the operator bool.

I’m not sure how we decided in which cases to have a default value and in which
cases callers have to explicitly pass { }. Since specifying a pre-determined
identifier is fairly unusual, we should consider adding a default everywhere,
and these calls to HTMLDocument::create would not have to change.