Bug 92676 – [WK2] Kill the concept of secondary shared process

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Alexey Proskuryakov]

A client can build a singleton just as easily on top of WKCreateContext. In fact, both clients that use WKContextGetSharedProcessContext already wrap it in their own singleton!

[Alexey Proskuryakov]

Created attachment 155358 [details]
proposed patch

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/124092>.

[Rafael Brandao]

Reopening to attach new patch.

[Rafael Brandao]

Created attachment 155393 [details]
Patch

This patch broke qt build. Could you double check if this is the proper fix?

[Alexey Proskuryakov]

Comment on attachment 155393 [details]
Patch

Yes. Apologies for breaking the build - I grepped for API usage, but didn't realize that Qt used WK2 internals directly.

[Rafael Brandao]

This patch was committed in <http://trac.webkit.org/changeset/124114>, yet that link gives me a "No changeset 124114 in the repository". :-(

[Ryosuke Niwa]

Windows build fix landed in http://trac.webkit.org/changeset/124153.

[Alexey Proskuryakov]

The Windows fix is not correct - WKContextGetSharedProcessContext should be replaced with WKContextCreate. Sorry, I cannot land an updated one right now.

WKContextGetSharedThreadContext may or may not work at all, but it's definitely not the regular mode for running MiniBrowser.

[Ryosuke Niwa]

(In reply to comment #8)
> The Windows fix is not correct - WKContextGetSharedProcessContext should be replaced with WKContextCreate. Sorry, I cannot land an updated one right now.

Okay, fixed in http://trac.webkit.org/changeset/124165.

[Alexey Proskuryakov]

I looked at this function more carefully, and while it's correct that WKContextGetSharedProcessContext should be replaced with WKContextCreate, it's not quite that easy. We need to return a singleton object, so the result of WKContextCreate should be put into a static.

Creating a new context for every view will likely work at the first glance, but wouldn't be the normal mode either.

My apologies for an incomplete advice.

[Ryosuke Niwa]

(In reply to comment #10)
> I looked at this function more carefully, and while it's correct that WKContextGetSharedProcessContext should be replaced with WKContextCreate, it's not quite that easy. We need to return a singleton object, so the result of WKContextCreate should be put into a static.
> 
> Creating a new context for every view will likely work at the first glance, but wouldn't be the normal mode either.

Okay. Would you be able to fix sometime soon or should we roll patches out?

[Alexey Proskuryakov]

Since the build is not broken any more, it seems fine to leave as is for a little while.

[Alexey Proskuryakov]

Re-opening to re-fix Windows MiniBrowser.

[Alexey Proskuryakov]

Created attachment 155435 [details]
MiniBrowser fix

[Rafael Brandao]

Comment on attachment 155435 [details]
MiniBrowser fix

I think we can apply the same logic for Qt's previous usage of WebContext::sharedProcessContext. I'm wondering if it would be a good idea to modify this API and use something like WebContext::instance() instead like we usually do for singletons.

[WebKit Review Bot]

Comment on attachment 155435 [details]
MiniBrowser fix

Clearing flags on attachment: 155435

Committed r124220: <http://trac.webkit.org/changeset/124220>