Bug 213051

Summary: Some style improvements to main thread code
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, darin, ews-watchlist, koivisto, sihui_liu, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
darin: review+
Patch for landing none

Description Geoffrey Garen 2020-06-10 15:10:21 PDT
Some style improvements to main thread code
Comment 1 Geoffrey Garen 2020-06-10 15:23:56 PDT
Created attachment 401590 [details]
Patch
Comment 2 Geoffrey Garen 2020-06-10 15:32:45 PDT
Created attachment 401591 [details]
Patch
Comment 3 Darin Adler 2020-06-10 16:12:46 PDT
Comment on attachment 401591 [details]
Patch

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

> Source/WTF/wtf/cocoa/MainThreadCocoa.mm:61
> +static pthread_t s_webThreadPthread { nullptr };

The "{ nullptr }" has no effect or value here.

> Source/WTF/wtf/cocoa/MainThreadCocoa.mm:106
> +    if (RunLoop::webIfExists()) {
>          RunLoop::web().dispatch(dispatchFunctionsFromMainThread);
>          return;
>      }

Why get it twice? Put it in a local variable and use it?

> Source/WebKit/Shared/WebKit2Initialize.cpp:43
>      JSC::initializeThreading();
> -    RunLoop::initializeMainRunLoop();
> +    RunLoop::initializeMain();

Is there any reason that these are two separate function calls that both need to be called in each location? If JSC::initializeThreading called RunLoop::initializeMain, what would go wrong?
Comment 4 Geoffrey Garen 2020-06-10 16:21:48 PDT
> > Source/WebKit/Shared/WebKit2Initialize.cpp:43
> >      JSC::initializeThreading();
> > -    RunLoop::initializeMainRunLoop();
> > +    RunLoop::initializeMain();
> 
> Is there any reason that these are two separate function calls that both
> need to be called in each location? If JSC::initializeThreading called
> RunLoop::initializeMain, what would go wrong?

JSC::initializeThreading can be called on any thread. RunLoop::initializeMain must be called on the main thread.

But maybe we could add a JSC::initializeMain() convenience function to do both from the main thread.
Comment 5 Geoffrey Garen 2020-06-10 16:43:21 PDT
Created attachment 401603 [details]
Patch for landing
Comment 6 EWS 2020-06-10 17:15:11 PDT
Committed r262878: <https://trac.webkit.org/changeset/262878>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 401603 [details].
Comment 7 Radar WebKit Bug Importer 2020-06-10 17:16:22 PDT
<rdar://problem/64231824>