Bug 189636 – Expose XPCServiceMain on a WebProcess object rather than WKProcessPool

• Component: New Bugs
• Status: RESOLVED FIXED

[Alex Christensen]

Expose XPCServiceMain on a WebProcess object rather than WKProcessPool

[Alex Christensen]

Created attachment 349814 [details]
Patch

[Tim Horton]

Comment on attachment 349814 [details]
Patch

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

> Source/WebKit/ChangeLog:9
> +        Confusingly I had to un-unify the build of NetscapePluginMac.mm because it introduced weird naming conflicts
> +        because it includes Carbon.h which doesn't play nicely with other .mm files.  Specifically Rect and TextEncoding conflicted.

That means that an upstream file does 'using namespace WebCore' or something. Judging by the sorting I'd guess it's one of the ones in PluginProcess/mac. Fixing that is the right way to fix this.

[Tim Horton]

Like, say, PluginProcessMac.mm

[Alex Christensen]

Created attachment 349832 [details]
Patch

Comment on attachment 349832 [details]
Patch

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

> Source/WebKit/WebProcess/InjectedBundle/API/Cocoa/WKWebProcess.h:30
> +@interface WKWebProcess : NSObject

The class needs an availability annotation (and also something to take care of visibility, I think; WK_CLASS_AVAILABLE should take care of both.

We normally underscore-prefix private classes (but not public and internal classes) so I’d name this _WKWebProcess.

> Source/WebKit/WebProcess/InjectedBundle/API/Cocoa/WKWebProcess.h:34
> ++ (int)main;

Why this is exposed as a method is something I don’t understand. The prevailing pattern is for “main” functions to be exposed as functions. Examples include NSApplicationMain(), UIApplicationMain() and xpc_main().

[Alex Christensen]

(In reply to mitz from comment #5)
> Comment on attachment 349832 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=349832&action=review
> 
> > Source/WebKit/WebProcess/InjectedBundle/API/Cocoa/WKWebProcess.h:30
> > +@interface WKWebProcess : NSObject
> 
> The class needs an availability annotation (and also something to take care
> of visibility, I think; WK_CLASS_AVAILABLE should take care of both.

Totally forgot the availability annotations :(

> Why this is exposed as a method is something I don’t understand. The
> prevailing pattern is for “main” functions to be exposed as functions.
> Examples include NSApplicationMain(), UIApplicationMain() and xpc_main().

NSThread has a main for something similar, though not exactly the same.  Numerous internal projects use an ObjC main call like this.  I did it in ObjC to get closer to a world where we have an ObjC bundle API and because we don't have a precedent for availability annotations in C APIs in WebKit, but we do in JSC so I'll do something similar.

[Alex Christensen]

Created attachment 349852 [details]
Patch

[Alex Christensen]

Created attachment 349864 [details]
Patch

Comment on attachment 349864 [details]
Patch

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

(In reply to Alex Christensen from comment #6)
> I did it in
> ObjC to get closer to a world where we have an ObjC bundle API

We do have Objective-C bundle API (it’s the only bundle API that Safari uses on iOS), in WebProcess/InjectedBundle/API/{Cocoa,mac}. An equivalent, modern place to put this function declaration would be WKWebProcessPlugIn.h. But since this API isn’t meant to be called from bundle code, perhaps a new separate header under API/Cocoa (that imports WKFoundation.h and uses WK_API_AVAILABLE) would be even better.

> Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.h:81
> +WK_EXPORT int WKWebProcessMain(int argc, const char** argv);

Still no availability.

[Alex Christensen]

Created attachment 349887 [details]
Patch

[Alex Christensen]

Created attachment 349894 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 349894 [details]
Patch

Clearing flags on attachment: 349894

Committed r236075: <https://trac.webkit.org/changeset/236075>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44528499>