[Chromedriver] Support long command line on Android

[Chromedriver] Support long command line on Android

On Android devices, ChromeDriver used to use shell "echo" command
to send Chrome command line to the device. Adb limits the length
of the command line that can be sent this way, ranging from less
than 1000 characters to about 4000 characters, depending on the Adb
version.

To allow longer command line to be used, ChromeDriver now uses Adb
file transfer protocol to send the command line file to the device.
This is the same underlying protocol used by "adb push" command.

BUG=727979
Review-Url: https://codereview.chromium.org/2972913002
Cr-Commit-Position: refs/heads/master@{#486815}
Committed: https://chromium.googlesource.com/chromium/src/+/4e04992e510213a402ea51649a6e9a45ff1a5f10

[johnchen, 2017-07-05 22:20:20]

johnchen@chromium.org changed reviewers:
+ jbudorick@chromium.org

[johnchen, 2017-07-05 22:20:21]

jbudorick@: Could you please take a look? This change allows ChromeDriver users
to use longer command lines on Android.

[johnchen, 2017-07-10 15:46:33]

jbudorick@: Could you please take a look at this CL? Thanks.

[jbudorick, 2017-07-13 15:25:30]

Looks pretty good.

https://codereview.chromium.org/2972913002/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/adb_impl.cc (right):

https://codereview.chromium.org/2972913002/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/adb_impl.cc:146: return Status(kUnknownError,
"Failed to set command line file " +
Should this just return status? Previously, this particular Status was only
returned if status.IsOk() but it couldn't find the expected exit code from the
echo command.

https://codereview.chromium.org/2972913002/diff/1/chrome/test/chromedriver/ne...
File chrome/test/chromedriver/net/adb_client_socket.cc (right):

https://codereview.chromium.org/2972913002/diff/1/chrome/test/chromedriver/ne...
chrome/test/chromedriver/net/adb_client_socket.cc:375: size_t length =
content_.length() - offset;
nit: length = std::min(content_.length() - offset, kAdbDataChunkSize)?

https://codereview.chromium.org/2972913002/diff/1/chrome/test/chromedriver/ne...
chrome/test/chromedriver/net/adb_client_socket.cc:404: for (int i = 0; i < 4;
i++) {
I'm looking for a way to do this using something in base/ (maybe
base/sys_byteorder.h?), though it looks like this is an unusual way to encode
integers for transport :(

https://codereview.chromium.org/2972913002/diff/1/chrome/test/chromedriver/ne...
chrome/test/chromedriver/net/adb_client_socket.cc:405: buffer.append(1,
(char)(data & 0xff));
nit: static_cast<char>

[johnchen, 2017-07-13 18:58:18]

Thanks for the feedback. I've updated the CL.

[johnchen, 2017-07-13 18:58:49]

The CQ bit was checked by johnchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-13 18:58:58]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-13 20:11:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-13 20:11:21]

Dry run: This issue passed the CQ dry run.

[johnchen, 2017-07-14 17:59:50]

jbudorick@: Do you have any further concerns about this CL? Thanks.

[jbudorick, 2017-07-14 18:08:56]

On 2017/07/14 17:59:50, johnchen wrote:
> jbudorick@: Do you have any further concerns about this CL? Thanks.

nope, just slipped through the cracks yesterday. lgtm

[johnchen, 2017-07-14 18:16:00]

The CQ bit was checked by johnchen@chromium.org

[commit-bot: I haz the power, 2017-07-14 18:16:16]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-14 18:21:55]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1500056160467900,
"parent_rev": "1d46ecdc00b7d228d987e20616f2001a987c1f65", "commit_rev":
"4e04992e510213a402ea51649a6e9a45ff1a5f10"}

[commit-bot: I haz the power, 2017-07-14 18:23:12]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1500056160467900,
"parent_rev": "1d46ecdc00b7d228d987e20616f2001a987c1f65", "commit_rev":
"4e04992e510213a402ea51649a6e9a45ff1a5f10"}

[commit-bot: I haz the power, 2017-07-14 18:23:34]

Description was changed from

==========
[Chromedriver] Support long command line on Android

On Android devices, ChromeDriver used to use shell "echo" command
to send Chrome command line to the device. Adb limits the length
of the command line that can be sent this way, ranging from less
than 1000 characters to about 4000 characters, depending on the Adb
version.

To allow longer command line to be used, ChromeDriver now uses Adb
file transfer protocol to send the command line file to the device.
This is the same underlying protocol used by "adb push" command.

BUG=727979
==========

to

==========
[Chromedriver] Support long command line on Android

On Android devices, ChromeDriver used to use shell "echo" command
to send Chrome command line to the device. Adb limits the length
of the command line that can be sent this way, ranging from less
than 1000 characters to about 4000 characters, depending on the Adb
version.

To allow longer command line to be used, ChromeDriver now uses Adb
file transfer protocol to send the command line file to the device.
This is the same underlying protocol used by "adb push" command.

BUG=727979

Review-Url: https://codereview.chromium.org/2972913002
Cr-Commit-Position: refs/heads/master@{#486815}
Committed:
https://chromium.googlesource.com/chromium/src/+/4e04992e510213a402ea51649a6e...
==========

[commit-bot: I haz the power, 2017-07-14 18:23:36]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4e04992e510213a402ea51649a6e...