Add a basic NativeViewportMac for Mojo

Add a basic NativeViewportMac for Mojo

This CL adds a basic implementation of NativeViewport for Mac OS X. It puts a
window on screen and initializes GL for that window using an in-process
command buffer.

R=ben@chromium.org, ccameron@chromium.org, kbr@chromium.org

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

[abarth-chromium, 2013-11-02 08:38:24]

No need to actually review this CL, but I took a stab at implementing
NativeViewportMac.  There seems to be a bunch of things missing on Mac, which
means we probably shouldn't worry about it for a while.

[Ben Goodger (Google), 2013-11-04 19:30:09]

interesting.


On Sat, Nov 2, 2013 at 1:38 AM, <abarth@chromium.org> wrote:

> Reviewers: Ben Goodger (Google),
>
> Message:
> No need to actually review this CL, but I took a stab at implementing
> NativeViewportMac.  There seems to be a bunch of things missing on Mac,
> which
> means we probably shouldn't worry about it for a while.
>
> Description:
> Failed attempt to implement NativeViewportMac
>
> We appear to be missing a bunch of infrastructure on Mac. This CL mostly
> works
> but crashes in swap buffers.
>
> Please review this at https://codereview.chromium.org/56833002/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+93, -19 lines):
>   M mojo/mojo.gyp
>   A mojo/services/native_viewport/native_viewport_mac.mm
>   M mojo/shell/desktop/mojo_main.cc
>   M ui/gl/gl.gyp
>   M ui/gl/gl_context_nsview.h
>   M ui/gl/gl_context_nsview.mm
>   M ui/gl/gl_surface_mac.cc
>   M ui/gl/gl_surface_nsview.h
>
>
>

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

[abarth-chromium, 2013-11-05 05:44:11]

This works now.  It just needs a bit of cleanup before its ready for review.

[abarth-chromium, 2013-11-05 06:54:10]

Ok, this is ready for review.  PTAL.  Thanks!

[Ben Goodger (Google), 2013-11-05 06:57:00]

mojo lgtm, you will need to get ui/gl owners to review that dir.

https://codereview.chromium.org/56833002/diff/130001/mojo/services/native_vie...
File mojo/services/native_viewport/native_viewport_mac.mm (right):

https://codereview.chromium.org/56833002/diff/130001/mojo/services/native_vie...
mojo/services/native_viewport/native_viewport_mac.mm:30:
styleMask:NSBorderlessWindowMask
for developer convenience, we might like to make this a framed window for the
time being, at least on desktop platforms.

https://codereview.chromium.org/56833002/diff/130001/mojo/services/native_vie...
mojo/services/native_viewport/native_viewport_mac.mm:53: // TODO(beng): perform
this in response to XWindow destruction.
in response to NSWindow destruction

[abarth-chromium, 2013-11-05 07:03:24]

https://codereview.chromium.org/56833002/diff/130001/mojo/services/native_vie...
File mojo/services/native_viewport/native_viewport_mac.mm (right):

https://codereview.chromium.org/56833002/diff/130001/mojo/services/native_vie...
mojo/services/native_viewport/native_viewport_mac.mm:30:
styleMask:NSBorderlessWindowMask
On 2013/11/05 06:57:01, Ben Goodger (Google) wrote:
> for developer convenience, we might like to make this a framed window for the
> time being, at least on desktop platforms. 

Will do.

https://codereview.chromium.org/56833002/diff/130001/mojo/services/native_vie...
mojo/services/native_viewport/native_viewport_mac.mm:53: // TODO(beng): perform
this in response to XWindow destruction.
On 2013/11/05 06:57:01, Ben Goodger (Google) wrote:
> in response to NSWindow destruction

Yep.

[abarth-chromium, 2013-11-05 07:04:41]

@kbr: Would you be willing to take a look at the ui/gl changes in this CL? 
Thanks!

[Ken Russell (switch to Gerrit), 2013-11-05 19:43:30]

+ccameron for review of the Mac changes

I've sent this through the win_gpu, linux_gpu, mac_gpu and mac_gpu_retina try
bots. Let's see how it does there.

[ccameron, 2013-11-05 19:56:39]

On 2013/11/05 19:43:30, Ken Russell wrote:
> +ccameron for review of the Mac changes
> 
> I've sent this through the win_gpu, linux_gpu, mac_gpu and mac_gpu_retina try
> bots. Let's see how it does there.

The changes in ui/gl lgtm.

The big picture for mojo/ isn't clear to me, but we can cross that bridge when
we get there.

[Ken Russell (switch to Gerrit), 2013-11-05 20:24:41]

LGTM if the mac_gpu_retina job is green, and/or if it fails for unrelated
reasons (please email me if so).

[abarth-chromium, 2013-11-05 20:26:42]

On 2013/11/05 19:56:39, ccameron1 wrote:
> On 2013/11/05 19:43:30, Ken Russell wrote:
> > +ccameron for review of the Mac changes
> > 
> > I've sent this through the win_gpu, linux_gpu, mac_gpu and mac_gpu_retina
try
> > bots. Let's see how it does there.
> 
> The changes in ui/gl lgtm.

Thanks!

> The big picture for mojo/ isn't clear to me, but we can cross that bridge when
> we get there.

I sent you a doc by email that has some more information.

[abarth-chromium, 2013-11-05 21:12:41]

On 2013/11/05 20:24:41, Ken Russell wrote:
> LGTM if the mac_gpu_retina job is green, and/or if it fails for unrelated
> reasons (please email me if so).

mac_gpu_retina bot is green -> heading to CQ.

[commit-bot: I haz the power, 2013-11-05 21:24:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/56833002/150002

[commit-bot: I haz the power, 2013-11-05 23:14:29]

Retried try job too often on linux_rel for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-11-05 23:33:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/56833002/150002

[commit-bot: I haz the power, 2013-11-06 03:41:49]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-06 04:38:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/56833002/150002

[abarth-chromium, 2013-11-06 06:02:09]

Committed patchset #6 manually as r233227 (presubmit successful).