Content handler for python.

Content handler for python.

This CL includes a content handler for Python, rules to package python applications for Mojo, and an apptest (doubling as an example app) exercising python execution.

R=qsr@chromium.org, qsr

Committed: https://chromium.googlesource.com/external/mojo/+/0ccbbc18f3ad098797df0f3b23d32aa06c4b0820

[etiennej, 2015-01-07 14:37:24]

Changes:
 - Added a TODO for dynamic loading of python library
 - Use of *Inittab to import python modules, renaming of mojo.system into
mojo_system

[qsr, 2015-01-07 15:14:46]

https://codereview.chromium.org/796373006/diff/1/examples/python/BUILD.gn
File examples/python/BUILD.gn (right):

https://codereview.chromium.org/796373006/diff/1/examples/python/BUILD.gn#new...
examples/python/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All rights
reserved.
This is now wrong. (2014 -> 2015)

https://codereview.chromium.org/796373006/diff/1/examples/python/BUILD.gn#new...
examples/python/BUILD.gn:19: deps = [
order seems wrong. Does gn format solve this? Otherwise, please follow the
declaration order in https://code.google.com/p/chromium/wiki/GNStyleGuide

https://codereview.chromium.org/796373006/diff/1/examples/python/__mojo__.py
File examples/python/__mojo__.py (right):

https://codereview.chromium.org/796373006/diff/1/examples/python/__mojo__.py#...
examples/python/__mojo__.py:25: self._service_provider =
ServiceProviderImpl(provider)
This is kind of wrong. You must consider that AcceptConnection can be called
multiple time. You should keep a list of provider, and also listen for error on
provider to clean those.

https://codereview.chromium.org/796373006/diff/1/examples/python/__mojo__.py#...
examples/python/__mojo__.py:34: self.__name_to_service_connector = {}
We do not use double underscore, just use _name_to_service_connector

https://codereview.chromium.org/796373006/diff/1/examples/python/__mojo__.py#...
examples/python/__mojo__.py:40: service =
self.__name_to_service_connector[interface_name]
You might want to protect all of this with:
if interface_name in self._name_to_service_connector

https://codereview.chromium.org/796373006/diff/1/examples/python/python_appte...
File examples/python/python_apptest.cc (right):

https://codereview.chromium.org/796373006/diff/1/examples/python/python_appte...
examples/python/python_apptest.cc:19: class PythonApplicationTest : public
test::ApplicationTestBase {
Did you think about using //example/apptest instead of rewriting a new test. You
should be able to reimplement the same application, written in python, and the
same test should pass -> you would use mappings to point the tested application
to the python implementation.

https://codereview.chromium.org/796373006/diff/1/mojo/public/python/rules.gni
File mojo/public/python/rules.gni (right):

https://codereview.chromium.org/796373006/diff/1/mojo/public/python/rules.gni...
mojo/public/python/rules.gni:36: zip_inputs += [
"$dep_target_out_dir/$dep_name.zip" ]
Please use pyzip for all the python zipped package.

https://codereview.chromium.org/796373006/diff/1/mojo/public/python/rules.gni...
mojo/public/python/rules.gni:70: package_output =
"$target_out_dir/$package_name.zip"
Same thing, please use pyzip.

https://codereview.chromium.org/796373006/diff/1/mojo/python/BUILD.gn
File mojo/python/BUILD.gn (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/BUILD.gn#newcode43
mojo/python/BUILD.gn:43:
"//mojo/public/interfaces/bindings/tests:test_interfaces_python",
Is this needed?

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/c_async_waiter.pxd (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/c_async_waiter.pxd:1: # Copyright 2014 The Chromium
Authors. All rights reserved.
I do not see any change in this file -> is this some sort of git issue? Can you
reset your local changes?

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/c_base.pxd (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/c_base.pxd:14: from libc.stdint cimport int32_t,
int64_t, uint32_t, uint64_t, uintptr_t, intptr_t
Do you need all those cimports?

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/c_export.pxd (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/c_export.pxd:1: # Copyright 2014 The Chromium
Authors. All rights reserved.
Again, seems to be no change here.

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/content_handler_main.cc (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/content_handler_main.cc:52:
MOJO_CHECK(temp_dir_.CreateUniqueTempDir());
You cannot use a class member for temp_dir_, as the RunApplication can be called
multiple time for multiple application.

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/content_handler_main.cc:69: PyObject*
SetupPythonEnvironment(const std::string& application_path) {
Could you use there and below the helper scoped classes. Ref counting by hand is
tedious.

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/system_impl.pyx (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/system_impl.pyx:17: raise NotImplementedError()
I think you should not even write the method.

[etiennej, 2015-01-08 11:23:13]

https://codereview.chromium.org/796373006/diff/1/examples/python/BUILD.gn
File examples/python/BUILD.gn (right):

https://codereview.chromium.org/796373006/diff/1/examples/python/BUILD.gn#new...
examples/python/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All rights
reserved.
On 2015/01/07 15:14:45, qsr wrote:
> This is now wrong. (2014 -> 2015)

Done.

https://codereview.chromium.org/796373006/diff/1/examples/python/BUILD.gn#new...
examples/python/BUILD.gn:19: deps = [
On 2015/01/07 15:14:45, qsr wrote:
> order seems wrong. Does gn format solve this? Otherwise, please follow the
> declaration order in https://code.google.com/p/chromium/wiki/GNStyleGuide

Done. gn format does not fix that.

https://codereview.chromium.org/796373006/diff/1/examples/python/__mojo__.py
File examples/python/__mojo__.py (right):

https://codereview.chromium.org/796373006/diff/1/examples/python/__mojo__.py#...
examples/python/__mojo__.py:25: self._service_provider =
ServiceProviderImpl(provider)
On 2015/01/07 15:14:45, qsr wrote:
> This is kind of wrong. You must consider that AcceptConnection can be called
> multiple time. You should keep a list of provider, and also listen for error
on
> provider to clean those.

I'll look into it. Note that I am not planning to do a full port of
mojo/public/cpp/application here, but just have enough to demonstrate that
running an application and a service works. A full port may be useful, but
should live in mojo/public/python/application so it could be shared.

https://codereview.chromium.org/796373006/diff/1/examples/python/__mojo__.py#...
examples/python/__mojo__.py:34: self.__name_to_service_connector = {}
On 2015/01/07 15:14:45, qsr wrote:
> We do not use double underscore, just use _name_to_service_connector

Done.

https://codereview.chromium.org/796373006/diff/1/examples/python/__mojo__.py#...
examples/python/__mojo__.py:40: service =
self.__name_to_service_connector[interface_name]
On 2015/01/07 15:14:45, qsr wrote:
> You might want to protect all of this with:
> if interface_name in self._name_to_service_connector

Done.

https://codereview.chromium.org/796373006/diff/1/examples/python/python_appte...
File examples/python/python_apptest.cc (right):

https://codereview.chromium.org/796373006/diff/1/examples/python/python_appte...
examples/python/python_apptest.cc:19: class PythonApplicationTest : public
test::ApplicationTestBase {
On 2015/01/07 15:14:45, qsr wrote:
> Did you think about using //example/apptest instead of rewriting a new test.
You
> should be able to reimplement the same application, written in python, and the
> same test should pass -> you would use mappings to point the tested
application
> to the python implementation.

//example/apptest requires more support code: in particular, implementation of
binding services to clients. Given that I want to test the content handler here,
I think it would make more sense to wait until we have a proper python support
library before testing more complex cases, and merge tests.

https://codereview.chromium.org/796373006/diff/1/mojo/public/python/rules.gni
File mojo/public/python/rules.gni (right):

https://codereview.chromium.org/796373006/diff/1/mojo/public/python/rules.gni...
mojo/public/python/rules.gni:36: zip_inputs += [
"$dep_target_out_dir/$dep_name.zip" ]
On 2015/01/07 15:14:45, qsr wrote:
> Please use pyzip for all the python zipped package.

Done.

https://codereview.chromium.org/796373006/diff/1/mojo/public/python/rules.gni...
mojo/public/python/rules.gni:70: package_output =
"$target_out_dir/$package_name.zip"
On 2015/01/07 15:14:45, qsr wrote:
> Same thing, please use pyzip.

Done.

https://codereview.chromium.org/796373006/diff/1/mojo/python/BUILD.gn
File mojo/python/BUILD.gn (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/BUILD.gn#newcode43
mojo/python/BUILD.gn:43:
"//mojo/public/interfaces/bindings/tests:test_interfaces_python",
On 2015/01/07 15:14:45, qsr wrote:
> Is this needed?

Done.

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/c_async_waiter.pxd (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/c_async_waiter.pxd:1: # Copyright 2014 The Chromium
Authors. All rights reserved.
On 2015/01/07 15:14:46, qsr wrote:
> I do not see any change in this file -> is this some sort of git issue? Can
you
> reset your local changes?

Git issue: this is a new file branched from
mojo/public/python/c_async_waiter.pxd. I didn't find a proper way to use only
one file at both locations.

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/c_base.pxd (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/c_base.pxd:14: from libc.stdint cimport int32_t,
int64_t, uint32_t, uint64_t, uintptr_t, intptr_t
On 2015/01/07 15:14:46, qsr wrote:
> Do you need all those cimports?

Done.

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/c_export.pxd (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/c_export.pxd:1: # Copyright 2014 The Chromium
Authors. All rights reserved.
On 2015/01/07 15:14:46, qsr wrote:
> Again, seems to be no change here.

See my other comment.

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/content_handler_main.cc (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/content_handler_main.cc:52:
MOJO_CHECK(temp_dir_.CreateUniqueTempDir());
On 2015/01/07 15:14:46, qsr wrote:
> You cannot use a class member for temp_dir_, as the RunApplication can be
called
> multiple time for multiple application.

Done.

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/content_handler_main.cc:69: PyObject*
SetupPythonEnvironment(const std::string& application_path) {
On 2015/01/07 15:14:46, qsr wrote:
> Could you use there and below the helper scoped classes. Ref counting by hand
is
> tedious.

I am not sure what you mean by "helper scoped classes".

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/system_impl.pyx (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/system_impl.pyx:17: raise NotImplementedError()
On 2015/01/07 15:14:46, qsr wrote:
> I think you should not even write the method.

Done.

[etiennej, 2015-01-08 17:14:29]

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
File mojo/python/content_handler/content_handler_main.cc (right):

https://codereview.chromium.org/796373006/diff/1/mojo/python/content_handler/...
mojo/python/content_handler/content_handler_main.cc:69: PyObject*
SetupPythonEnvironment(const std::string& application_path) {
On 2015/01/08 11:23:13, etiennej wrote:
> On 2015/01/07 15:14:46, qsr wrote:
> > Could you use there and below the helper scoped classes. Ref counting by
hand
> is
> > tedious.
> 
> I am not sure what you mean by "helper scoped classes".

Done.

[qsr, 2015-01-12 17:37:45]

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
File examples/python/__mojo__.py (right):

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
examples/python/__mojo__.py:10: import example_service_mojom
Alphabetize

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
examples/python/__mojo__.py:12: import mojo_bindings.reflection
This is (luckily) not used. We do not really want anyone using this directly.

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
examples/python/__mojo__.py:26:
service_provider.AddService("mojo::ExampleService",
Isn't this available somewhere from the example_service_mojom.ExampleService
manager? If not, this should be added, if yes, you can change the signature or
this to only take the class.

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
examples/python/__mojo__.py:43: service.manager.Bind(service(), pipe)
Maybe some logging if the class is not registered.

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
examples/python/__mojo__.py:46: class
PyServiceImpl(example_service_mojom.ExampleService):
What about calling this ExampleServiceImpl or something like this.

https://codereview.chromium.org/796373006/diff/100001/mojo/public/python/rule...
File mojo/public/python/rules.gni (right):

https://codereview.chromium.org/796373006/diff/100001/mojo/public/python/rule...
mojo/public/python/rules.gni:67: # python_package targets.
I wonder if we need mojom_deps. With your changes, a mojom target will generate
a $dep_name.pyzip file at the right location, so it should work as a simple
deps, and you do not need to differenciate those.

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
File mojo/python/content_handler/c_async_waiter.pxd (right):

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
mojo/python/content_handler/c_async_waiter.pxd:2: # Use of this source code is
governed by a BSD-style license that can be
Didn't had time to look into this. We need to find a solution for this before
pushing this. I really do not want to have copy pasted files :(

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
File mojo/python/content_handler/content_handler_main.cc (right):

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
mojo/python/content_handler/content_handler_main.cc:141: // py_input reference
is stolen by py_arguments
Please use a ScopedPyRfe for py_input, and just add a release method to
ScopedPyRef, so that the ownership is always clear.

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
File mojo/python/content_handler/python_system_impl_helper.h (right):

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
mojo/python/content_handler/python_system_impl_helper.h:18: namespace python {
Could you add a ban line here.

https://codereview.chromium.org/796373006/diff/100001/mojo/tools/data/apptests
File mojo/tools/data/apptests (right):

https://codereview.chromium.org/796373006/diff/100001/mojo/tools/data/apptest...
mojo/tools/data/apptests:26: "test": "mojo:example_apptests",
I guess we need name somewhere here, but this is not on the scope for this CL.

[etiennej, 2015-01-14 00:54:04]

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
File examples/python/__mojo__.py (right):

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
examples/python/__mojo__.py:10: import example_service_mojom
On 2015/01/12 17:37:45, qsr wrote:
> Alphabetize

Done.

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
examples/python/__mojo__.py:12: import mojo_bindings.reflection
On 2015/01/12 17:37:44, qsr wrote:
> This is (luckily) not used. We do not really want anyone using this directly.

Done.

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
examples/python/__mojo__.py:26:
service_provider.AddService("mojo::ExampleService",
On 2015/01/12 17:37:45, qsr wrote:
> Isn't this available somewhere from the example_service_mojom.ExampleService
> manager? If not, this should be added, if yes, you can change the signature or
> this to only take the class.

Done.

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
examples/python/__mojo__.py:43: service.manager.Bind(service(), pipe)
On 2015/01/12 17:37:45, qsr wrote:
> Maybe some logging if the class is not registered.

Done.

https://codereview.chromium.org/796373006/diff/100001/examples/python/__mojo_...
examples/python/__mojo__.py:46: class
PyServiceImpl(example_service_mojom.ExampleService):
On 2015/01/12 17:37:44, qsr wrote:
> What about calling this ExampleServiceImpl or something like this. 

Done.

https://codereview.chromium.org/796373006/diff/100001/mojo/public/python/rule...
File mojo/public/python/rules.gni (right):

https://codereview.chromium.org/796373006/diff/100001/mojo/public/python/rule...
mojo/public/python/rules.gni:67: # python_package targets.
On 2015/01/12 17:37:45, qsr wrote:
> I wonder if we need mojom_deps. With your changes, a mojom target will
generate
> a $dep_name.pyzip file at the right location, so it should work as a simple
> deps, and you do not need to differenciate those.

Done.

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
File mojo/python/content_handler/c_async_waiter.pxd (right):

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
mojo/python/content_handler/c_async_waiter.pxd:2: # Use of this source code is
governed by a BSD-style license that can be
On 2015/01/12 17:37:45, qsr wrote:
> Didn't had time to look into this. We need to find a solution for this before
> pushing this. I really do not want to have copy pasted files :(

Done.

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
File mojo/python/content_handler/content_handler_main.cc (right):

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
mojo/python/content_handler/content_handler_main.cc:141: // py_input reference
is stolen by py_arguments
On 2015/01/12 17:37:45, qsr wrote:
> Please use a ScopedPyRfe for py_input, and just add a release method to
> ScopedPyRef, so that the ownership is always clear.

Done.

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
File mojo/python/content_handler/python_system_impl_helper.h (right):

https://codereview.chromium.org/796373006/diff/100001/mojo/python/content_han...
mojo/python/content_handler/python_system_impl_helper.h:18: namespace python {
On 2015/01/12 17:37:45, qsr wrote:
> Could you add a ban line here.

Done.

https://codereview.chromium.org/796373006/diff/100001/mojo/tools/data/apptests
File mojo/tools/data/apptests (right):

https://codereview.chromium.org/796373006/diff/100001/mojo/tools/data/apptest...
mojo/tools/data/apptests:26: "test": "mojo:example_apptests",
On 2015/01/12 17:37:45, qsr wrote:
> I guess we need name somewhere here, but this is not on the scope for this CL.

Acknowledged.

[qsr, 2015-01-14 18:08:42]

https://codereview.chromium.org/796373006/diff/140001/examples/python/__mojo_...
File examples/python/__mojo__.py (right):

https://codereview.chromium.org/796373006/diff/140001/examples/python/__mojo_...
examples/python/__mojo__.py:27:
service_provider.AddService(ExampleServiceImpl.manager.name,
You do not need to pass 2 parameters here, the class is enough.

https://codereview.chromium.org/796373006/diff/140001/mojo/public/python/mojo...
File mojo/public/python/mojo_bindings/reflection.py (right):

https://codereview.chromium.org/796373006/diff/140001/mojo/public/python/mojo...
mojo/public/python/mojo_bindings/reflection.py:185: normalized_name =
descriptor['normalized_name']
I would use fully_qualified_name instead of normalized.

https://codereview.chromium.org/796373006/diff/140001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/generators/mojom_python_generator.py (right):

https://codereview.chromium.org/796373006/diff/140001/mojo/public/tools/bindi...
mojo/public/tools/bindings/generators/mojom_python_generator.py:335:
interface.normalized_name = self.GetNormalizedName(interface)
Try to not modify the object if not needed. Either export the method, or do the
computation in the template: See the java template, you can do:
{{namespace|replace(".","::")}}::{{interface.name}}

[etiennej, 2015-01-15 00:03:07]

https://codereview.chromium.org/796373006/diff/140001/examples/python/__mojo_...
File examples/python/__mojo__.py (right):

https://codereview.chromium.org/796373006/diff/140001/examples/python/__mojo_...
examples/python/__mojo__.py:27:
service_provider.AddService(ExampleServiceImpl.manager.name,
On 2015/01/14 18:08:41, qsr wrote:
> You do not need to pass 2 parameters here, the class is enough.

Done.

https://codereview.chromium.org/796373006/diff/140001/mojo/public/python/mojo...
File mojo/public/python/mojo_bindings/reflection.py (right):

https://codereview.chromium.org/796373006/diff/140001/mojo/public/python/mojo...
mojo/public/python/mojo_bindings/reflection.py:185: normalized_name =
descriptor['normalized_name']
On 2015/01/14 18:08:41, qsr wrote:
> I would use fully_qualified_name instead of normalized.

Done.

https://codereview.chromium.org/796373006/diff/140001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/generators/mojom_python_generator.py (right):

https://codereview.chromium.org/796373006/diff/140001/mojo/public/tools/bindi...
mojo/public/tools/bindings/generators/mojom_python_generator.py:335:
interface.normalized_name = self.GetNormalizedName(interface)
On 2015/01/14 18:08:41, qsr wrote:
> Try to not modify the object if not needed. Either export the method, or do
the
> computation in the template: See the java template, you can do:
> {{namespace|replace(".","::")}}::{{interface.name}}

Done.

[qsr, 2015-01-15 20:47:33]

LGTM with nits.

https://codereview.chromium.org/796373006/diff/180001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/generators/mojom_python_generator.py (right):

https://codereview.chromium.org/796373006/diff/180001/mojo/public/tools/bindi...
mojo/public/tools/bindings/generators/mojom_python_generator.py:314: def
GetGlobals(self):
Do you need Globas? Can you put this in GeneratePythonModule? I think that
should work.

[etiennej, 2015-01-15 20:48:45]

https://codereview.chromium.org/796373006/diff/180001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/generators/mojom_python_generator.py (right):

https://codereview.chromium.org/796373006/diff/180001/mojo/public/tools/bindi...
mojo/public/tools/bindings/generators/mojom_python_generator.py:314: def
GetGlobals(self):
On 2015/01/15 20:47:33, qsr wrote:
> Do you need Globas? Can you put this in GeneratePythonModule? I think that
> should work.

I used GetGlobals to do the same as in mojom_java_generator.py. I'll look into
GeneratePythonModule.

[qsr, 2015-01-15 20:49:43]

On 2015/01/15 20:48:45, etiennej wrote:
>
https://codereview.chromium.org/796373006/diff/180001/mojo/public/tools/bindi...
> File mojo/public/tools/bindings/generators/mojom_python_generator.py (right):
> 
>
https://codereview.chromium.org/796373006/diff/180001/mojo/public/tools/bindi...
> mojo/public/tools/bindings/generators/mojom_python_generator.py:314: def
> GetGlobals(self):
> On 2015/01/15 20:47:33, qsr wrote:
> > Do you need Globas? Can you put this in GeneratePythonModule? I think that
> > should work.
> 
> I used GetGlobals to do the same as in mojom_java_generator.py. I'll look into
> GeneratePythonModule.

 It was needed for java because it is used in imported template file that do not
have access to the context, but this is not the case for python.

[etiennej, 2015-01-16 00:38:44]

https://codereview.chromium.org/796373006/diff/180001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/generators/mojom_python_generator.py (right):

https://codereview.chromium.org/796373006/diff/180001/mojo/public/tools/bindi...
mojo/public/tools/bindings/generators/mojom_python_generator.py:314: def
GetGlobals(self):
On 2015/01/15 20:48:45, etiennej wrote:
> On 2015/01/15 20:47:33, qsr wrote:
> > Do you need Globas? Can you put this in GeneratePythonModule? I think that
> > should work.
> 
> I used GetGlobals to do the same as in mojom_java_generator.py. I'll look into
> GeneratePythonModule.

Done.

[etiennej, 2015-01-16 00:43:30]

Committed patchset #11 (id:200001) manually as
0ccbbc18f3ad098797df0f3b23d32aa06c4b0820 (presubmit successful).

[jamesr, 2015-01-16 18:56:13]

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

[jamesr, 2015-01-16 18:56:13]

No try runs?  :(

This appears to be making app_tests time out on the linux rel and asan bots:

http://build.chromium.org/p/client.mojo/builders/Mojo%20Linux%20ASan/builds/443
http://build.chromium.org/p/client.mojo/builders/Mojo%20Linux/builds/1037