Setting command line switches programatically

[GoogleCodeExporter]

It would be nice for the cefpython.Initialize() function to accept
an another parameter named "commandLineSwitches" which would be 
a dict allowing to set any of the Chromium command line switches:

http://peter.sh/experiments/chromium-command-line-switches/

When switches are set programatically they must be set in in the
main process and in the child processes:
 * CefApp::OnBeforeCommandLineProcessing()
 * CefBrowserProcessHandler::OnBeforeChildProcessLaunch()

Original issue reported on code.google.com by czarek.t...@gmail.com on 13 Jun 2013 at 7:24

[GoogleCodeExporter]

Switches should be set using the command_line->AppendSwitchWithValue() method.

Original comment by czarek.t...@gmail.com on 27 Sep 2013 at 9:00

[GoogleCodeExporter]

For example to set a proxy server:

    command_line->AppendSwitchWithValue("proxy-server", "socks5://127.0.0.1:8888");

It is enough to set the proxy switch only for the browser process in 
CefApp::OnBeforeCommandLineProcessing().

Original comment by czarek.t...@gmail.com on 27 Sep 2013 at 9:07

[GoogleCodeExporter]

Given that not all switches apply in both cases wouldn't it make sense for 
CefApp switches to be specified on cefpython.Initialize but 
CefBrowserProcessHandler switches be specified in CreateBrowserSync? Or is 
there no harm in setting all switches everywhere?

Original comment by finna...@gmail.com on 16 Dec 2013 at 10:22

[GoogleCodeExporter]

You might want to set different switches for different browsers. So, switches 
should be passed to both Initialize() and CreateBrowserSync().

Original comment by czarek.t...@gmail.com on 17 Dec 2013 at 9:18

[GoogleCodeExporter]

I can't see how to get a reference to the appropriate browser in either 
callback so I've just implemented global switches through 
OnBeforeCommandLineProcessing. I had a look at OnRenderThreadCreated as 
suggested but can't see how that helps me get a reference to the browser?

I had to patch client_handler.cpp to avoid the following build error
"client_handler.cpp:125:33: error: passing NULL to non-pointer argument 5"
Just changing the NULL to 0 on that line was sufficient, this change isn't 
included in my patch as I suspect it maybe platform dependent and I'm only 
testing against linux 32 bit.

I'm not sure where to document the behaviour, but as suggested you pass a 
dictionary to cefpython.Initialize(). The value field for a dictionary key can 
be empty as appropriate. Switches must not be prefixed with '-' or they will be 
ignored. All switches are appended to the command line arguments of all browser 
processes, no effort is made to deduplicate arguments.

Only tested on linux 32bit.

Original comment by finn.hug...@gmail.com on 24 Dec 2013 at 4:11

Attachments:

• 0001-Added-ability-to-programatically-set-command-line-sw.patch

[GoogleCodeExporter]

Comments on the patch below.

OnBeforeCommandLineProcessing is a callback from the CefApp class, but appears 
in the browser_process_hander.pyx file which is for the 
CefBrowserProcessHandler class. It should probably go to a separate file named 
"app.pyx".

The callback in the pyx file should have in its name "browser process" 
(App_OnBeforeCommandLineProcessing_BrowserProcess) to indicate that it is 
called only for the browser process.

The extraApplicationArgs param in Initialize() should be immutable, the same as 
applicationSettings is. Set the default value to None and if necessary set it 
to empty dictionary in the body of the function. In this case it doesn't make 
any difference, because Initialize() is called only once for the app lifetime, 
but if it was called twice then mutable object as a default value could cause 
troubles - let's avoid such code so that it does not get copied in the future.

This line of code:
    if switch and switch[0] != '-':
Should probably check if switch is a string before comparing first char.

The rest of the code looks good.

Original comment by czarek.t...@gmail.com on 24 Dec 2013 at 5:16

[GoogleCodeExporter]

The compile error in client handler was fixed in commit 44ce421a1082. It should
be passed 0, not NULL.

Original comment by czarek.t...@gmail.com on 24 Dec 2013 at 5:52

[GoogleCodeExporter]

> I can't see how to get a reference to the appropriate browser in either 
callback
> so I've just implemented global switches through 
OnBeforeCommandLineProcessing. 
> I had a look at OnRenderThreadCreated as suggested but can't see how that 
helps 
> me get a reference to the browser?

I see that there is no CefBrowser argument in 
CefBrowserProcessHandler::OnBeforeChildProcessLaunch().

Some switches need to be set in both the browser process and in the renderer 
process (or gpu/plugin
process) to take effect. So we should definitely allow for setting switches for 
processes other
than browser.

The same code that sets switches should also be executed in 
OnBeforeChildProcessLaunch().

But should it be the default or optional? Is propagating some unnecessary 
options to other processes 
going to cause troubles?

There is no way to know to which process switch will be propagated, so it can 
be either set:
1. in the Browser process 
2. or in the Browser process + all other sub-processes.
3. or only in sub-processes

The extraApplicationArgs param in Initialize() could have some flag. Maybe 
require the value to
be a tuple? Or maybe expose a python class CommandLine that have a third param 
in AppendSwitch()
method?

I think that for now we should go with the simplest solution - to propagate 
switches to all
sub-processes by default. I think that if a switch is not recognized by the 
renderer process
(or other) it will just be ignored, there shoudln't be any harm caused by that. 
You may ask
on the CEF C++ Forum to confirm that.

Original comment by czarek.t...@gmail.com on 24 Dec 2013 at 6:47

[GoogleCodeExporter]

As ever great feedback, sorry for the delay I've been away and then had my 
laptop stolen. I have been intending to post something on the cef forum about 
whether it's going to cause a problem but haven't got that far yet.

I've updated my patch to take account of your comments. I'm only testing with 
--enable-media-stream as that's the motivation behind preparing the patch. I 
can see that the switch is applied to all processes and applied twice to 
renderers. Although this doesn't seem to cause a problem I'll be tempted to 
knock out command line appending from OnBeforeChildProcessLaunch in our build 
simply to avoid the unnecessary additions...

Original comment by finna...@gmail.com on 6 Jan 2014 at 5:08

Attachments:

• 0001-Added-ability-to-programatically-set-command-line-sw.patch

[GoogleCodeExporter]

Can you add debug statements to both C++ and Python? In C++ use DebugLog(), in 
Python Debug(). And please post example logs so we can see for which proceccess 
the switch is set and for how many times. Patch looks good, only some debug 
code would be nice, so we can see what is going on.

Original comment by czarek.t...@gmail.com on 6 Jan 2014 at 7:16

[GoogleCodeExporter]

I'm not really sure what you're after, for me the most useful debug was 
printing out the command line used after appending the switches but that seems 
fairly verbose when ps works so well to show you the same thing. I left the 
printf to show the command line in the code but commented out. I've attached 
the output of 'ps -C subprocess -o args' which demonstrates the behaviour 
outlined and its verbosity.

The thing that tempts me is putting the GetCommandLineString and GetSwitchValue 
in the .pxd so that debugging and filtering are easy from python? I could then 
at least have a debug message saying something like "appending global switches 
to 'zygote' process" and only print that if there are command line switches 
specified.

Original comment by finna...@gmail.com on 7 Jan 2014 at 9:48

Attachments:

• enable-media-stream.txt

[GoogleCodeExporter]

It seems from the logs that "enable-media-stream" is also set twice for 
gpu-process. We could use CefCommandLine::HasSwitch() and GetSwitchValue() to 
check if switch was already set.

DebugLog() prints the message and logs it to the debug.log file. The same for 
Debug() in Python. And when debugging flag is off then it won't be printed. 

Debugging the whole command line string would be a nice addition, this would 
give us a look at what is going under the CEF hood.

Original comment by czarek.t...@gmail.com on 7 Jan 2014 at 10:56

[GoogleCodeExporter]

In OnBeforeCommandLineProcessing() the debug statement for logging the whole 
command line string should be outside the "#ifdef BROWSER_PROCESS" condition. 
On Windows printing messages in processes other than browser process are not 
visible in the console (on Linux they are visible).The only way to see them is 
to log messages to a debug.log file. So the DebugLog() function can be useful 
here. The "ps" command won't work on Windows.

Original comment by czarek.t...@gmail.com on 7 Jan 2014 at 11:04

[GoogleCodeExporter]

OK, I'll add python debug to print the command line, do you want me to print it 
regardless or only when we are appending switches?

I could deduplicate the command line but there are common norms for processes 
to handle this themselves so I deemed it unnecessary and some sort of filtering 
a more sensible place to add additional complexity. Maybe I should add code so 
that if 'value' is a dictionary with 'value' and 'filter' entries we'll apply 
the switch only to processes with a type in the list of specified filters (a 
tuple instead of a dictionary would also be fine).

Maybe OnBeforeCommandLineProcessing is entirely unnecessary since everything is 
a subprocess? In which case I'll stop that call from appending switches at all? 
Obviously we could still have duplicate entries but I'd prefer to let chrome 
deal with that...

Hmmm, just seen your follow up comment but I don't think that changes anything 
I've just written?

Original comment by finna...@gmail.com on 7 Jan 2014 at 11:23

[GoogleCodeExporter]

I would print the command line string for all processes even when no switches 
were provided to Initialize(). That is some useful debug information that we 
could use.

You can easily detect if switch was set using HasSwitch() and GetSwitchValue().

I would like to get rid of the double switch values, as we are printing the 
whole command line strings, and having repeated values makes it harder to read.

The OnBeforeCommandLineProcessing callback is necessary, it is the only way to 
set switches for the main process. It seems that the problem is that switches 
are automatically inherited for the subprocesses, when set in 
OnBeforeCommandLineProcessing, thus the repeated values in command line string. 
But there is no guarantee that a flag set in main process, will also be set for 
a subprocess (or maybe there is?). That's why I think that we should check if 
the switch has been set in OnBeforeChildProcessLaunch. Also the value should be 
compared, because if it's different then it should be overwritten.

> OK, I'll add python debug to print the command line

Putting print debug statements in Python code will display command line string 
only for the main process. In comment #13 I've explained what to do to display 
command line string for all processes.

Original comment by czarek.t...@gmail.com on 7 Jan 2014 at 11:50

[GoogleCodeExporter]

I think I've finally understood the relationship betweeen the two functions 
(took long enough). For all child processes we can modify the command line 
before launching the process so we use OnBeforeChildProcessLaunch. The main 
process is an exception so we modify the arguments after launching the process, 
this isn't generally safe so we only do it for the main browser process. I hope 
that's right because it finally makes sense to me ;)

I take it you are OK with duplicate switches where the value is different as 
that was the case I didn't want to tackle...

I'll up load a new patch in a minute, I've attached a log.

Original comment by finna...@gmail.com on 8 Jan 2014 at 10:42

Attachments:

• cefpython.log

[GoogleCodeExporter]

Hopefully I've addressed your concerns and this is ready for inclusion, I'm 
happier with it anyway.

Original comment by finna...@gmail.com on 8 Jan 2014 at 11:09

Attachments:

• 0001-Added-ability-to-programatically-set-command-line-sw.patch

[GoogleCodeExporter]

Patch looks good. Good work Finn, thanks.

Original comment by czarek.t...@gmail.com on 8 Jan 2014 at 11:56

[GoogleCodeExporter]

Fixed in revision fd0ed30fb387.

Updated the cefpython wiki page with the description of the new 
commandLineSwitches param.

Original comment by czarek.t...@gmail.com on 15 Jan 2014 at 9:38

• Changed state: Fixed

[GoogleCodeExporter]

Mentioned that you're the author of the patch, on the Changelog3 wiki page.

Original comment by czarek.t...@gmail.com on 15 Jan 2014 at 10:52

[GoogleCodeExporter]

Created the CommandLineSwitches wiki page.

Original comment by czarek.t...@gmail.com on 16 Jan 2014 at 9:48

[GoogleCodeExporter]

If you think there are some useful switches that would be nice to have them 
listed on the CommandLineSwitches wiki page, let me know.

Original comment by czarek.t...@gmail.com on 16 Jan 2014 at 9:53

[GoogleCodeExporter]

The only other switch I'm currently using is for remote debugging as outlined 
in that issue.

Original comment by finn.hug...@gmail.com on 16 Jan 2014 at 10:00

[GoogleCodeExporter]

I've added the remote-debugging-port switch example on the wiki page with a 
link to your comment. Also a complete list of chromium source files with 
switches was assembled. The switches on peter.sh may not containt all the 
latest switches. Also that list splits switches into categories.

Original comment by czarek.t...@gmail.com on 16 Jan 2014 at 10:49

[GoogleCodeExporter]

Nice to see you've refactored the debug flags, I was tempted by something like 
that but felt like I'd best concentrate on the commandlineswitches patch as it 
had already taken far longer than I'd anticipated ;)

I hadn't spotted the changelog, maybe a link on the homepage in the download 
section as I tend to associate changelog with version download rather than 
looking for it in the wiki. I don't think it's a real issue as I was just 
looking at the commit log which is easily found.

Original comment by finn.hug...@gmail.com on 16 Jan 2014 at 11:33

[GoogleCodeExporter]

Yes, the debug flags are now set in the right manner. Though, even after that 
change, there was problem displaying the command line string for subprocesses, 
as OnBeforeCommandLineProcessing was called before the debug flags were set in 
OnRenderThreadCreated. But I've made some workaround to fix that.

There is a link to the Changelog3 wiki page on the CEF3_Download_Linux and 
CEF3_Download_Windows wiki pages. It's highlighted in red. I'll try to make it 
more visible.

Original comment by czarek.t...@gmail.com on 16 Jan 2014 at 11:44