New run shell implementation for DeviceUtils

New run shell implementation for DeviceUtils

The main differences are:
- it uses AdbWrapper.Shell to actually execute the command.
- when the cmd is supplied as a list of a command and its arguments,
  the arguments are quoted to prevent them from being (mis)interpreted
  by the shell.
- a new single_line option to check that the output produces contains a single line, and return the value of that line.

BUG=267773
Committed: https://crrev.com/ab18bcca0cc1475cf381211e595e75c1146c9115
Cr-Commit-Position: refs/heads/master@{#300237}

[perezju, 2014-10-14 16:06:59]

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

[perezju, 2014-10-14 16:07:00]

The tests also show some example usage, and I also included a sample
implementation of GetExternalStoragePath using the new interface.

[jbudorick, 2014-10-14 16:55:19]

This is definitely on the right track. Still thinking about doing this
separately from _RunShellCommandImpl.

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:149: def GetExternalStoragePath(self,
timeout=_DEFAULT_TIMEOUT,
I'm not crazy about moving the timeout/retry boundary way down to the
implementation of AdbWrapper.Shell, which is what this appears to do. I'd like
to stick with handling them on the DeviceUtils boundary.

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:170: if not value:
I'm not sure that this _has_ to be an error (although there may be enough
clients depending on it existing that it should be).

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:173: self._cache['external_storage']
= value
I like the idea of caching things like this and certain system properties -- we
do both of these already in AndroidCommands -- but let's talk about the
implementation.

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:333: def _NewRunShellImpl(self, cmd,
cwd=None, env=None, as_root=False,
I'm not sure about eliminating check_return here. I think the goal is to
eventually switch to default True, but I don't think we want to get rid of it.

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:364: if not
re.match('^[a-zA-Z_][a-zA-Z0-9_]*$', key):
Compile the regex as a constant and then use that to do the matching.

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:373: cmd = '%s %s' % ('
'.join(env_quote(*kv) for kv in env.iteritems()), cmd)
This seems a little more readable to me:

  env_quote(k, v) for k, v in env.iteritems()

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
File build/android/pylib/device/device_utils_test.py (right):

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils_test.py:301: with
mock.patch.object(self.device, '_NewRunShellImpl',
I'm not sure I like mocking here vs mocking between DeviceUtils and AdbWrapper -
in this case, mocking AdbWrapper.Shell.

[perezju, 2014-10-15 09:22:54]

Also added some notes on the DeviceUtils implementation doc.

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:170: if not value:
On 2014/10/14 16:55:19, jbudorick wrote:
> I'm not sure that this _has_ to be an error (although there may be enough
> clients depending on it existing that it should be).

This is what the current implementation does, so not sure if clients are
depending on it.

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:173: self._cache['external_storage']
= value
On 2014/10/14 16:55:19, jbudorick wrote:
> I like the idea of caching things like this and certain system properties --
we
> do both of these already in AndroidCommands -- but let's talk about the
> implementation.

Acknowledged.

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:333: def _NewRunShellImpl(self, cmd,
cwd=None, env=None, as_root=False,
On 2014/10/14 16:55:19, jbudorick wrote:
> I'm not sure about eliminating check_return here. I think the goal is to
> eventually switch to default True, but I don't think we want to get rid of it.

I would argue that we shouldn't make it easy for clients to ignore errors. If a
non-zero exit code is to be expected, they should just catch the exception and
read the exit code from it. (And we should  make sure that the exception allows
to easily recover the exit code.) Thoughts?

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:364: if not
re.match('^[a-zA-Z_][a-zA-Z0-9_]*$', key):
On 2014/10/14 16:55:19, jbudorick wrote:
> Compile the regex as a constant and then use that to do the matching.

Acknowledged.

https://codereview.chromium.org/659533002/diff/1/build/android/pylib/device/d...
build/android/pylib/device/device_utils.py:373: cmd = '%s %s' % ('
'.join(env_quote(*kv) for kv in env.iteritems()), cmd)
On 2014/10/14 16:55:19, jbudorick wrote:
> This seems a little more readable to me:
> 
>   env_quote(k, v) for k, v in env.iteritems()
> 

Acknowledged.

[jbudorick, 2014-10-17 09:04:54]

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/cmd_...
File build/android/pylib/cmd_helper.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/cmd_...
build/android/pylib/cmd_helper.py:25: quote = pipes.quote
I don't think I like this. Let callers use pipes.quote.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/cmd_...
build/android/pylib/cmd_helper.py:27: def dquote(s):
While I understand that you're mimicking pipes.quote, I don't think I like
breaking our style guide here by starting with a lower-case letter.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/cmd_...
build/android/pylib/cmd_helper.py:37: if not s:
Also, this function should have some unit tests.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
File build/android/pylib/device/device_errors.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_errors.py:31: class
AdbShellCommandFailedError(CommandFailedError):
This should inherit from AdbCommandFailedError.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_errors.py:36: 'adb shell command %r failed
with return code %d and output %r' %
You may want to rephrase this to distinguish the exception message from the
content, i.e., quote the command, return code, and output (or some other
delineator).

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:32: _VALID_SHELL_VARIABLE =
re.compile('^[a-zA-Z_][a-zA-Z0-9_]*$')
Not sure about having this at this scope; it should probably be a constant in
DeviceUtils.

(The other two are out here because they're used by top level functions. All
other constants in this file are defined within DeviceUtils.)

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:133: try:
Sneaking a conversion in here, I see.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:192: if 'external_storage' in
self._cache:
Another sneaky conversion.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:230:
self.old_interface.RunShellCommand('dumpsys wifi')):
What's up with this? Is this because of the unit test?

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:329: want_lines: A boolean indicating
whether to return the output of the
After looking at this and thinking about it, I'm not crazy about this. I'm not
convinced that an entire string _should_ be the new behavior.

Even if we do want to go that way, we shouldn't attempt to do so in this CL. The
switch to AdbWrapper.Shell has sweeping effects on our code as it is.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:348: try:
This implementation all should move into the _Impl function. The _only_ thing in
here should be the call to _RunShellCommandImpl. The only point of the _Impl
split is to allow calling functions that are already running in their own
timeout/retry thread to avoid spawning another one.

Splitting the functions beyond that will make it more challenging to revisit the
decorator implementation to do something smarter. At the moment, we can revise
the decorator, remove the _Impl functions, and change all calls to _Impl
functions to whatever smarter thing we come up with.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:359: def _RunShellCommandImpl(self,
cmd, cwd=None, env=None, as_root=False):
You'll have to restore the options you split between the two functions.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:370: return '%s=%s' % (key,
cmd_helper.dquote(value))
Maybe the dquote functionality should just live inside env_quote? I don't
anticipate us using it anywhere else...

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:373: cmd = '
'.join(cmd_helper.quote(s) for s in cmd)
I really don't like the 'quote = pipes.quote' in practice.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:376: if env is not None:
As below, this change only matters if env is empty, and I don't think we want to
do the below in that case.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:379: if cwd is not None:
The only time when this change matters is if cwd is the empty string, and I
don't think we want to add the cwd logic in that case.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:563:
self._RunShellCommandImpl(['mkdir', '-p', d])
following from my comments on the _Impl split, restore check_return=True here.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:603: ['realpath',
device_path]).rstrip()
and here.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:695: as_root=True,
and here.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:839: self._RunShellCommandImpl(cmd,
as_root=as_root)
and here.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
File build/android/pylib/device/device_utils_test.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils_test.py:229: def __repr__(self):
- Why are you defining __repr__?
- Why is __repr__ so different from __str__?

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils_test.py:298: def
assertShellCallSequence(self, calls):
I'm torn on these functions.

The result is great, but they're not at all extensible to non-shell calls. I'm
going to have to think about these and get back to you.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils_test.py:1335: with
self.assertShellCallSequence([
This is what I mean when I say the result looks great -- this is a big
improvement over what we had before, although that's partially due to the fact
that we're mocking at a different level now.

[perezju, 2014-10-17 11:17:09]

Replying to comments. Will upload a new CL soon...

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/cmd_...
File build/android/pylib/cmd_helper.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/cmd_...
build/android/pylib/cmd_helper.py:27: def dquote(s):
On 2014/10/17 09:04:53, jbudorick wrote:
> While I understand that you're mimicking pipes.quote, I don't think I like
> breaking our style guide here by starting with a lower-case letter.

Done.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/cmd_...
build/android/pylib/cmd_helper.py:37: if not s:
On 2014/10/17 09:04:52, jbudorick wrote:
> Also, this function should have some unit tests.

Done.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
File build/android/pylib/device/device_errors.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_errors.py:31: class
AdbShellCommandFailedError(CommandFailedError):
On 2014/10/17 09:04:53, jbudorick wrote:
> This should inherit from AdbCommandFailedError.

Done.

The new version will output something like:

AdbShellCommandFailedError: (device: 0123456789abcdef) adb command 'shell'
failed with message: command 'ls /root' on device failed with return code 1 and
output 'opendir failed, Permission denied\r\n'

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_errors.py:36: 'adb shell command %r failed
with return code %d and output %r' %
On 2014/10/17 09:04:53, jbudorick wrote:
> You may want to rephrase this to distinguish the exception message from the
> content, i.e., quote the command, return code, and output (or some other
> delineator).

That's what %r does :)

And the return code should be a number, so %d

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:32: _VALID_SHELL_VARIABLE =
re.compile('^[a-zA-Z_][a-zA-Z0-9_]*$')
On 2014/10/17 09:04:53, jbudorick wrote:
> Not sure about having this at this scope; it should probably be a constant in
> DeviceUtils.
> 
> (The other two are out here because they're used by top level functions. All
> other constants in this file are defined within DeviceUtils.)

Done.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:230:
self.old_interface.RunShellCommand('dumpsys wifi')):
On 2014/10/17 09:04:53, jbudorick wrote:
> What's up with this? Is this because of the unit test?

Yes, because all the others are calls to old_interface, I had to also move back
this call or try writing some DeviceUtilsHybridImplTest with mixed calls to
run_command.RunCommand and adb.Shell. :S

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:348: try:
On 2014/10/17 09:04:53, jbudorick wrote:
> This implementation all should move into the _Impl function. The _only_ thing
in
> here should be the call to _RunShellCommandImpl. The only point of the _Impl
> split is to allow calling functions that are already running in their own
> timeout/retry thread to avoid spawning another one.
> 
> Splitting the functions beyond that will make it more challenging to revisit
the
> decorator implementation to do something smarter. At the moment, we can revise
> the decorator, remove the _Impl functions, and change all calls to _Impl
> functions to whatever smarter thing we come up with.

Done.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:376: if env is not None:
On 2014/10/17 09:04:53, jbudorick wrote:
> As below, this change only matters if env is empty, and I don't think we want
to
> do the below in that case.

Acknowledged.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:563:
self._RunShellCommandImpl(['mkdir', '-p', d])
On 2014/10/17 09:04:53, jbudorick wrote:
> following from my comments on the _Impl split, restore check_return=True here.

All Done.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
File build/android/pylib/device/device_utils_test.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils_test.py:229: def __repr__(self):
On 2014/10/17 09:04:53, jbudorick wrote:
> - Why are you defining __repr__?
> - Why is __repr__ so different from __str__?

From python docs:

repr: For many types, this function makes an attempt to return a string that
would yield an object with the same value when passed to eval() --- this is very
useful when debugging, to show which kind of object are we dealing with!

str: a string containing a nicely printable representation of an object

In this case:

repr(Args('hello', option='world')) -> "Args('hello', option='world')"
str(Args('hello', option='world')) -> "'hello', option='world'"

The str version is useful for printing these arguments as if they were passed to
other functions, e.g. as done by the error messages for failed assertions in
MockCallSequence.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils_test.py:298: def
assertShellCallSequence(self, calls):
On 2014/10/17 09:04:53, jbudorick wrote:
> I'm torn on these functions.
> 
> The result is great, but they're not at all extensible to non-shell calls. I'm
> going to have to think about these and get back to you.

Acknowledged.

[jbudorick, 2014-10-17 15:48:01]

lgtm w/nit w.r.t. the name of "want_value"

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
File build/android/pylib/device/device_errors.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_errors.py:36: 'adb shell command %r failed
with return code %d and output %r' %
On 2014/10/17 11:17:09, perezju wrote:
> On 2014/10/17 09:04:53, jbudorick wrote:
> > You may want to rephrase this to distinguish the exception message from the
> > content, i.e., quote the command, return code, and output (or some other
> > delineator).
> 
> That's what %r does :)
> 
> And the return code should be a number, so %d

Oh, neat. I didn't know about %r and somehow read it as %s.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils.py:230:
self.old_interface.RunShellCommand('dumpsys wifi')):
On 2014/10/17 11:17:09, perezju wrote:
> On 2014/10/17 09:04:53, jbudorick wrote:
> > What's up with this? Is this because of the unit test?
> 
> Yes, because all the others are calls to old_interface, I had to also move
back
> this call or try writing some DeviceUtilsHybridImplTest with mixed calls to
> run_command.RunCommand and adb.Shell. :S

Hmm. I'm not really a fan of either of those options, but I can't think of a
reasonable alternative.

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
File build/android/pylib/device/device_utils_test.py (right):

https://codereview.chromium.org/659533002/diff/80001/build/android/pylib/devi...
build/android/pylib/device/device_utils_test.py:229: def __repr__(self):
On 2014/10/17 11:17:09, perezju wrote:
> On 2014/10/17 09:04:53, jbudorick wrote:
> > - Why are you defining __repr__?
> > - Why is __repr__ so different from __str__?
> 
> From python docs:
> 
> repr: For many types, this function makes an attempt to return a string that
> would yield an object with the same value when passed to eval() --- this is
very
> useful when debugging, to show which kind of object are we dealing with!
> 
> str: a string containing a nicely printable representation of an object
> 
> In this case:
> 
> repr(Args('hello', option='world')) -> "Args('hello', option='world')"
> str(Args('hello', option='world')) -> "'hello', option='world'"
> 
> The str version is useful for printing these arguments as if they were passed
to
> other functions, e.g. as done by the error messages for failed assertions in
> MockCallSequence.

Makes sense. I might have to start trying to write __repr__s, then :)

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/cmd...
File build/android/pylib/cmd_helper_test.py (right):

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/cmd...
build/android/pylib/cmd_helper_test.py:11: 
Add a TODO for me to make these run on the bots.

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
File build/android/pylib/device/device_errors.py (right):

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
build/android/pylib/device/device_errors.py:37: 'command %r on device failed
with return code %d and output %r'
So, to be clear, raising one of these will result in this message:

(device: 0123456789abcdef) adb command "shell" failed with message: command
"foo" on device failed with return code 1 and output "Permission denied"?

Actually, that looks pretty good.

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:327: This behaviour is consistent
with that of command runners in cmd_helper as
I guess we can do both the American and British spellings of behavior. :)

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:342: want_value: A boolean indicating
if a single line of output is expected,
I don't think I like the name "want_value" -- maybe "single_line"? -- but I'm
now ok with the functionality.

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:384: output = self.adb.Shell(cmd,
expect_rc=0)
Shell returns the output as a single string, not a list of lines? Interesting.

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:386: if check_return:
I'm definitely looking forward to getting rid of check_return and making clients
deal with the exception... some day.

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:393: if len(output) != 1:
This was a good call, I suspect we'll be able to use this in quite a few places.

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
File build/android/pylib/device/device_utils_test.py (right):

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
build/android/pylib/device/device_utils_test.py:317: def assertShellCall(self,
cmd, return_value=''):
Ok, I'm fine with this for now. We can revisit it later once we've done more
conversions.

[perezju, 2014-10-17 16:11:12]

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
File build/android/pylib/device/device_errors.py (right):

https://codereview.chromium.org/659533002/diff/120001/build/android/pylib/dev...
build/android/pylib/device/device_errors.py:37: 'command %r on device failed
with return code %d and output %r'
On 2014/10/17 15:48:00, jbudorick wrote:
> So, to be clear, raising one of these will result in this message:
> 
> (device: 0123456789abcdef) adb command "shell" failed with message: command
> "foo" on device failed with return code 1 and output "Permission denied"?
> 
> Actually, that looks pretty good.

Yes, but with single rather than double quotes. Also you would probably get
'Permission denied\r\n' which is pretty useful to debug when the output contains
non-printable chars.

[perezju, 2014-10-20 08:13:58]

The CQ bit was checked by perezju@chromium.org

[commit-bot: I haz the power, 2014-10-20 08:16:08]

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

[commit-bot: I haz the power, 2014-10-20 09:35:55]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2014-10-20 09:36:34]

Patchset 8 (id:??) landed as
https://crrev.com/ab18bcca0cc1475cf381211e595e75c1146c9115
Cr-Commit-Position: refs/heads/master@{#300237}

[perezju, 2014-10-20 13:15:38]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/666943002/ by perezju@chromium.org.

The reason for reverting is: Broke some telemetry telemetry unittests and perf
unittests.

[jochen (gone - plz use gerrit), 2014-10-20 13:20:43]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/670463002/ by jochen@chromium.org.

The reason for reverting is: Appears to break telemetry_perf_unittests.