Summary: | Add initial implementation for the Web Lock API | ||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||||||||||||||||||||||||||||||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||||||||||||||||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||||||||||||||
Severity: | Normal | CC: | achristensen, annulen, benjamin, cmarcelo, darin, esprehn+autocc, ews-watchlist, ggaren, gyuyoung.kim, kangil.han, kondapallykalyan, mark.lam, ryuan.choi, sergio, webkit-bug-importer, youennf, ysuzuki | ||||||||||||||||||||||||||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||||||||||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=233655 | ||||||||||||||||||||||||||||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||||||||||||||||||||||||||||
Bug Blocks: | 232436, 233719 | ||||||||||||||||||||||||||||||||||||||||||||||||
Attachments: |
|
Description
Chris Dumez
2021-11-17 15:58:33 PST
Created attachment 444611 [details]
WIP Patch
Created attachment 444686 [details]
WIP Patch
Created attachment 444702 [details]
WIP Patch
Created attachment 444703 [details]
WIP Patch
Created attachment 444705 [details]
WIP Patch
Created attachment 444755 [details]
WIP Patch
Created attachment 444757 [details]
WIP Patch
Created attachment 444759 [details]
WIP Patch
Created attachment 444763 [details]
WIP Patch
Created attachment 444764 [details]
WIP Patch
Created attachment 444772 [details]
WIP Patch
Created attachment 444819 [details]
WIP Patch
Created attachment 444823 [details]
WIP Patch
Created attachment 444824 [details]
WIP Patch
Created attachment 444830 [details]
Patch
Created attachment 444847 [details]
Patch
Created attachment 444869 [details]
Patch
Created attachment 444870 [details]
Patch
Created attachment 444878 [details]
Patch
Created attachment 444883 [details]
Patch
Created attachment 445277 [details]
Patch
Comment on attachment 445277 [details] 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. Created attachment 445364 [details]
Patch
(In reply to Geoffrey Garen from comment #23) > Comment on attachment 445277 [details] > 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. Good ideas, thanks. I made those changes. Committed r286284 (244643@main): <https://commits.webkit.org/244643@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 445364 [details]. Comment on attachment 445364 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445364&action=review > Source/WebCore/bindings/js/JSDOMConvertPromise.h:54 > + auto& scriptController = *downcast<WorkerGlobalScope>(*scriptExecutionContext).script(); Just noticed that this is a function returning a pointer, but we are unconditionally dereferencing. Can we change this to return a reference? Or should we add a null check? Or a comment explaining why. Not super-important, but mysteriously missing null checks are not my favorite. (In reply to Darin Adler from comment #27) > Comment on attachment 445364 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=445364&action=review > > > Source/WebCore/bindings/js/JSDOMConvertPromise.h:54 > > + auto& scriptController = *downcast<WorkerGlobalScope>(*scriptExecutionContext).script(); > > Just noticed that this is a function returning a pointer, but we are > unconditionally dereferencing. Can we change this to return a reference? Or > should we add a null check? Or a comment explaining why. > > Not super-important, but mysteriously missing null checks are not my > favorite. Will follow-up in https://bugs.webkit.org/show_bug.cgi?id=233655 to add a null check since the script controller can in theory be null. |