Support startup performance tests on Galaxy S3

Support startup performance tests on Galaxy S3
    
    The Galaxy S3's build of android is of type 'user' (as one would
    expect for a production device). This prevents the use of 'adb root',
    even on rooted devices, however one can root an S3 and install an
    'su' command on it without changing the android build. The changes
    in this commit switch to using the 'su' command in place of 'adb root'.
    Since writing to a protected file using su is quite complex they
    encapsulate this in a the function SetProtectedFileContents()
    and make sure that all calls in the Android test framework that write
    to protected files call this.
    
    Also, allow variable wait for logcat entries. This is now needed since
    other test changes mean that the 10 second default may not be adequate
    in some cases.
    
    BUG=169011, 163336
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177508

[bulach, 2013-01-14 17:26:50]

lgtm, thanks! small nits / suggestions:

https://codereview.chromium.org/11876021/diff/1/build/android/pylib/flag_chan...
File build/android/pylib/flag_changer.py (right):

https://codereview.chromium.org/11876021/diff/1/build/android/pylib/flag_chan...
build/android/pylib/flag_changer.py:94: self._android_cmd.RunShellCommand(
nit: the escaping makes it hard to read :)
how about SetFileContents to somewhere public in $STORAGE_CARD, then use 
"su cp SRC TARGET"

https://codereview.chromium.org/11876021/diff/1/build/android/pylib/perf_test...
File build/android/pylib/perf_tests_helper.py (right):

https://codereview.chromium.org/11876021/diff/1/build/android/pylib/perf_test...
build/android/pylib/perf_tests_helper.py:136: self._num_cpus =
int(re.findall(r'\d+',num_cpus[0])[-1])
nit: space after ,

[aberent, 2013-01-14 20:19:10]

https://codereview.chromium.org/11876021/diff/1/build/android/pylib/flag_chan...
File build/android/pylib/flag_changer.py (right):

https://codereview.chromium.org/11876021/diff/1/build/android/pylib/flag_chan...
build/android/pylib/flag_changer.py:94: self._android_cmd.RunShellCommand(
On 2013/01/14 17:26:50, bulach wrote:
> nit: the escaping makes it hard to read :)
> how about SetFileContents to somewhere public in $STORAGE_CARD, then use 
> "su cp SRC TARGET"
It didn't work anyway, once I retested it, so that is what I am ending up with.
I don't know how it worked first time.
See my next patch.

https://codereview.chromium.org/11876021/diff/1/build/android/pylib/perf_test...
File build/android/pylib/perf_tests_helper.py (right):

https://codereview.chromium.org/11876021/diff/1/build/android/pylib/perf_test...
build/android/pylib/perf_tests_helper.py:136: self._num_cpus =
int(re.findall(r'\d+',num_cpus[0])[-1])
On 2013/01/14 17:26:50, bulach wrote:
> nit: space after ,

Done.

[bulach, 2013-01-15 09:51:00]

lgtm, thanks! just one nit and one ignorable suggestion below:

https://codereview.chromium.org/11876021/diff/5002/build/android/pylib/flag_c...
File build/android/pylib/flag_changer.py (right):

https://codereview.chromium.org/11876021/diff/5002/build/android/pylib/flag_c...
build/android/pylib/flag_changer.py:12:
CHROME_TEMP_COMMAND_FILE='/sdcard/chrome-command-line-temp'
I guess "sdcard" should be "$EXTERNAL_STORAGE"

https://codereview.chromium.org/11876021/diff/5002/build/android/pylib/flag_c...
build/android/pylib/flag_changer.py:102: self._android_cmd.RunShellCommand('rm '
+ CHROME_TEMP_COMMAND_FILE)
we could potentially generate a shell script for these three commands and source
it, but I don't think it'd be any more readable...

[aberent, 2013-01-16 19:06:14]

Significant changes, please re-review.

[bulach, 2013-01-16 19:11:05]

lgtm, thanks! just some small suggestions below:

https://codereview.chromium.org/11876021/diff/8001/build/android/pylib/androi...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/11876021/diff/8001/build/android/pylib/androi...
build/android/pylib/android_commands.py:674: self.GetExternalStorage() + '/' +
base_name % i):
since base_name is required to be a format and this is an internal method, just
remove the param and use _TEMP_FILE_BASE directly.. also, add a "_FMT" suffix,
the % operation can be ambiguous..

https://codereview.chromium.org/11876021/diff/8001/build/android/pylib/androi...
build/android/pylib/android_commands.py:679: """Writes |contents| to the file
specified by |filename|.
nit: add a \n (python style requires a one-line docstring, then \n, then the
extended documentation.
also, the next lines should be unaligned, the """ needs to be on a line on its
own, see 702-709 below.

https://codereview.chromium.org/11876021/diff/8001/build/android/pylib/flag_c...
File build/android/pylib/flag_changer.py (right):

https://codereview.chromium.org/11876021/diff/8001/build/android/pylib/flag_c...
build/android/pylib/flag_changer.py:12:
CHROME_TEMP_COMMAND_FILE='/sdcard/chrome-command-line-temp'
not needed?

[aberent, 2013-01-17 17:18:03]

https://codereview.chromium.org/11876021/diff/8001/build/android/pylib/androi...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/11876021/diff/8001/build/android/pylib/androi...
build/android/pylib/android_commands.py:674: self.GetExternalStorage() + '/' +
base_name % i):
On 2013/01/16 19:11:05, bulach wrote:
> since base_name is required to be a format and this is an internal method,
just
> remove the param and use _TEMP_FILE_BASE directly.. also, add a "_FMT" suffix,
> the % operation can be ambiguous..
Will add format, but can't use _TEMP_FILE_BASE(_FMT) directly without
duplicating code since this is also used with _TEMP_SCRIPT_FILE_BASE(_FMT)

https://codereview.chromium.org/11876021/diff/8001/build/android/pylib/androi...
build/android/pylib/android_commands.py:679: """Writes |contents| to the file
specified by |filename|.
On 2013/01/16 19:11:05, bulach wrote:
> nit: add a \n (python style requires a one-line docstring, then \n, then the
> extended documentation.
> also, the next lines should be unaligned, the """ needs to be on a line on its
> own, see 702-709 below.

Done.

https://codereview.chromium.org/11876021/diff/8001/build/android/pylib/flag_c...
File build/android/pylib/flag_changer.py (right):

https://codereview.chromium.org/11876021/diff/8001/build/android/pylib/flag_c...
build/android/pylib/flag_changer.py:12:
CHROME_TEMP_COMMAND_FILE='/sdcard/chrome-command-line-temp'
On 2013/01/16 19:11:05, bulach wrote:
> not needed?

Done.

[commit-bot: I haz the power, 2013-01-17 17:24:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aberent@chromium.org/11876021/13001

[frankf, 2013-01-17 18:45:39]

We want to enable pylint presubmit for pylib eventually (issue 168518). If you
touch a file, please make sure to run lint before committing.

https://codereview.chromium.org/11876021/diff/13001/build/android/pylib/flag_...
File build/android/pylib/flag_changer.py (right):

https://codereview.chromium.org/11876021/diff/13001/build/android/pylib/flag_...
build/android/pylib/flag_changer.py:92:
self._android_cmd.SetProtectedFileContents(CHROME_COMMAND_FILE,
alignment.

[commit-bot: I haz the power, 2013-01-17 20:31:28]

List of reviewers changed. frankf@chromium.org did a drive-by without LGTM'ing!

[frankf, 2013-01-17 21:56:45]

lgtm, not blocking this.

[commit-bot: I haz the power, 2013-01-17 22:02:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aberent@chromium.org/11876021/13001

[commit-bot: I haz the power, 2013-01-17 22:07:12]

Change committed as 177508