Bug 151473 – [GTK] Reimplement webkit_web_context_[get,set]_process_model

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKitGTK

[Michael Catanzaro]

Reimplement webkit_web_context_[get,set]_process_model() to keep the API working as before. We can implement WEBKIT_PROCESS_MODEL_SHARED_SECONDARY_PROCESS by setting a process limit of 1.

[Michael Catanzaro]

This applies on top of the patch in bug #151418.

[Michael Catanzaro]

Created attachment 265935 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 265935 [details]
Patch

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

The original patch has been rolled out, maybe we can land this as part of the network process option removal instead of a follow up patch.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:1149
> +        if (context->priv->context->maximumNumberOfProcesses() != 1)
> +            context->priv->context->setMaximumNumberOfProcesses(1);

Don't need to check the value didn't change, this simply sets the value, there's no ipc traffic or any other thing to avoid here.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:1153
> +        if (context->priv->context->maximumNumberOfProcesses() == 1)
> +            context->priv->context->setMaximumNumberOfProcesses(0);

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:1156
> +    default:
> +        ASSERT_NOT_REACHED();

Just remove the default, since there aren't more values in the enum. That way we will get a compile warning/error if we add a new value and it's not handled here.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:1174
> +    return context->priv->context->maximumNumberOfProcesses() == 1 ?
> +        WEBKIT_PROCESS_MODEL_SHARED_SECONDARY_PROCESS : WEBKIT_PROCESS_MODEL_MULTIPLE_SECONDARY_PROCESSES;

This is not accurate. You can do set_process_model (MULTIPLE_SECONDARY_PROCESSES); set set_web_process_count_limit(1) and then get_process_model will return SHARED_SECONDARY_PROCESS. What we set with the setter is what we should return in the getter, the fact that now it's implemented changing the limit should be an implementation detail not exposed to the users.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:1186
> - * The default value is 0 and means no limit.
> + * The default value is 1. If you change the process model to
> + * %WEBKIT_PROCESS_MODEL_MULTIPLE_SECONDARY_PROCESSES, then the process count
> + * limit will be removed. Setting the process count limit to a value other than
> + * 1 automatically changes the process model to %WEBKIT_PROCESS_MODEL_MULTIPLE_SECONDARY_PROCESSES.

Don't change this either. We should just keep the current model cached in the instance and return that in the getter, and here we could just return early if the process model is SHARED_SECONDARY_PROCESS and in the getter we always return 0 when process model is SHARED_SECONDARY_PROCESS even if it's actually 1 internally.

[Michael Catanzaro]

(In reply to comment #3)
> Don't change this either. We should just keep the current model cached in
> the instance and return that in the getter, and here we could just return
> early if the process model is SHARED_SECONDARY_PROCESS and in the getter we
> always return 0 when process model is SHARED_SECONDARY_PROCESS even if it's
> actually 1 internally.

OK, if that's what you prefer. You did that in bug #151418, so we don't need this bug anymore.