Summary: | Drop some unnecessary RunLoop::isMain() / IsMainRunLoop() checks | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, benjamin, cmarcelo, darin, eric.carlson, ews-watchlist, ggaren, glenn, jer.noble, philipj, sam, sergio, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=223105 | ||||||||||
Attachments: |
|
Description
Chris Dumez
2021-03-11 14:55:17 PST
Created attachment 422975 [details]
Patch
Found some more. Will update the patch. Created attachment 422978 [details]
Patch
Comment on attachment 422978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422978&action=review > Source/WebKit/Platform/IPC/Connection.cpp:933 > + ensureOnMainRunLoop([this, protectedThis = makeRef(*this), messageName]() mutable { I slightly prefer the "only capture protectedThis" style, but I guess you like the "capture both" style? > Source/WebKit/Shared/Plugins/NPObjectProxy.cpp:307 > + NPObjectProxy* npObjectProxy = toNPObjectProxy(npObject); > + delete npObjectProxy; Better as a one-liner: delete toNPObjectProxy(npObject); > Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.cpp:225 > + ensureOnMainRunLoop([this, protectedThis = makeRef(*this)] { Maybe we should just capture the three identifiers and not capture the object at all? Would be nice to not have to worry about reference counting at all. > Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.cpp:226 > + if (auto* serviceWorkerThreadProxy = SWContextManager::singleton().serviceWorkerThreadProxy(m_serviceWorkerIdentifier)) Just call the local variable "proxy"? Comment on attachment 422978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422978&action=review >> Source/WebKit/Platform/IPC/Connection.cpp:933 >> + ensureOnMainRunLoop([this, protectedThis = makeRef(*this), messageName]() mutable { > > I slightly prefer the "only capture protectedThis" style, but I guess you like the "capture both" style? Yes, I like capturing this because it makes the code more concise in the lambda. >> Source/WebKit/Shared/Plugins/NPObjectProxy.cpp:307 >> + delete npObjectProxy; > > Better as a one-liner: > > delete toNPObjectProxy(npObject); OK >> Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.cpp:225 >> + ensureOnMainRunLoop([this, protectedThis = makeRef(*this)] { > > Maybe we should just capture the three identifiers and not capture the object at all? Would be nice to not have to worry about reference counting at all. Good idea, will do. >> Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.cpp:226 >> + if (auto* serviceWorkerThreadProxy = SWContextManager::singleton().serviceWorkerThreadProxy(m_serviceWorkerIdentifier)) > > Just call the local variable "proxy"? Ok. Created attachment 422984 [details]
Patch
Committed r274322: <https://commits.webkit.org/r274322> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422984 [details]. |