Use local ids for Surfaces APIs that can only apply to local surfaces

Use local ids for Surfaces APIs that can only apply to local surfaces

Surfaces identifiers have a local and a namespace component. The
namespace is particular to a connection. The local component is up to
the endpoint of the connection to manage. In contexts where a surface
that may be from anywhere is referenced, a fully qualified ID with both
the local and namespace component is needed in order to be unambiguous.
In contexts that can only apply to local surfaces only the local id is
needed.

This updates the mojo.Surface APIs that can only refer to local IDs to
only take the local component. In particular creating, destroying, or
submitting a frame can only refer to surfaces created on that connection.
References to surfaces within a frame may refer to local or foreign
surfaces so they use fully qualified IDs.

This also skips the SurfacesService indirection since many applications
can perform useful operations on a mojo.Surface interface such as create
surfaces and submit frames without knowing their ID namespace. The
namespace component is needed only to pass fully qualified IDs to other
actors that may wish to reference the produced frame.

R=sky@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/099fb6d741bdf62067d1905879302e99303eb966

[jamesr, 2015-01-13 01:29:10]

Some cleanups in preparation for de-clienting.

[sky, 2015-01-13 18:14:27]

https://codereview.chromium.org/826423008/diff/20001/examples/ganesh_app/text...
File examples/ganesh_app/texture_uploader.cc (right):

https://codereview.chromium.org/826423008/diff/20001/examples/ganesh_app/text...
examples/ganesh_app/texture_uploader.cc:45: surface_->DestroySurface(local_id_);
As the pipe is destroyed in the destructor is this call strictly necessary?

https://codereview.chromium.org/826423008/diff/20001/examples/surfaces_app/ch...
File examples/surfaces_app/child_gl_impl.cc (right):

https://codereview.chromium.org/826423008/diff/20001/examples/surfaces_app/ch...
examples/surfaces_app/child_gl_impl.cc:49: : local_id_(1u),
start_time_(base::TimeTicks::Now()), next_resource_id_(1) {
Member initialize id_namespace_ too.

https://codereview.chromium.org/826423008/diff/20001/examples/surfaces_app/ch...
File examples/surfaces_app/child_impl.cc (right):

https://codereview.chromium.org/826423008/diff/20001/examples/surfaces_app/ch...
examples/surfaces_app/child_impl.cc:77: surface_->CreateSurface(kLocalId);
I'm not familiar with this class. Is ProduceFrame only run once?

https://codereview.chromium.org/826423008/diff/20001/mojo/aura/surface_bindin...
File mojo/aura/surface_binding.cc (right):

https://codereview.chromium.org/826423008/diff/20001/mojo/aura/surface_bindin...
mojo/aura/surface_binding.cc:41: uint32_t* next_local_id);
Document who owns next_local_id.

https://codereview.chromium.org/826423008/diff/20001/mojo/services/surfaces/p...
File mojo/services/surfaces/public/interfaces/surface_id.mojom (right):

https://codereview.chromium.org/826423008/diff/20001/mojo/services/surfaces/p...
mojo/services/surfaces/public/interfaces/surface_id.mojom:13: // client. This
can be used to created, destroy and submit frames to
created->create

https://codereview.chromium.org/826423008/diff/20001/services/native_viewport...
File services/native_viewport/viewport_surface.h (right):

https://codereview.chromium.org/826423008/diff/20001/services/native_viewport...
services/native_viewport/viewport_surface.h:45: bool
gles2_bound_surface_created_;
Initialize this.

https://codereview.chromium.org/826423008/diff/20001/services/view_manager/di...
File services/view_manager/display_manager.h (right):

https://codereview.chromium.org/826423008/diff/20001/services/view_manager/di...
services/view_manager/display_manager.h:90: uint32_t local_id_;
Since local_id_ is only has two values, it seems like a boolean would be better.
Maybe did_create_surface_?

[jamesr, 2015-01-14 01:50:30]

Thanks, PTAL.

https://codereview.chromium.org/826423008/diff/20001/examples/ganesh_app/text...
File examples/ganesh_app/texture_uploader.cc (right):

https://codereview.chromium.org/826423008/diff/20001/examples/ganesh_app/text...
examples/ganesh_app/texture_uploader.cc:45: surface_->DestroySurface(local_id_);
On 2015/01/13 18:14:27, sky wrote:
> As the pipe is destroyed in the destructor is this call strictly necessary?

It's not, closing the pipe destroys all associated surfaces.  Removed.

https://codereview.chromium.org/826423008/diff/20001/examples/surfaces_app/ch...
File examples/surfaces_app/child_gl_impl.cc (right):

https://codereview.chromium.org/826423008/diff/20001/examples/surfaces_app/ch...
examples/surfaces_app/child_gl_impl.cc:49: : local_id_(1u),
start_time_(base::TimeTicks::Now()), next_resource_id_(1) {
On 2015/01/13 18:14:27, sky wrote:
> Member initialize id_namespace_ too.

Done.

https://codereview.chromium.org/826423008/diff/20001/examples/surfaces_app/ch...
File examples/surfaces_app/child_impl.cc (right):

https://codereview.chromium.org/826423008/diff/20001/examples/surfaces_app/ch...
examples/surfaces_app/child_impl.cc:77: surface_->CreateSurface(kLocalId);
On 2015/01/13 18:14:27, sky wrote:
> I'm not familiar with this class. Is ProduceFrame only run once?

That's right - this app produces exactly one frame ever and is done.

https://codereview.chromium.org/826423008/diff/20001/mojo/aura/surface_bindin...
File mojo/aura/surface_binding.cc (right):

https://codereview.chromium.org/826423008/diff/20001/mojo/aura/surface_bindin...
mojo/aura/surface_binding.cc:41: uint32_t* next_local_id);
On 2015/01/13 18:14:27, sky wrote:
> Document who owns next_local_id.

Done.

https://codereview.chromium.org/826423008/diff/20001/mojo/services/surfaces/p...
File mojo/services/surfaces/public/interfaces/surface_id.mojom (right):

https://codereview.chromium.org/826423008/diff/20001/mojo/services/surfaces/p...
mojo/services/surfaces/public/interfaces/surface_id.mojom:13: // client. This
can be used to created, destroy and submit frames to
On 2015/01/13 18:14:27, sky wrote:
> created->create

Done.

https://codereview.chromium.org/826423008/diff/20001/services/native_viewport...
File services/native_viewport/viewport_surface.h (right):

https://codereview.chromium.org/826423008/diff/20001/services/native_viewport...
services/native_viewport/viewport_surface.h:45: bool
gles2_bound_surface_created_;
On 2015/01/13 18:14:27, sky wrote:
> Initialize this.

Done.

https://codereview.chromium.org/826423008/diff/20001/services/view_manager/di...
File services/view_manager/display_manager.h (right):

https://codereview.chromium.org/826423008/diff/20001/services/view_manager/di...
services/view_manager/display_manager.h:90: uint32_t local_id_;
On 2015/01/13 18:14:27, sky wrote:
> Since local_id_ is only has two values, it seems like a boolean would be
better.
> Maybe did_create_surface_?

Hmm, yeah that'd be better.  Added surface_allocated_ since it can transition
multiple times from true->false->true over time

[sky, 2015-01-14 16:15:11]

LGTM

[jamesr, 2015-01-14 18:25:36]

Committed patchset #3 (id:40001) manually as
099fb6d741bdf62067d1905879302e99303eb966 (presubmit successful).