Disable the shared process mode

[joone]

The shared process mode makes debugging very hard so
we need to disable it at runtime.

Bug: XWALK-2987

[crosswalk-trybot]

Testing patch series with joone/crosswalk@0e14605 as its head.

Bot	Status
Crosswalk Android-X86	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/2503)
Crosswalk Linux	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/2503)
Crosswalk Tizen IVI	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Tizen IVI/builds/2496)
Crosswalk Tizen 3 Common	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Tizen 3 Common/builds/2093)

[joone]

This patch works fine with opening a URL, but it crashes when launching a Modello application.
Any idea?

[halton]

• code LGTM
• Commit log: Bug: XWALK-2987 -> BUG=XWALK-2987
• And questions for you:

The shared process mode makes debugging very hard so we need to disable it at runtime.

Any other reasons other than debugging? The process model might affect security work. CC @xuzhang

[rakuco]

In addition to what Halton said, this patch also needs a better commit and pull request message:

• The first line needs to indicate this is a Tizen-related commit, so it should have the "[Tizen]" prefix.
• This commit is not disabling shared process, but rather making it conditionally activated based on a command line option instead of an ifdef. You need to make this clear.

Additionally, if you are removing the ifdef, you can also remove the processing of the shared_process_mode variable in gyp (and consequently its setting in the spec file).

[rakuco]

+@pozdnyakov

[rakuco]

And remember to update the pull request message in addition to the commit message.

[pozdnyakov]

1. pls. remove compile time flags, as @rakuco proposed
2. Apparently your patch affects Crosswalk Desktop code path, which is not acceptable.
3. Instead of checking cmd line every where please add bool XWalkRunner::shared_process_mode_enabled() { return shared_process_mode_enabled_; }, shared_process_mode_enabled_ should be 'false' for every platform other than Tizen, for Tizen it should be initialized, based on cmd line.
4. We should somehow make sure that the new cmd flag is only for development purpose, and it should not be available on "product" version. @rakuco @halton any ideas how to achieve it?

[halton]

@tmpsantos no for me, thanks for the summary.

[tmpsantos]

4. We should somehow make sure that the new cmd flag is only for development purpose, and it should not be available on "product" version. @rakuco @halton any ideas how to achieve it?

We might need a #if !defined(NDEBUG) here.

[rakuco]

4. We should somehow make sure that the new cmd flag is only for development purpose, and it should not be available on "product" version. @rakuco @halton any ideas how to achieve it?

Shouldn't it? I was considering it something like Chromium's "--single-process": I can pass it to my Debian's chromium binary, but there's a warning saying this is an unsupported configuration and I'm on my own if something crashes. We don't need to add a warning, but I thought it would be harmless to leave the option there in all builds.

[tmpsantos]

Shouldn't it? I was considering it something like Chromium's "--single-process": I can pass it to my Debian's chromium binary, but there's a warning saying this is an unsupported configuration and I'm on my own if something crashes. We don't need to add a warning, but I thought it would be harmless to leave the option there in all builds.

Sounds like a better idea to me.

[pozdnyakov]

Shouldn't it? I was considering it something like Chromium's "--single-process": I can pass it to my Debian's chromium binary, but there's a warning saying this is an unsupported configuration and I'm on my own if something crashes. We don't need to add a warning, but I thought it would be harmless to leave the option there in all builds.

If Tizen platform/security guys are OK with it I'm OK with it too. Btw @rakuco should we place xwalk also to /usr/bin in this case?

[rakuco]

Btw @rakuco should we place xwalk also to /usr/bin in this case?

That's a decision I'd rather leave to the Tizen people. Personally, if this is only going to be used for debugging, I'd be fine with leaving the binary only in %{_libdir}/xwalk/lib like we do today (I'm assuming calling /usr/lib{64}/xwalk/lib/xwalk --single-process would work without one having to set any additional environment variables).

[joone]

@rakuco @halton @pozdnyakov @tmpsantos Thanks for the review.
I've updated this patch:
#2663

[rakuco]

It should be very clear by now that you are not supposed to create a new pull request for this. You should have updated the existing one.