Try Yet Again to handle the output we get from android in run-webkit-tests.

Try Yet Again to handle the output we get from android in run-webkit-tests.

We're seeing more crashes logging debug output (adb logcat) on the bots.
We had been assuming that the stdout and stderr from the subprocess
were regular binary pipes, but it turns out the android port (and only
the android port) was actually setting universal_newlines=True when
launching the subprocess (this is apparently needed because 'adb shell'
does implicit linefeed conversion).

This has the nasty side effect of making the file objects unicode instead of
bytestrs, which was causing us to crash down the road when we tried to
.decode('ascii') a unicode string (.decode() only works on bytes).

This patch fixes the bug and adds some more assertions and comments.

R=peter@chromium.org, wangxianzhu@chromium.org, joelo@chromium.org
BUG=386343, 496983
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196605

[Dirk Pranke, 2015-06-05 01:37:27]

Patchset #1 (id:1) has been deleted

[Dirk Pranke, 2015-06-05 01:39:37]

dpranke@chromium.org changed reviewers:
+ wangxianzhu@chromium.org
- kbr@chromium.org

[Dirk Pranke, 2015-06-05 01:43:09]

Patchset #1 (id:20001) has been deleted

[Dirk Pranke, 2015-06-05 01:47:56]

Joel, please tell me if this change is relatively understandable to someone new
to the code and Python (since you'll get stuck with it).

The crashes that triggered this latest patch:

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Android%20%28N...

etc.

Peter, Xianzhu - can one of you review this to confirm the Android behavior? I
had
http://stackoverflow.com/questions/13578416/read-binary-stdout-data-from-adb-...
for independent confirmation but was too lazy to figure out how I'd actually
test this myself.

Ken, Jochen, Levi - this is mostly for your amusement, since you've all been
bitten by this bug at this point. I think this is attempt #7 at trying to fix
it. 

I think I at least understand what the heck is going on at this point and why
we've been confused before, but I'm not totally convinced the code is
bulletproof yet. This patch should fix this case, but I've filed bug 496983 to
do a more thorough analysis and hopefully bring some consistency someday to the
android driver.

[leviw_travelin_and_unemployed, 2015-06-05 03:04:17]

On 2015/06/05 at 01:47:56, dpranke wrote:
> Joel, please tell me if this change is relatively understandable to someone
new to the code and Python (since you'll get stuck with it).
> 
> The crashes that triggered this latest patch:
> 
>
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Android%20%28N...
> 
> etc.
> 
> Peter, Xianzhu - can one of you review this to confirm the Android behavior? I
had
http://stackoverflow.com/questions/13578416/read-binary-stdout-data-from-adb-...
for independent confirmation but was too lazy to figure out how I'd actually
test this myself.
> 
> Ken, Jochen, Levi - this is mostly for your amusement, since you've all been
bitten by this bug at this point. I think this is attempt #7 at trying to fix
it. 
> 
> I think I at least understand what the heck is going on at this point and why
we've been confused before, but I'm not totally convinced the code is
bulletproof yet. This patch should fix this case, but I've filed bug 496983 to
do a more thorough analysis and hopefully bring some consistency someday to the
android driver.

Thanks for the thoroughness. Color me amused :)

I'd have never guessed you had to jump through such hoops...

[Xianzhu, 2015-06-05 04:15:10]

lgtm.

I vaguely remember zhenghao@ ever put encode('utf8') somewhere perhaps to fix a
similar issue several years ago, but I can't find it in the current code.

[Dirk Pranke, 2015-06-05 19:43:19]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2015-06-05 19:43:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1169433005/40001

[joelo, 2015-06-05 20:37:17]

lgtm

dpranke@: Yup, looks grokkable to me. Thanks!!

[Ken Russell (switch to Gerrit), 2015-06-05 20:57:54]

kbr@chromium.org changed reviewers:
+ kbr@chromium.org

[Ken Russell (switch to Gerrit), 2015-06-05 20:57:54]

Very nice diagnosis Dirk. LGTM FWIW.

[commit-bot: I haz the power, 2015-06-05 21:07:50]

Committed patchset #1 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196605