SurfacelessSurfaces for Mus

SurfacelessSurfaces for Mus

Implement SurfacelessSurfaces for Mus as needed for ChromeBook/ozone
native rendering.

BUG=596549
Committed: https://crrev.com/586223331c8295e28d7cb4708f2a5abde943dc38
Cr-Commit-Position: refs/heads/master@{#386892}

[rjkroege, 2016-04-12 01:12:27]

Description was changed from

==========
SurfacelessSurfaces for Mus

Implement SurfacelessSurfaces for Mus as needed for ChromeBook/ozone
native rendering.

BUG=580596
==========

to

==========
SurfacelessSurfaces for Mus

Implement SurfacelessSurfaces for Mus as needed for ChromeBook/ozone
native rendering.

BUG=596549
==========

[rjkroege, 2016-04-12 01:12:51]

rjkroege@chromium.org changed reviewers:
+ fsamuel@chromium.org

[rjkroege, 2016-04-12 01:12:51]

ptal

[Fady Samuel, 2016-04-12 02:54:41]

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
File components/mus/surfaces/buffer_queue.cc (right):

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:149:
scoped_ptr<BufferQueue::AllocatedSurface> BufferQueue::RecreateBuffer(
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:150: scoped_ptr<AllocatedSurface>
surface) {
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:154: scoped_ptr<AllocatedSurface>
new_surface(GetNextSurface());
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:198:
scoped_ptr<BufferQueue::AllocatedSurface> BufferQueue::GetNextSurface() {
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:200: scoped_ptr<AllocatedSurface>
surface =
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:215: scoped_ptr<gfx::GpuMemoryBuffer>
buffer(
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:228: // its implementation of the
GpuControl interface. In that case, we need to
nit: weird wrapping.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:243: return make_scoped_ptr(new
AllocatedSurface(this, std::move(buffer), texture,
WrapUnique

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:249: scoped_ptr<gfx::GpuMemoryBuffer>
buffer,
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
File components/mus/surfaces/buffer_queue.h (right):

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.h:74: scoped_ptr<gfx::GpuMemoryBuffer>
buffer;
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.h:100: scoped_ptr<AllocatedSurface>
GetNextSurface();
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.h:102: scoped_ptr<AllocatedSurface>
RecreateBuffer(
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.h:103: scoped_ptr<AllocatedSurface>
surface);
nit: std::unique_ptr.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/dir...
File components/mus/surfaces/direct_output_surface_ozone.cc (right):

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/dir...
components/mus/surfaces/direct_output_surface_ozone.cc:81: // in the
GpuBrowserCompositorOutputSurface context.
nit: Better wrapping please?

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/sur...
File components/mus/surfaces/surfaces_context_provider.cc (right):

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/sur...
components/mus/surfaces/surfaces_context_provider.cc:42: if (implementation_) {
nit: no need for {

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/top...
File components/mus/surfaces/top_level_display_client.cc (right):

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/top...
components/mus/surfaces/top_level_display_client.cc:18: #include
"ui/compositor/compositor_vsync_manager.h"
Do we want this dependency on ui/compositor from mus?

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/top...
components/mus/surfaces/top_level_display_client.cc:49:
scoped_refptr<SurfacesContextProvider> scp(
nit: better variable name please.

[rjkroege, 2016-04-12 19:27:55]

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
File components/mus/surfaces/buffer_queue.cc (right):

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:149:
scoped_ptr<BufferQueue::AllocatedSurface> BufferQueue::RecreateBuffer(
On 2016/04/12 02:54:40, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:150: scoped_ptr<AllocatedSurface>
surface) {
On 2016/04/12 02:54:40, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:154: scoped_ptr<AllocatedSurface>
new_surface(GetNextSurface());
On 2016/04/12 02:54:41, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:198:
scoped_ptr<BufferQueue::AllocatedSurface> BufferQueue::GetNextSurface() {
On 2016/04/12 02:54:40, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:200: scoped_ptr<AllocatedSurface>
surface =
On 2016/04/12 02:54:40, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:215: scoped_ptr<gfx::GpuMemoryBuffer>
buffer(
On 2016/04/12 02:54:41, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:228: // its implementation of the
GpuControl interface. In that case, we need to
On 2016/04/12 02:54:40, Fady Samuel wrote:
> nit: weird wrapping.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:243: return make_scoped_ptr(new
AllocatedSurface(this, std::move(buffer), texture,
On 2016/04/12 02:54:41, Fady Samuel wrote:
> WrapUnique

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.cc:249: scoped_ptr<gfx::GpuMemoryBuffer>
buffer,
On 2016/04/12 02:54:40, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
File components/mus/surfaces/buffer_queue.h (right):

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.h:74: scoped_ptr<gfx::GpuMemoryBuffer>
buffer;
On 2016/04/12 02:54:41, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.h:100: scoped_ptr<AllocatedSurface>
GetNextSurface();
On 2016/04/12 02:54:41, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.h:102: scoped_ptr<AllocatedSurface>
RecreateBuffer(
On 2016/04/12 02:54:41, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/buf...
components/mus/surfaces/buffer_queue.h:103: scoped_ptr<AllocatedSurface>
surface);
On 2016/04/12 02:54:41, Fady Samuel wrote:
> nit: std::unique_ptr.

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/dir...
File components/mus/surfaces/direct_output_surface_ozone.cc (right):

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/dir...
components/mus/surfaces/direct_output_surface_ozone.cc:81: // in the
GpuBrowserCompositorOutputSurface context.
On 2016/04/12 02:54:41, Fady Samuel wrote:
> nit: Better wrapping please?

Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/sur...
File components/mus/surfaces/surfaces_context_provider.cc (right):

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/sur...
components/mus/surfaces/surfaces_context_provider.cc:42: if (implementation_) {
On 2016/04/12 02:54:41, Fady Samuel wrote:
> nit: no need for {

Given that's Chrome style, why doesn't pre-sub tell us? And... I think this is
an error in style. Done.

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/top...
File components/mus/surfaces/top_level_display_client.cc (right):

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/top...
components/mus/surfaces/top_level_display_client.cc:18: #include
"ui/compositor/compositor_vsync_manager.h"
On 2016/04/12 02:54:41, Fady Samuel wrote:
> Do we want this dependency on ui/compositor from mus?

no. I removed all of the actual dependency code. This was an oversight. Done

https://codereview.chromium.org/1881793002/diff/1/components/mus/surfaces/top...
components/mus/surfaces/top_level_display_client.cc:49:
scoped_refptr<SurfacesContextProvider> scp(
On 2016/04/12 02:54:41, Fady Samuel wrote:
> nit: better variable name please.

Done.

[rjkroege, 2016-04-12 19:28:16]

ptal

[Fady Samuel, 2016-04-12 19:31:52]

lgtm + nit.

https://codereview.chromium.org/1881793002/diff/20001/components/mus/surfaces...
File components/mus/surfaces/buffer_queue.cc (right):

https://codereview.chromium.org/1881793002/diff/20001/components/mus/surfaces...
components/mus/surfaces/buffer_queue.cc:227: // This ends up calling back into
the CommandBufferLocal via  its
nit: extra space between via and its

[rjkroege, 2016-04-12 19:36:25]

https://codereview.chromium.org/1881793002/diff/20001/components/mus/surfaces...
File components/mus/surfaces/buffer_queue.cc (right):

https://codereview.chromium.org/1881793002/diff/20001/components/mus/surfaces...
components/mus/surfaces/buffer_queue.cc:227: // This ends up calling back into
the CommandBufferLocal via  its
On 2016/04/12 19:31:52, Fady Samuel wrote:
> nit: extra space between via and its

Done.

[rjkroege, 2016-04-12 19:36:56]

The CQ bit was checked by rjkroege@chromium.org

[rjkroege, 2016-04-12 19:36:56]

The patchset sent to the CQ was uploaded after l-g-t-m from fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/1881793002/#ps40001
(title: "nits")

[commit-bot: I haz the power, 2016-04-12 19:37:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1881793002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1881793002/40001

[commit-bot: I haz the power, 2016-04-12 19:48:02]

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

[commit-bot: I haz the power, 2016-04-12 19:48:03]

Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rjkroege, 2016-04-12 23:39:27]

The CQ bit was checked by rjkroege@chromium.org

[rjkroege, 2016-04-12 23:39:27]

The patchset sent to the CQ was uploaded after l-g-t-m from fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/1881793002/#ps60001
(title: "fix build")

[commit-bot: I haz the power, 2016-04-12 23:39:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1881793002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1881793002/60001

[commit-bot: I haz the power, 2016-04-13 01:19:22]

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

[commit-bot: I haz the power, 2016-04-13 01:19:23]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[rjkroege, 2016-04-13 01:34:50]

The CQ bit was checked by rjkroege@chromium.org

[commit-bot: I haz the power, 2016-04-13 01:35:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1881793002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1881793002/60001

[commit-bot: I haz the power, 2016-04-13 01:43:13]

Description was changed from

==========
SurfacelessSurfaces for Mus

Implement SurfacelessSurfaces for Mus as needed for ChromeBook/ozone
native rendering.

BUG=596549
==========

to

==========
SurfacelessSurfaces for Mus

Implement SurfacelessSurfaces for Mus as needed for ChromeBook/ozone
native rendering.

BUG=596549
==========

[commit-bot: I haz the power, 2016-04-13 01:43:15]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-13 01:46:49]

Description was changed from

==========
SurfacelessSurfaces for Mus

Implement SurfacelessSurfaces for Mus as needed for ChromeBook/ozone
native rendering.

BUG=596549
==========

to

==========
SurfacelessSurfaces for Mus

Implement SurfacelessSurfaces for Mus as needed for ChromeBook/ozone
native rendering.

BUG=596549

Committed: https://crrev.com/586223331c8295e28d7cb4708f2a5abde943dc38
Cr-Commit-Position: refs/heads/master@{#386892}
==========

[commit-bot: I haz the power, 2016-04-13 01:46:50]

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