Enable manual bisects on Chrome for Android.

Enable manual bisects on Chrome for Android.

This patch fixes various problems in the bisect scripts that were
preventing their use for Chrome for Android. I have only tried these
using run-manual-bisect-test, and there could potentially be
further problems with bisect wrapper scripts.

Also fixes the --use-goma option to actually use Goma, rather than
simply increasing the number of threads.

BUG=none

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

[aberent, 2014-06-13 11:11:54]

Motivated by needing to bisect a bug on Android.

[bulach, 2014-06-13 11:33:55]

lgtm, thanks! please wait for prasadv and ross, I'm not too familiar here :)

[rmcilroy, 2014-06-13 14:04:21]

https://codereview.chromium.org/331593007/diff/40001/tools/run-bisect-manual-...
File tools/run-bisect-manual-test.py (right):

https://codereview.chromium.org/331593007/diff/40001/tools/run-bisect-manual-...
tools/run-bisect-manual-test.py:74: cmd.extend(['--target_build_type',
options.browser_type.title()])
I don't think these two lines are required - target_build_type should always be
set (by default to "Release" on line 122) not explicitly set by the user, right?
 Also, if this is needed, should it be an "elif" here, since that means it
doesn't get executed for android or android-chrome target platforms?

Just wondering - is there a reason why you need to set target_build_type
differently from options.browser_type.title() ?

https://codereview.chromium.org/331593007/diff/40001/tools/run-bisect-manual-...
tools/run-bisect-manual-test.py:122: default='Release',
Maybe we shouldn't have a default here and instead just rely on the value of
options.browser_type.title() as the default if the user doesn't set it
explicitly?

[aberent, 2014-06-13 14:37:36]

New patch will follow, but I may be a few hours.

https://codereview.chromium.org/331593007/diff/40001/tools/run-bisect-manual-...
File tools/run-bisect-manual-test.py (right):

https://codereview.chromium.org/331593007/diff/40001/tools/run-bisect-manual-...
tools/run-bisect-manual-test.py:74: cmd.extend(['--target_build_type',
options.browser_type.title()])
Because if, for example, browser-type is android-chrome (which is what is used
for Clank), then title is Android-Chrome, yet the build type should still be
either Release or Debug, corresponding to the build subdirectories.

I was including this to attempt to keep compatibility with the old code, but, as
you say, because target_build_type has a default it will never be used. I will
take out the default.

The elif is definitely needed, we never want to set target_build_type to
Android-Chrome or Android (or Cros).

On 2014/06/13 14:04:21, rmcilroy wrote:
> I don't think these two lines are required - target_build_type should always
be
> set (by default to "Release" on line 122) not explicitly set by the user,
right?
>  Also, if this is needed, should it be an "elif" here, since that means it
> doesn't get executed for android or android-chrome target platforms?
> 
> Just wondering - is there a reason why you need to set target_build_type
> differently from options.browser_type.title() ?

https://codereview.chromium.org/331593007/diff/40001/tools/run-bisect-manual-...
tools/run-bisect-manual-test.py:122: default='Release',
On 2014/06/13 14:04:21, rmcilroy wrote:
> Maybe we shouldn't have a default here and instead just rely on the value of
> options.browser_type.title() as the default if the user doesn't set it
> explicitly?

Yes. See above.

[aberent, 2014-06-17 13:28:17]

Finally got back to this!

https://codereview.chromium.org/331593007/diff/40001/tools/run-bisect-manual-...
File tools/run-bisect-manual-test.py (right):

https://codereview.chromium.org/331593007/diff/40001/tools/run-bisect-manual-...
tools/run-bisect-manual-test.py:122: default='Release',
On 2014/06/13 14:37:36, aberent wrote:
> On 2014/06/13 14:04:21, rmcilroy wrote:
> > Maybe we shouldn't have a default here and instead just rely on the value of
> > options.browser_type.title() as the default if the user doesn't set it
> > explicitly?
> 
> Yes. See above.

Done.

[rmcilroy, 2014-06-17 17:20:12]

lgtm, thanks!

https://codereview.chromium.org/331593007/diff/60001/tools/bisect_utils.py
File tools/bisect_utils.py (right):

https://codereview.chromium.org/331593007/diff/60001/tools/bisect_utils.py#ne...
tools/bisect_utils.py:463: 'use_goma=1')
nit - align to brackets.

https://codereview.chromium.org/331593007/diff/60001/tools/run-bisect-manual-...
File tools/run-bisect-manual-test.py (right):

https://codereview.chromium.org/331593007/diff/60001/tools/run-bisect-manual-...
tools/run-bisect-manual-test.py:123: '(default), or "Debug".')
remove "(default)" from help string (or mention the default is based on the
browser type).

[aberent, 2014-06-18 10:10:11]

https://codereview.chromium.org/331593007/diff/60001/tools/bisect_utils.py
File tools/bisect_utils.py (right):

https://codereview.chromium.org/331593007/diff/60001/tools/bisect_utils.py#ne...
tools/bisect_utils.py:463: 'use_goma=1')
On 2014/06/17 17:20:12, rmcilroy wrote:
> nit - align to brackets.

Done.

https://codereview.chromium.org/331593007/diff/60001/tools/run-bisect-manual-...
File tools/run-bisect-manual-test.py (right):

https://codereview.chromium.org/331593007/diff/60001/tools/run-bisect-manual-...
tools/run-bisect-manual-test.py:123: '(default), or "Debug".')
On 2014/06/17 17:20:12, rmcilroy wrote:
> remove "(default)" from help string (or mention the default is based on the
> browser type).

Done.

[aberent, 2014-06-18 10:10:19]

The CQ bit was checked by aberent@chromium.org

[commit-bot: I haz the power, 2014-06-18 10:12:37]

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

[commit-bot: I haz the power, 2014-06-18 15:23:10]

Change committed as 278082