[webkit-reviews] review granted: [Bug 233289] Add initial implementation for the Web Lock API : [Attachment 445277] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 233289: Add initial implementation for the Web Lock API
https://bugs.webkit.org/show_bug.cgi?id=233289

Attachment 445277: Patch

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




--- Comment #23 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 445277
  --> https://bugs.webkit.org/attachment.cgi?id=445277
Patch

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

r=me

> Source/WTF/wtf/Deque.h:576
> +inline size_t Deque<T, inlineCapacity>::removeAllMatching(const Func& func)
>  {
> +    size_t removedCount = 0;
>      size_t count = size();
>      while (count--) {
>	   T value = takeFirst();
>	   if (!func(value))
>	       append(WTFMove(value));
> +	   else
> +	       ++removedCount;
>      }
> +    return removedCount;
>  }

I wonder if we should just return oldSize - size() here. Simplifies the loop a
bit.

> Source/WebCore/Modules/web-locks/WebLockRegistry.cpp:80
> +    bool shouldPrepend = false;

Might be clearer as "bool shouldPrepend = steal;", rather than using a branch
below.