[chromedriver] Write command line file to both /data/local and /data/local/tmp for Chrome on Androi…

[chromedriver] Write command line file to both /data/local and /data/local/tmp for Chrome on Android.

This supports the new feature where setting chrome as the debug app
causes Chrome to look for the command line file in tmp.

BUG=chromedriver:665
TEST=None

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

[frankf, 2014-01-15 00:34:26]

+bulach

Long term (after bulach's changes makes its way to the oldest supported
release), why can't we require chrome to be set as the debug app as a
prerequisite for running ChromeDriver? This avoids one unnecessary round trip
and should work for all devices.

[craigdh, 2014-01-15 01:05:55]

On 2014/01/15 00:34:26, frankf wrote:
> +bulach
> 
> Long term (after bulach's changes makes its way to the oldest supported
> release), why can't we require chrome to be set as the debug app as a
> prerequisite for running ChromeDriver? This avoids one unnecessary round trip
> and should work for all devices.

Yeah, I'm fine with that. In general it would be nice to be able to query for
whether chrome is the debug app but I don't see any way to do that with "adb
shell am".

[bulach, 2014-01-15 12:58:43]

lgtm when frank is happy

small suggestions below..
yeah, unfortunately there's no way to query the debug app..
btw, not quite sure at which level of chrome driver it could be hooked up, but
in telemetry, we're about to set up the debug app to chrome before launching the
browser, see:
https://codereview.chromium.org/138093002/

https://codereview.chromium.org/138873003/diff/30001/chrome/test/chromedriver...
File chrome/test/chromedriver/chrome/device_manager.cc (right):

https://codereview.chromium.org/138873003/diff/30001/chrome/test/chromedriver...
chrome/test/chromedriver/chrome/device_manager.cc:19: const char*
kChromeCmdLineFile = "/data/local/chrome-command-line";
you may want to create  bug and a TODO, link this patch to it, and date it to
m36 ? :)

then, could do with:

const char* kChromeCmdLineFile = "/data/local/tmp/chrome-command-line";
const char* kChromeCmdLineBeforeM33 = "/data/local/chrome-command-line";

https://codereview.chromium.org/138873003/diff/30001/chrome/test/chromedriver...
chrome/test/chromedriver/chrome/device_manager.cc:89: serial_,
"/data/local/tmp/chrome-command-line", exec_name, args);
see above, this would then become kChromeCmdLineFileBeforeM33, with a TODO to
remove this block.

[frankf, 2014-01-15 20:35:35]

Agree with Marcus, we should set the debug app programatically. One less step
required from the user.

[craigdh, 2014-01-16 17:40:21]

On 2014/01/15 20:35:35, frankf wrote:
> Agree with Marcus, we should set the debug app programatically. One less step
> required from the user.

So I mostly agree, the only thing that concerns me is that this option is
persistent so we could be switching which command line file Chrome reads from
after the test is done. Since we can't read the debug app we have no idea what
state to return to after quitting.

[bulach, 2014-01-16 19:35:40]

yeah, we had exactly the same discussion on my patch above :)

I think the trade off is, well, if you plug adb and want to run the tests, it's
probably more convenient to have it automated..
conversely, the risk is that chrome would be doing something "uncommon".

considering this is done only in bots and/or developer's phones, it seemed
simpler to automate it..

[frankf, 2014-01-17 18:39:14]

On 2014/01/16 19:35:40, bulach wrote:
> yeah, we had exactly the same discussion on my patch above :)
> 
> I think the trade off is, well, if you plug adb and want to run the tests,
it's
> probably more convenient to have it automated..
> conversely, the risk is that chrome would be doing something "uncommon".
> 
> considering this is done only in bots and/or developer's phones, it seemed
> simpler to automate it..

I think as long as you document the behaviour, it's fine either way. Either
the user needs to set the debug app as a prerequisite, or they're warned
that ChromeDriver messes with the debug app setting. Couple of questions:

1. Is the debug app persitant across reboots?
2. Can you set it through the system settings sqline3 database? This is just a
curiosity,
we shouldn't be messing with this. Take a look at full_deploy.py downstream.

[craigdh, 2014-01-22 22:11:43]

https://codereview.chromium.org/138873003/diff/30001/chrome/test/chromedriver...
File chrome/test/chromedriver/chrome/device_manager.cc (right):

https://codereview.chromium.org/138873003/diff/30001/chrome/test/chromedriver...
chrome/test/chromedriver/chrome/device_manager.cc:19: const char*
kChromeCmdLineFile = "/data/local/chrome-command-line";
On 2014/01/15 12:58:43, bulach_ooo_until_feb_2 wrote:
> you may want to create  bug and a TODO, link this patch to it, and date it to
> m36 ? :)
> 
> then, could do with:
> 
> const char* kChromeCmdLineFile = "/data/local/tmp/chrome-command-line";
> const char* kChromeCmdLineBeforeM33 = "/data/local/chrome-command-line";

So after some discussion it was decided to not set the debug app automatically,
in which case we'll need to keep writing to both these locations. Some of the
reasons were:

- What if the user wants to have another app set as debug app? No workaround if
we set it. This way we can support both.
- Element of least surprise: we don't magically switch where chrome reads the
command line file from for tests running afterwards.

[craigdh, 2014-01-29 19:29:04]

Modified it to set the debug app after more discussion with frank. Frank is OOO
now so +stgao instead.

[stgao, 2014-01-29 20:00:42]

lgtm if you guys have a agreement on this.

[commit-bot: I haz the power, 2014-01-29 20:33:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/craigdh@chromium.org/138873003/150001

[commit-bot: I haz the power, 2014-01-30 08:45:05]

Change committed as 247901