[webkit-reviews] review denied: [Bug 134794] Migrate the usage of DEPRECATED_DEFINE_STATIC_LOCAL(T, name, args) to "static NeverDestroyed<T> name(args)" : [Attachment 234695] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Vivek Galatage <vivekg at webkit.org>'s
request for review:
Bug 134794: Migrate the usage of DEPRECATED_DEFINE_STATIC_LOCAL(T, name, args)
to "static NeverDestroyed<T> name(args)"
https://bugs.webkit.org/show_bug.cgi?id=134794

Attachment 234695: Patch
https://bugs.webkit.org/attachment.cgi?id=234695&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=234695&action=review


Great idea, but this giant patch doesn’t build. How about a sequence of smaller
patches to achieve the same thing? I’d like to carefully review each one to
spot mistakes.

For String and AtomicString, it might be better to get rid of some of these
rather than converting to NeverDestroyed; I think we’re overdoing the global
AtomicString pattern.

A good idea for as first patch is something that only deals with
clearly-correct cases such as maps and mutexes, and sticks to a manageable
smaller list. Then we can tackle the trickier cases like strings. Also need a
patch that successfully applies and compiles.

> Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.h:86
> +    static NeverDestroyed<JSTestActiveDOMObjectOwner>
jsTestActiveDOMObjectOwner;

These are generated files, so they should not be changed manually. Instead we
need to change the script and regenerate.