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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 18 14:17:39 PST 2022

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

Attachment 449390: Patch


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

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.

More information about the webkit-reviews mailing list