Bug 147108 - [GTK] Add API to set the maximum number of web processes per WebKitWebContext
Summary: [GTK] Add API to set the maximum number of web processes per WebKitWebContext
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2015-07-20 06:13 PDT by Carlos Garcia Campos
Modified: 2015-07-28 23:52 PDT (History)
5 users (show)

See Also:


Attachments
Patch (6.60 KB, patch)
2015-07-20 06:16 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (7.03 KB, patch)
2015-07-22 02:46 PDT, Carlos Garcia Campos
gustavo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2015-07-20 06:13:01 PDT
This is useful for limited resources systems.
Comment 1 Carlos Garcia Campos 2015-07-20 06:16:52 PDT
Created attachment 257090 [details]
Patch
Comment 2 Martin Robinson 2015-07-20 08:08:34 PDT
Comment on attachment 257090 [details]
Patch

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

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:1208
> + * This method **must be called before any other functions**,
> + * as early as possible in your application. Calling it later will make
> + * your application crash.

Woh, isn't there a way to prevent this crash at all? Can't we guard the contents of the method call somehow?
Comment 3 Michael Catanzaro 2015-07-20 08:12:39 PDT
The crash is good; it prevents programmers from screwing up and calling it too late. This isn't the only place we do that (I think for set process model, for example).
Comment 4 Martin Robinson 2015-07-20 08:24:12 PDT
(In reply to comment #3)
> The crash is good; it prevents programmers from screwing up and calling it
> too late. This isn't the only place we do that (I think for set process
> model, for example).

How about a warning or a GLib failure though?
Comment 5 Michael Catanzaro 2015-07-20 08:34:09 PDT
I guess

"This method **must be called before any other functions**, as early as possible in your application. Calling it later will make your application crash."

is not quite right; it implies you cannot use both webkit_web_context_set_process_model and this function, since the documentation is the same on both functions. Can you not use the function on a non-default WebKitWebContext? And of course you must call a function to get the WebKitWebContext in the first place.
Comment 6 Carlos Garcia Campos 2015-07-20 09:52:45 PDT
(In reply to comment #3)
> The crash is good; it prevents programmers from screwing up and calling it
> too late. This isn't the only place we do that (I think for set process
> model, for example).

Right, we (WebProcessPool actually) has some CRASH calls in setProcessModel and setMaximumNumberOfProcesses. We can prevent this by making process model and web process count limit construct-only properties, but that makes the API less convenient and doesn't allow to change them for the default context.
Comment 7 Carlos Garcia Campos 2015-07-20 09:53:05 PDT
(In reply to comment #5)
> I guess
> 
> "This method **must be called before any other functions**, as early as
> possible in your application. Calling it later will make your application
> crash."
> 
> is not quite right; it implies you cannot use both
> webkit_web_context_set_process_model and this function, since the
> documentation is the same on both functions. Can you not use the function on
> a non-default WebKitWebContext? And of course you must call a function to
> get the WebKitWebContext in the first place.

I agree the documentation is not accurate
Comment 8 Carlos Garcia Campos 2015-07-22 02:46:20 PDT
Created attachment 257257 [details]
Updated patch

Updated the docs
Comment 9 Carlos Garcia Campos 2015-07-27 06:01:14 PDT
ping
Comment 10 Gustavo Noronha (kov) 2015-07-27 07:46:23 PDT
Comment on attachment 257257 [details]
Updated patch

I am not so sure about the word 'count', but I did not manage to come up with a suggestion either, so it looks good to me.
Comment 11 Carlos Garcia Campos 2015-07-28 23:52:24 PDT
Committed r187543: <http://trac.webkit.org/changeset/187543>