Bug 188367 - Add SPI for launching WebContent process with pre-linked injected bundle
Summary: Add SPI for launching WebContent process with pre-linked injected bundle
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ben Richards
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-06 17:14 PDT by Ben Richards
Modified: 2018-08-07 14:05 PDT (History)
7 users (show)

See Also:


Attachments
Patch (16.05 KB, patch)
2018-08-06 20:14 PDT, Ben Richards
no flags Details | Formatted Diff | Diff
Patch (17.08 KB, patch)
2018-08-06 21:31 PDT, Ben Richards
no flags Details | Formatted Diff | Diff
Patch for landing (17.11 KB, patch)
2018-08-07 11:11 PDT, Ben Richards
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Richards 2018-08-06 17:14:26 PDT
Adding this SPI will allow other applications to create "stub" WebContent processes that are hard linked to an injected bundle. This makes the dlopen call in InjectedBundle::initialize free.
Comment 1 Ben Richards 2018-08-06 20:14:50 PDT
Created attachment 346679 [details]
Patch
Comment 2 Ryosuke Niwa 2018-08-06 20:31:01 PDT
Comment on attachment 346679 [details]
Patch

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

> Source/WebKit/UIProcess/API/APIProcessPoolConfiguration.h:154
> +    const WTF::String& optimizedWebProcessName() const { return m_optimizedWebProcessName; }
> +    void setOptimizedWebProcessName(const WTF::String& optimizedWebProcessName) { m_optimizedWebProcessName = optimizedWebProcessName; }

I think calling this "optimized web process name" is confusing because we're not changing the process name per se.
What we're modifying is the XPC service we're launching.
As we discussed in person, setAlternateWebContentServiceBundleIdentifier / setCustomWebContentServiceBundleIdentifier sounds better to me.

> Source/WebKit/UIProcess/WebProcessProxy.cpp:166
> +    if (!m_processPool->optimizedWebProcessName().isNull())
> +        launchOptions.extraInitializationData.add("OptimizedWebProcessName", m_processPool->optimizedWebProcessName());

It's a bit weird to put this on extraInitializationData since that's what gets sent to web content process as a parameter.
Why don't we just add a new field to launchOptions as we discussed in person?
Comment 3 Ben Richards 2018-08-06 21:31:25 PDT
Created attachment 346683 [details]
Patch
Comment 4 Alex Christensen 2018-08-07 10:11:56 PDT
Comment on attachment 346683 [details]
Patch

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

> Source/WebKit/Shared/EntryPointUtilities/mac/XPCService/XPCServiceMain.mm:163
> +using namespace WebKit;

This seems unnecessary.  Just use WebKit:: in one place.

> Source/WebKit/UIProcess/WebProcessProxy.cpp:166
> +        launchOptions.customWebContentServiceBundleIdentifier = m_processPool->customWebContentServiceBundleIdentifier().ascii();

https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html#//apple_ref/doc/uid/20001431-102070 says the bundle identifier must be ascii.  Could we use utf8 just in case, though?  There's no reason not to.
Or maybe we could just add a "contains only ascii" check to our "Guard against API misuse" check above.
Comment 5 Ben Richards 2018-08-07 11:11:26 PDT
Created attachment 346716 [details]
Patch for landing
Comment 6 Ben Richards 2018-08-07 11:12:13 PDT
Let's let EWS run before cq+
Comment 7 WebKit Commit Bot 2018-08-07 14:04:49 PDT
Comment on attachment 346716 [details]
Patch for landing

Clearing flags on attachment: 346716

Committed r234668: <https://trac.webkit.org/changeset/234668>
Comment 8 WebKit Commit Bot 2018-08-07 14:04:51 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-08-07 14:05:56 PDT
<rdar://problem/43019446>