[Android] Add an option to view the tracing record categories when running from python script.

[Android] Add an option to view the tracing record categories when running from python script.

There is no way to see/list the categories present for recording the chrome traces (when running from adb_profile_chrome.py) unlike the UI. Adding the "list" command for --categories option, to keep the usage same as with systrace option.

Usage: build/android/adb_profile_chrome --categories list

BUG=366679
TESTS=To be added.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=268841

[r.kasibhatla, 2014-05-05 08:54:26]

PTAL.

[r.kasibhatla, 2014-05-05 09:17:25]

PTAL.

[Xianzhu, 2014-05-05 16:32:21]

https://codereview.chromium.org/257093004/diff/40001/build/android/adb_profil...
File build/android/adb_profile_chrome.py (right):

https://codereview.chromium.org/257093004/diff/40001/build/android/adb_profil...
build/android/adb_profile_chrome.py:62: category_list =
device.old_interface.WaitForLogMatch(
You may want to use 'import json'.

https://codereview.chromium.org/257093004/diff/40001/content/browser/android/...
File content/browser/android/tracing_controller_android.cc (right):

https://codereview.chromium.org/257093004/diff/40001/content/browser/android/...
content/browser/android/tracing_controller_android.cc:92: const
std::set<std::string>& categories_received) {
As the Java side doesn't actually need the categories but just print a log
message for the python script, can we avoid going back to Java by printing a log
message here?

[r.kasibhatla, 2014-05-06 11:04:05]

PTAL.

https://codereview.chromium.org/257093004/diff/40001/build/android/adb_profil...
File build/android/adb_profile_chrome.py (right):

https://codereview.chromium.org/257093004/diff/40001/build/android/adb_profil...
build/android/adb_profile_chrome.py:62: category_list =
device.old_interface.WaitForLogMatch(
On 2014/05/05 16:32:21, Xianzhu wrote:
> You may want to use 'import json'.

Done.

https://codereview.chromium.org/257093004/diff/40001/content/browser/android/...
File content/browser/android/tracing_controller_android.cc (right):

https://codereview.chromium.org/257093004/diff/40001/content/browser/android/...
content/browser/android/tracing_controller_android.cc:92: const
std::set<std::string>& categories_received) {
On 2014/05/05 16:32:21, Xianzhu wrote:
> As the Java side doesn't actually need the categories but just print a log
> message for the python script, can we avoid going back to Java by printing a
log
> message here?

Done.

[Sami, 2014-05-06 14:29:51]

Thanks, this is useful stuff! A few comments below.

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
File build/android/adb_profile_chrome.py (right):

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:75: logging.error('JSON format error')
I'd prefer just passing these exceptions through since if anything like this
happens it is more useful to fail noisily with a backtrace.

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:77: _PrintMessage('Record Categories:')
Instead of printing the results here, could you return a tuple
(record_categories, disabled_by_default_categories) and print them out at the
call site? That way we can use this function for other purposes  too (e.g.,
checking that we're using valid categories).

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:78: _PrintMessage('\n'.join(str('\t%s' %
item) \
Nit: "str" is unneeded here.

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:82: _PrintMessage('\n'.join(str('\t%s' %
item) \
Ditto.

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:85: except pexpect.TIMEOUT:
Please move this try/except block immediately around WaitForLogMatch since
that's the only place where we expect to get this exception.

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:423: _PrintMessage('Collecting data...',
'\r')
Nit: please use eol='' to match the other functions here.

https://codereview.chromium.org/257093004/diff/80001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java
(right):

https://codereview.chromium.org/257093004/diff/80001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java:49:
private static final String ACTION_LIST_CATEGORY = "GPU_PROFILER_LIST_CATEGORY";
Nit: CATEGORIES instead of CATEGORY.

[r.kasibhatla, 2014-05-06 15:22:28]

Updated as per review comments.

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
File build/android/adb_profile_chrome.py (right):

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:75: logging.error('JSON format error')
On 2014/05/06 14:29:51, Sami wrote:
> I'd prefer just passing these exceptions through since if anything like this
> happens it is more useful to fail noisily with a backtrace.

Done.

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:77: _PrintMessage('Record Categories:')
On 2014/05/06 14:29:51, Sami wrote:
> Instead of printing the results here, could you return a tuple
> (record_categories, disabled_by_default_categories) and print them out at the
> call site? That way we can use this function for other purposes  too (e.g.,
> checking that we're using valid categories).
In the original patch, I was returning the tuple, but changed to avoid the
unnecessary creation of lists at two places. Anyways, I reverted to previous
patch logic of returning the tuple. :)

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:78: _PrintMessage('\n'.join(str('\t%s' %
item) \
On 2014/05/06 14:29:51, Sami wrote:
> Nit: "str" is unneeded here.

Done.

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:82: _PrintMessage('\n'.join(str('\t%s' %
item) \
On 2014/05/06 14:29:51, Sami wrote:
> Ditto.

Done.

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:85: except pexpect.TIMEOUT:
On 2014/05/06 14:29:51, Sami wrote:
> Please move this try/except block immediately around WaitForLogMatch since
> that's the only place where we expect to get this exception.

Done.

https://codereview.chromium.org/257093004/diff/80001/build/android/adb_profil...
build/android/adb_profile_chrome.py:423: _PrintMessage('Collecting data...',
'\r')
On 2014/05/06 14:29:51, Sami wrote:
> Nit: please use eol='' to match the other functions here.

Done.

https://codereview.chromium.org/257093004/diff/80001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java
(right):

https://codereview.chromium.org/257093004/diff/80001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/TracingControllerAndroid.java:49:
private static final String ACTION_LIST_CATEGORY = "GPU_PROFILER_LIST_CATEGORY";
On 2014/05/06 14:29:51, Sami wrote:
> Nit: CATEGORIES instead of CATEGORY.

Done.

[Xianzhu, 2014-05-06 16:56:23]

lgtm. Thanks.

[Xianzhu, 2014-05-06 17:00:57]

+Yaron for owner's approval.

[Yaron, 2014-05-06 17:41:07]

lgtm

[r.kasibhatla, 2014-05-06 17:44:15]

The CQ bit was checked by r.kasibhatla@samsung.com

[commit-bot: I haz the power, 2014-05-06 17:44:56]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/r.kasibhatla@samsung.com/257093004/100001

[commit-bot: I haz the power, 2014-05-06 23:33:14]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_chromium_compile_dbg on tryserver.chromium

[r.kasibhatla, 2014-05-07 05:16:22]

The CQ bit was unchecked by r.kasibhatla@samsung.com

[r.kasibhatla, 2014-05-07 05:16:24]

The CQ bit was checked by r.kasibhatla@samsung.com

[commit-bot: I haz the power, 2014-05-07 05:17:28]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/r.kasibhatla@samsung.com/257093004/100001

[r.kasibhatla, 2014-05-07 09:08:32]

The CQ bit was unchecked by r.kasibhatla@samsung.com

[r.kasibhatla, 2014-05-07 09:08:45]

The CQ bit was checked by r.kasibhatla@samsung.com

[commit-bot: I haz the power, 2014-05-07 09:12:36]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/r.kasibhatla@samsung.com/257093004/100001

[commit-bot: I haz the power, 2014-05-07 17:37:24]

Change committed as 268841