Teach the native viewport service to draw a Surface into itself

Teach the native viewport service to draw a Surface into itself

This teaches the native viewport service to accept a surface id from its
client and draw it into the native viewport. This won't work if the
client also tries to bind an onscreen context to the native viewport's
view id, so the surface initialization is lazy and only happens when
the client specifies a surface id. If a client does both it'll blow up.

R=sky@chromium.org

Committed: https://crrev.com/4ebfaeec388609951860700413edc800ff52cd60
Cr-Commit-Position: refs/heads/master@{#292548}

[jamesr, 2014-08-26 23:05:50]

https://codereview.chromium.org/510553002/diff/1/mojo/services/native_viewpor...
File mojo/services/native_viewport/native_viewport_impl.cc (right):

https://codereview.chromium.org/510553002/diff/1/mojo/services/native_viewpor...
mojo/services/native_viewport/native_viewport_impl.cc:134: 
the rest of this function is an unfortunately large amount of boilerplate. we'll
probably want to extract a lot of this into a common helper that view_manager
will use as well. imo it's worth doing that when we have a second caller

https://codereview.chromium.org/510553002/diff/1/mojo/services/native_viewpor...
File mojo/services/native_viewport/native_viewport_impl.h (right):

https://codereview.chromium.org/510553002/diff/1/mojo/services/native_viewpor...
mojo/services/native_viewport/native_viewport_impl.h:56: SurfacesServicePtr
surfaces_service_;
might be worth splitting all this crap out into a helper class. wdyt?

[sky, 2014-08-26 23:16:32]

https://codereview.chromium.org/510553002/diff/1/mojo/services/native_viewpor...
File mojo/services/native_viewport/native_viewport_impl.cc (right):

https://codereview.chromium.org/510553002/diff/1/mojo/services/native_viewpor...
mojo/services/native_viewport/native_viewport_impl.cc:75:
base::Unretained(this)));
Is it possible for 'this' to be deleted before the callback, resulting in
OnSurfaceConnectionCreated being called on a deleted object?

https://codereview.chromium.org/510553002/diff/1/mojo/services/native_viewpor...
File mojo/services/native_viewport/native_viewport_impl.h (right):

https://codereview.chromium.org/510553002/diff/1/mojo/services/native_viewpor...
mojo/services/native_viewport/native_viewport_impl.h:56: SurfacesServicePtr
surfaces_service_;
On 2014/08/26 23:05:50, jamesr wrote:
> might be worth splitting all this crap out into a helper class. wdyt?

Sure.

[jamesr, 2014-08-26 23:17:23]

https://codereview.chromium.org/510553002/diff/1/mojo/services/native_viewpor...
File mojo/services/native_viewport/native_viewport_impl.cc (right):

https://codereview.chromium.org/510553002/diff/1/mojo/services/native_viewpor...
mojo/services/native_viewport/native_viewport_impl.cc:75:
base::Unretained(this)));
On 2014/08/26 23:16:32, sky wrote:
> Is it possible for 'this' to be deleted before the callback, resulting in
> OnSurfaceConnectionCreated being called on a deleted object?

Oh whoops, it is.  I'll bind to a weak ptr

[jamesr, 2014-08-27 20:53:01]

PTAL.  This moves the surfaces interactions out of NativeViewportImpl into a
helper class.  The helper is a little bit complicated because we it can't do
anything useful until it has the gfx::AcceleratedWidget and it has a connection
to the surfaces service to provide an id namespace and there's no way to know
which will come back first.

[sky, 2014-08-27 21:44:14]

LGTM

https://codereview.chromium.org/510553002/diff/40001/mojo/services/native_vie...
File mojo/services/native_viewport/viewport_surface.h (right):

https://codereview.chromium.org/510553002/diff/40001/mojo/services/native_vie...
mojo/services/native_viewport/viewport_surface.h:32: void SetWidgetId(uint64_t
widget_id);
nit: no _t, the _t types are deprecated.

[jamesr, 2014-08-27 22:21:32]

On 2014/08/27 21:44:14, sky wrote:
> LGTM
> 
>
https://codereview.chromium.org/510553002/diff/40001/mojo/services/native_vie...
> File mojo/services/native_viewport/viewport_surface.h (right):
> 
>
https://codereview.chromium.org/510553002/diff/40001/mojo/services/native_vie...
> mojo/services/native_viewport/viewport_surface.h:32: void SetWidgetId(uint64_t
> widget_id);
> nit: no _t, the _t types are deprecated.

Other way around.  base/basictypes.h:

// DEPRECATED: Please use (u)int{8,16,32,64}_t instead (and include <stdint.h>).
typedef int8_t int8;
typedef uint8_t uint8;
typedef int16_t int16;
typedef int32_t int32;
typedef uint16_t uint16;
typedef uint32_t uint32;

[sky, 2014-08-27 22:50:39]

Sorry, my mistake.

On Wed, Aug 27, 2014 at 3:21 PM,  <jamesr@chromium.org> wrote:
> On 2014/08/27 21:44:14, sky wrote:
>>
>> LGTM
>
>
>
>
https://codereview.chromium.org/510553002/diff/40001/mojo/services/native_vie...
>>
>> File mojo/services/native_viewport/viewport_surface.h (right):
>
>
>
>
https://codereview.chromium.org/510553002/diff/40001/mojo/services/native_vie...
>>
>> mojo/services/native_viewport/viewport_surface.h:32: void
>> SetWidgetId(uint64_t
>> widget_id);
>> nit: no _t, the _t types are deprecated.
>
>
> Other way around.  base/basictypes.h:
>
> // DEPRECATED: Please use (u)int{8,16,32,64}_t instead (and include
> <stdint.h>).
> typedef int8_t int8;
> typedef uint8_t uint8;
> typedef int16_t int16;
> typedef int32_t int32;
> typedef uint16_t uint16;
> typedef uint32_t uint32;
>
>
> https://codereview.chromium.org/510553002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jamesr, 2014-08-27 23:55:26]

The CQ bit was checked by jamesr@chromium.org

[jamesr, 2014-08-27 23:57:45]

The CQ bit was checked by jamesr@chromium.org

[commit-bot: I haz the power, 2014-08-28 00:07:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jamesr@chromium.org/510553002/80001

[commit-bot: I haz the power, 2014-08-28 01:18:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-28 01:18:20]

Failed to commit the patch.

[jamesr, 2014-08-28 19:49:51]

The CQ bit was checked by jamesr@chromium.org

[commit-bot: I haz the power, 2014-08-28 19:51:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jamesr@chromium.org/510553002/80001

[commit-bot: I haz the power, 2014-08-28 19:54:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-28 19:54:52]

Failed to commit the patch.

[jamesr, 2014-08-28 21:56:49]

The CQ bit was checked by jamesr@chromium.org

[commit-bot: I haz the power, 2014-08-28 21:57:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jamesr@chromium.org/510553002/100001

[commit-bot: I haz the power, 2014-08-29 02:06:59]

Committed patchset #6 (id:100001) as 784be4ea464503566605b29f9e5e126dbba5c510

[commit-bot: I haz the power, 2014-09-10 03:05:17]

Patchset 6 (id:??) landed as
https://crrev.com/4ebfaeec388609951860700413edc800ff52cd60
Cr-Commit-Position: refs/heads/master@{#292548}