Add Android-backend to the memory_inspector (prerequisites).

Add Android-backend to the memory_inspector (prerequisites).

This change introduces the prerequisites for the upcoming Android
backend tracing classes of the memory_inspector tool. In particular:
 - The mock_adb binary, which will be used by the unittests to mock
   adb responses.
 - The prebuilts_fetcher.py which pulls and syncs the prebuilts from
   GCS (in a similar way to what telemetry currently does).

BUG=340294
NOTRY=true

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

[Primiano Tucci (use gerrit), 2014-02-10 16:04:55]

tonyg: I'd like to use the gs://chromium-telemetry GCS for putting the prebuilts
required by the memory_inspector (it's slowly coming to life :)). I expect those
prebuilts to be in order of hundreds of Kb and updated pretty rarely. Is it ok?

A further comment for the reviewers: I understand that prebuilts_fetcher.py can
look as a dup of the tools/telemetry/telemetry/page/cloud_storage.py script.
However:

- cloud_storage has a lot of dependencies on telemetry, and I'd like to avoid
such a dependency because conceptually the memory_inspector doesn't depend on
telemetry.
- cloud_storage uses gsutil (it downloads the gsutil.tar.gz package and installs
it before anything else) because it offers a fully fledged GCS interface
(upload, rename, oauth, ...).
- On the other side, I just need to download files from GCS, and that is luckily
as simple as a one-line
urllib.urlretrieve('https://storage.googleapis.com/bucket/object', local_file),
so it seems to me unreasonable adding all the complexity of cloud_storage to
this project.

Thanks,
Primiano

[Philippe, 2014-02-10 16:41:03]

lgtm with some really tiny comments, thanks! (assuming Tony is also OK with what
you asked)

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/m...
File tools/memory_inspector/memory_inspector/backends/prebuilts_fetcher.py
(right):

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/m...
tools/memory_inspector/memory_inspector/backends/prebuilts_fetcher.py:39: def
_IsChanged(local_file_path):
Nit: it might be slightly clearer to split this into e.g.
GetRemoteFileID(local_file_path) and IsLocalFileUpToDate(local_file_path). Up to
you though :)

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/m...
tools/memory_inspector/memory_inspector/backends/prebuilts_fetcher.py:55:
f.write("mock_prebuilt")
It's not immediately clear to me why you are writing that to the file. A comment
maybe? :)

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/t...
File tools/memory_inspector/tests/mock_adb/mock_adb.py (right):

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/t...
tools/memory_inspector/tests/mock_adb/mock_adb.py:15: Essentially this class is
used prepare a dictionary of mocked responses to adb
Nit: s/prepare/prepare to

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/t...
tools/memory_inspector/tests/mock_adb/mock_adb.py:28: f.close()
Nit: not sure this line is needed with the 'with' statement. I'm not the expert
here though :)

[Primiano Tucci (use gerrit), 2014-02-10 19:45:00]

Oops, I forgot +dai.

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/m...
File tools/memory_inspector/memory_inspector/backends/prebuilts_fetcher.py
(right):

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/m...
tools/memory_inspector/memory_inspector/backends/prebuilts_fetcher.py:39: def
_IsChanged(local_file_path):
On 2014/02/10 16:41:03, Philippe wrote:
> Nit: it might be slightly clearer to split this into e.g.
> GetRemoteFileID(local_file_path) and IsLocalFileUpToDate(local_file_path). Up
to
> you though :)
Yeah, makes sense.

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/m...
tools/memory_inspector/memory_inspector/backends/prebuilts_fetcher.py:55:
f.write("mock_prebuilt")
On 2014/02/10 16:41:03, Philippe wrote:
> It's not immediately clear to me why you are writing that to the file. A
comment
> maybe? :)
Yup, actually this should be just a truncate :)

[Primiano Tucci (use gerrit), 2014-02-11 12:10:51]

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/t...
File tools/memory_inspector/tests/mock_adb/mock_adb.py (right):

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/t...
tools/memory_inspector/tests/mock_adb/mock_adb.py:15: Essentially this class is
used prepare a dictionary of mocked responses to adb
On 2014/02/10 16:41:03, Philippe wrote:
> Nit: s/prepare/prepare to

Done.

https://codereview.chromium.org/158993002/diff/20001/tools/memory_inspector/t...
tools/memory_inspector/tests/mock_adb/mock_adb.py:28: f.close()
On 2014/02/10 16:41:03, Philippe wrote:
> Nit: not sure this line is needed with the 'with' statement. I'm not the
expert
> here though :)
I found this in some code snippets. I'm afraid that the rationale is that
os.fdopen behaves differently from open().
Not 100% sure (might be just the snippets were wrong). However I prefer to be
paranoid rather than risking of leaking a fd :)

[Primiano Tucci (use gerrit), 2014-02-13 10:12:23]

I talked with tonyg and he seems fine with using gs://chromium-telemetry for our
prebuilts.

[Primiano Tucci (use gerrit), 2014-02-13 10:14:03]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2014-02-13 10:14:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/primiano@chromium.org/158993002/250001

[commit-bot: I haz the power, 2014-02-13 16:09:39]

Change committed as 251045