Bug 223094 – Drop some unnecessary RunLoop::isMain() / IsMainRunLoop() checks

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Drop some unnecessary RunLoop::isMain() / IsMainRunLoop() checks. callOnMainThread() / ensureOnMainRunLoop() take care of calling their lambda synchronously when already on the main runloop.

[Chris Dumez]

Created attachment 422975 [details]
Patch

[Chris Dumez]

Found some more. Will update the patch.

[Chris Dumez]

Created attachment 422978 [details]
Patch

[Darin Adler]

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"?

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 422984 [details]
Patch

[EWS]

Committed r274322: <https://commits.webkit.org/r274322>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422984 [details].

[Radar WebKit Bug Importer]

<rdar://problem/75345742>