[Webkit-unassigned] [Bug 125463] [GTK] Add API to allow setting the process model in WebKitWebContext

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=125463





--- Comment #17 from Carlos Garcia Campos <cgarcia at igalia.com>  2014-01-26 08:33:59 PST ---
(In reply to comment #15)
> (In reply to comment #14)
> >
> > > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:931
> > > +    switch (processModel) {
> > > +    case WEBKIT_PROCESS_MODEL_SHARED_SECONDARY_PROCESS:
> > > +        newProcessModel = ProcessModelSharedSecondaryProcess;
> > > +        break;
> > > +    case WEBKIT_PROCESS_MODEL_MULTIPLE_SECONDARY_PROCESSES:
> > > +        newProcessModel = ProcessModelMultipleSecondaryProcesses;
> > > +        break;
> > > +    default:
> > > +        g_assert_not_reached();
> > > +    }
> > 
> > I think we can use the same values and add a compile assert macro to make sure they match. That way we don't need the switches.
> 
> I would rather keep the switches, read on below…
> 
> > > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.h:84
> > > +typedef enum {
> > > +    WEBKIT_PROCESS_MODEL_SHARED_SECONDARY_PROCESS,
> > > +    WEBKIT_PROCESS_MODEL_MULTIPLE_SECONDARY_PROCESSES
> > > +} WebKitProcessModel;
> > 
> > I have some doubts about the names. This is actually about web processes, so maybe it should be called WebKitWebProcessModel, but it's true that all other processes are transparent from the API point of view, so it's probably fine to use WebKitProcessModel. MULTIPLE_SECONDARY_PROCESSES is too generic, I'm thinking of a possible future model of one process per security domain or something like that. So we might call the current one WEBKIT_PROCESS_MODEL_SECONDARY_PROCESS_PER_WEB_VIEW or something like that. What do you think?
> 
> Right, …_SECONDARY_PROCESS_PER_WEB_VIEW better describes what the
> mode implies, so I also think it is a good idea to go with that name.
> Provided that the names of process models exposed in the API are not
> going to match the names used internally, and that the mapping is not
> exactly 1:1 (example: SECONDARY_PROCESS_PER_WEB_VIEW also enables the
> network process), I think that keeping the switches better represents
> that — even when at the moment the mapping quite direct right now.

That's why I suggested to add a compile assert, I don't think it's that important that the name matches exactly as long as it's obvious it's the same thing. The mapping is indeed 1:1, the network process is not a process model. WebKit internally expects the network process to be enabled when multiple web process model is set, there are several asserts to ensure that, see WebContext::networkingProcessConnection() for example.

> Apart that for the moment I am keeping the switches, the rest of the
> issues should be solved in the new version of the patch I am uploading
> in a moment.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.