Bug 130133 - [EFL][WK2] Add an API to set process model
Summary: [EFL][WK2] Add an API to set process model
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michal Pakula vel Rutka
URL:
Keywords:
Depends on:
Blocks: 110141 130190
  Show dependency treegraph
 
Reported: 2014-03-12 09:00 PDT by Michal Pakula vel Rutka
Modified: 2014-03-25 02:54 PDT (History)
7 users (show)

See Also:


Attachments
proposed patch (7.20 KB, patch)
2014-03-12 09:07 PDT, Michal Pakula vel Rutka
no flags Details | Formatted Diff | Diff
updated documentation (7.23 KB, patch)
2014-03-24 08:10 PDT, Michal Pakula vel Rutka
gyuyoung.kim: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michal Pakula vel Rutka 2014-03-12 09:00:16 PDT
Add an API which may be used to enable spawning multiple web processes in WebKit2. Also it enables network process.
Comment 1 Michal Pakula vel Rutka 2014-03-12 09:07:03 PDT
Created attachment 226513 [details]
proposed patch
Comment 2 Jinwoo Song 2014-03-13 20:53:26 PDT
Comment on attachment 226513 [details]
proposed patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:79
> + * @brief   Contains option for process model

It would be better to add more description for each process model.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context.cpp:117
> +    ASSERT_EQ(EWK_PROCESS_MODEL_SHARED_SECONDARY, ewk_context_process_model_get(context));

Could you add more test for 'EWK_PROCESS_MODEL_MULTIPLE_SECONDARY' model?
Also, is there any way to check if the network process is forked successfully?
Comment 3 Maciej Florek 2014-03-14 09:17:45 PDT
(In reply to comment #2)
> (From update of attachment 226513 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226513&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_context.h:79
> > + * @brief   Contains option for process model
> 
> It would be better to add more description for each process model.

The descriptions of these two options will be extended.

> > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context.cpp:117
> > +    ASSERT_EQ(EWK_PROCESS_MODEL_SHARED_SECONDARY, ewk_context_process_model_get(context));
> 
> Could you add more test for 'EWK_PROCESS_MODEL_MULTIPLE_SECONDARY' model?

We are working on the tests of this API, so they will be added in the beginning of next week.

> Also, is there any way to check if the network process is forked successfully?

Yes,it is possible to check this.
Comment 4 Michal Pakula vel Rutka 2014-03-24 07:56:01 PDT
(In reply to comment #2)
> (From update of attachment 226513 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226513&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_context.h:79
> > + * @brief   Contains option for process model
> 
> It would be better to add more description for each process model.
> 
> > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context.cpp:117
> > +    ASSERT_EQ(EWK_PROCESS_MODEL_SHARED_SECONDARY, ewk_context_process_model_get(context));
> 
> Could you add more test for 'EWK_PROCESS_MODEL_MULTIPLE_SECONDARY' model?
> Also, is there any way to check if the network process is forked successfully?

It is possible but it needs a separate test as process model can be changed only before first WebProcess is spawned.
Comment 5 Michal Pakula vel Rutka 2014-03-24 08:10:00 PDT
Created attachment 227643 [details]
updated documentation
Comment 6 Gyuyoung Kim 2014-03-24 16:35:16 PDT
Comment on attachment 227643 [details]
updated documentation

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

I'm happy now we can select web process model. LGTM except for my trivial comments. Please fix them before landing.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:384
> + * web process. When EWK_PROCESS_MODEL_MULTIPLY_SECONDARY is set a

Typo: EWK_PROCESS_MODEL_MULTIPLY_SECONDARY -> EWK_PROCESS_MODEL_MULTIPLE_SECONDARY.

I wonder whether current our process model only supports network process. Won't we support database process ?
Comment 7 Gyuyoung Kim 2014-03-24 16:36:33 PDT
Comment on attachment 227643 [details]
updated documentation

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

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:388
> + * @param context context object to set process model.

One more thing. We haven't used "." at the end of @param and @return lines.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:389
> + * @param process_model a #Ewk_Process_Model.

ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:396
> + * @param context context object to query.

ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:398
> + * @return the process model for the @a context.

ditto.
Comment 8 Michal Pakula vel Rutka 2014-03-25 02:30:58 PDT
(In reply to comment #6)
> (From update of attachment 227643 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=227643&action=review
> 
> I'm happy now we can select web process model. LGTM except for my trivial comments. Please fix them before landing.
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_context.h:384
> > + * web process. When EWK_PROCESS_MODEL_MULTIPLY_SECONDARY is set a
> 
> Typo: EWK_PROCESS_MODEL_MULTIPLY_SECONDARY -> EWK_PROCESS_MODEL_MULTIPLE_SECONDARY.
> 
> I wonder whether current our process model only supports network process. Won't we support database process ?

Currently it is only implemented for Mac platform, we need to add missing code and of course enable DATABASE_PROCESS flag.
Comment 9 Michal Pakula vel Rutka 2014-03-25 02:54:08 PDT
Committed r166228: <http://trac.webkit.org/changeset/166228>