Bug 189636 - Expose XPCServiceMain on a WebProcess object rather than WKProcessPool
Summary: Expose XPCServiceMain on a WebProcess object rather than WKProcessPool
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-09-14 15:03 PDT by Alex Christensen
Modified: 2018-09-17 11:46 PDT (History)
5 users (show)

See Also:


Attachments
Patch (12.23 KB, patch)
2018-09-14 15:13 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (15.94 KB, patch)
2018-09-14 17:14 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (11.31 KB, patch)
2018-09-14 22:46 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (11.26 KB, patch)
2018-09-15 20:40 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (24.15 KB, patch)
2018-09-17 09:01 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (22.74 KB, patch)
2018-09-17 09:47 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2018-09-14 15:03:44 PDT
Expose XPCServiceMain on a WebProcess object rather than WKProcessPool
Comment 1 Alex Christensen 2018-09-14 15:13:46 PDT
Created attachment 349814 [details]
Patch
Comment 2 Tim Horton 2018-09-14 15:53:35 PDT
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.
Comment 3 Tim Horton 2018-09-14 15:54:02 PDT
Like, say, PluginProcessMac.mm
Comment 4 Alex Christensen 2018-09-14 17:14:20 PDT
Created attachment 349832 [details]
Patch
Comment 5 mitz 2018-09-14 17:32:05 PDT
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().
Comment 6 Alex Christensen 2018-09-14 22:43:44 PDT
(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.
Comment 7 Alex Christensen 2018-09-14 22:46:06 PDT
Created attachment 349852 [details]
Patch
Comment 8 Alex Christensen 2018-09-15 20:40:03 PDT
Created attachment 349864 [details]
Patch
Comment 9 mitz 2018-09-15 21:02:46 PDT
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.
Comment 10 Alex Christensen 2018-09-17 09:01:53 PDT
Created attachment 349887 [details]
Patch
Comment 11 Alex Christensen 2018-09-17 09:47:28 PDT
Created attachment 349894 [details]
Patch
Comment 12 WebKit Commit Bot 2018-09-17 11:45:54 PDT
Comment on attachment 349894 [details]
Patch

Clearing flags on attachment: 349894

Committed r236075: <https://trac.webkit.org/changeset/236075>
Comment 13 WebKit Commit Bot 2018-09-17 11:45:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Radar WebKit Bug Importer 2018-09-17 11:46:54 PDT
<rdar://problem/44528499>