Use file to pass arguments to the shell.

Use file to pass arguments to the shell.

Passing parameters directly through the intent have the following
issues:
- It limits the size of the parameters to 1024 bytes.
- It is regularly broken by new versions of adb.

R=ppi@chromium.org
BUG=Fixes https://github.com/domokit/mojo/issues/488

Committed: https://chromium.googlesource.com/external/mojo/+/69df74a38358c7c3611babeeaa5b8258dce05619

[ppi, 2015-10-29 16:27:40]

https://codereview.chromium.org/1414633004/diff/1/mojo/devtools/common/devtoo...
File mojo/devtools/common/devtoolslib/android_shell.py (right):

https://codereview.chromium.org/1414633004/diff/1/mojo/devtools/common/devtoo...
mojo/devtools/common/devtoolslib/android_shell.py:354: '/sdcard/%s/%s' %
(_MOJO_SHELL_PACKAGE_NAME, str(uuid.uuid4())))
can you append or prepend "mojo_shell_args" to the name in case anybody ever
looks at these files?

https://codereview.chromium.org/1414633004/diff/1/mojo/devtools/common/devtoo...
mojo/devtools/common/devtoolslib/android_shell.py:354: '/sdcard/%s/%s' %
(_MOJO_SHELL_PACKAGE_NAME, str(uuid.uuid4())))
Should we push this to some /cache/ folder so that these files can be easily
cleaned up if they leak?

https://codereview.chromium.org/1414633004/diff/1/mojo/devtools/common/devtoo...
mojo/devtools/common/devtoolslib/android_shell.py:366: cmd += ['--es',
'parametersFile', device_filename]
Should we call it argsFile for consistency?

https://codereview.chromium.org/1414633004/diff/1/shell/android/apk/src/org/c...
File shell/android/apk/src/org/chromium/mojo/shell/ShellService.java (right):

https://codereview.chromium.org/1414633004/diff/1/shell/android/apk/src/org/c...
shell/android/apk/src/org/chromium/mojo/shell/ShellService.java:241: private
static String[] getParametersFromIntent(Intent intent) {
Same here, wdyt about saying "args" instead of "parameters" everywhere here, to
match the naming we use on the c++ side?

https://codereview.chromium.org/1414633004/diff/1/shell/android/apk/src/org/c...
shell/android/apk/src/org/chromium/mojo/shell/ShellService.java:246: if
(parameterFile != null) {
should we do 

if (parameterFile == null) {
  return null;
}

to limit nesting?

https://codereview.chromium.org/1414633004/diff/1/shell/android/apk/src/org/c...
shell/android/apk/src/org/chromium/mojo/shell/ShellService.java:248: if
(file.isFile()) {
same here, should we invert the condition and return early if not met?

[qsr, 2015-10-29 16:57:02]

https://codereview.chromium.org/1414633004/diff/1/mojo/devtools/common/devtoo...
File mojo/devtools/common/devtoolslib/android_shell.py (right):

https://codereview.chromium.org/1414633004/diff/1/mojo/devtools/common/devtoo...
mojo/devtools/common/devtoolslib/android_shell.py:354: '/sdcard/%s/%s' %
(_MOJO_SHELL_PACKAGE_NAME, str(uuid.uuid4())))
On 2015/10/29 16:27:40, ppi wrote:
> Should we push this to some /cache/ folder so that these files can be easily
> cleaned up if they leak?

Not that I know of. Cache folder are in the app own directory, so they are not
accessible on non-rooted device.

https://codereview.chromium.org/1414633004/diff/1/mojo/devtools/common/devtoo...
mojo/devtools/common/devtoolslib/android_shell.py:354: '/sdcard/%s/%s' %
(_MOJO_SHELL_PACKAGE_NAME, str(uuid.uuid4())))
On 2015/10/29 16:27:40, ppi wrote:
> can you append or prepend "mojo_shell_args" to the name in case anybody ever
> looks at these files?

It is already in a folder with the shell package name. So I just prepended args_

https://codereview.chromium.org/1414633004/diff/1/mojo/devtools/common/devtoo...
mojo/devtools/common/devtoolslib/android_shell.py:366: cmd += ['--es',
'parametersFile', device_filename]
On 2015/10/29 16:27:40, ppi wrote:
> Should we call it argsFile for consistency?

Done.

https://codereview.chromium.org/1414633004/diff/1/shell/android/apk/src/org/c...
File shell/android/apk/src/org/chromium/mojo/shell/ShellService.java (right):

https://codereview.chromium.org/1414633004/diff/1/shell/android/apk/src/org/c...
shell/android/apk/src/org/chromium/mojo/shell/ShellService.java:241: private
static String[] getParametersFromIntent(Intent intent) {
On 2015/10/29 16:27:40, ppi wrote:
> Same here, wdyt about saying "args" instead of "parameters" everywhere here,
to
> match the naming we use on the c++ side?

Done.

https://codereview.chromium.org/1414633004/diff/1/shell/android/apk/src/org/c...
shell/android/apk/src/org/chromium/mojo/shell/ShellService.java:246: if
(parameterFile != null) {
On 2015/10/29 16:27:40, ppi wrote:
> should we do 
> 
> if (parameterFile == null) {
>   return null;
> }
> 
> to limit nesting?

I'd rather not. In case we introduced another way to pass parameters. This is
only the third time we change this.

https://codereview.chromium.org/1414633004/diff/1/shell/android/apk/src/org/c...
shell/android/apk/src/org/chromium/mojo/shell/ShellService.java:248: if
(file.isFile()) {
On 2015/10/29 16:27:40, ppi wrote:
> same here, should we invert the condition and return early if not met?

Done.

[ppi, 2015-10-29 17:07:02]

lgtm

[qsr, 2015-10-29 17:07:25]

Committed patchset #2 (id:20001) manually as
69df74a38358c7c3611babeeaa5b8258dce05619 (presubmit successful).