Mus+Ash: Towards Unifying CompositorFrameSink terminology

Mus+Ash: Towards Unifying CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code
between Mus+Ash and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses
ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces
RequestSurface
4. WindowSurface/ServerWindowSurface use
MojoCompositorFrameSink interface and surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/86c4e98bfe7dbd67d30ce09b4a23d58b0a147c5a
Cr-Commit-Position: refs/heads/master@{#426851}

[Fady Samuel, 2016-10-18 12:38:29]

Description was changed from

==========
WIP: Mus+Ash: Unify CompositorFrameSink terminology

BUG=
==========

to

==========
WIP: Mus+Ash: Unify CompositorFrameSink terminology

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-10-18 12:38:49]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 12:39:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 13:28:04]

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

[commit-bot: I haz the power, 2016-10-18 13:28:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Fady Samuel, 2016-10-18 14:27:56]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 14:28:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 14:32:56]

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

[commit-bot: I haz the power, 2016-10-18 14:32:57]

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

[Fady Samuel, 2016-10-18 14:56:01]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 14:56:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 15:00:31]

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

[commit-bot: I haz the power, 2016-10-18 15:00:32]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Fady Samuel, 2016-10-18 15:05:01]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 15:05:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 15:27:34]

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

[commit-bot: I haz the power, 2016-10-18 15:27:35]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Fady Samuel, 2016-10-18 15:28:01]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 15:28:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 16:10:20]

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

[commit-bot: I haz the power, 2016-10-18 16:10:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Fady Samuel, 2016-10-18 20:54:35]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 20:55:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Fady Samuel, 2016-10-18 21:13:21]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 21:13:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 23:12:39]

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

[commit-bot: I haz the power, 2016-10-18 23:12:41]

Dry run: This issue passed the CQ dry run.

[Fady Samuel, 2016-10-19 04:27:05]

Description was changed from

==========
WIP: Mus+Ash: Unify CompositorFrameSink terminology

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Unify CompositorFrameSink terminology

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-10-19 04:29:26]

Description was changed from

==========
Mus+Ash: Unify CompositorFrameSink terminology

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Unify CompositorFrameSink terminology

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces
RequestSurface
4. WindowSurface/ServerWindowSurface use MojoCompositorFrameSink interface.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-10-19 04:33:03]

Description was changed from

==========
Mus+Ash: Unify CompositorFrameSink terminology

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces
RequestSurface
4. WindowSurface/ServerWindowSurface use MojoCompositorFrameSink interface.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Unify CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code between Mus+Ash
and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces
RequestSurface
4. WindowSurface/ServerWindowSurface use MojoCompositorFrameSink interface.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-10-19 04:34:10]

Description was changed from

==========
Mus+Ash: Unify CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code between Mus+Ash
and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces
RequestSurface
4. WindowSurface/ServerWindowSurface use MojoCompositorFrameSink interface.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Unify CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code between Mus+Ash
and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces
RequestSurface
4. WindowSurface/ServerWindowSurface use MojoCompositorFrameSink interface and
surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-10-19 04:34:23]

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

[Fady Samuel, 2016-10-19 04:34:24]

PTAL Rob!

[Fady Samuel, 2016-10-20 19:30:29]

Description was changed from

==========
Mus+Ash: Unify CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code between Mus+Ash
and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces
RequestSurface
4. WindowSurface/ServerWindowSurface use MojoCompositorFrameSink interface and
surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Towards Unifying CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code between Mus+Ash
and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces
RequestSurface
4. WindowSurface/ServerWindowSurface use MojoCompositorFrameSink interface and
surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-10-20 19:30:53]

Description was changed from

==========
Mus+Ash: Towards Unifying CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code between Mus+Ash
and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces
RequestSurface
4. WindowSurface/ServerWindowSurface use MojoCompositorFrameSink interface and
surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Towards Unifying CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code between Mus+Ash
and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses 
ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces 
RequestSurface
4. WindowSurface/ServerWindowSurface use 
MojoCompositorFrameSink interface and surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-10-20 19:31:17]

Description was changed from

==========
Mus+Ash: Towards Unifying CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code between Mus+Ash
and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses 
ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces 
RequestSurface
4. WindowSurface/ServerWindowSurface use 
MojoCompositorFrameSink interface and surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Towards Unifying CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code 
between Mus+Ash and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses 
ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces 
RequestSurface
4. WindowSurface/ServerWindowSurface use 
MojoCompositorFrameSink interface and surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[rjkroege, 2016-10-20 21:33:18]

bunch-o-nits.

https://codereview.chromium.org/2430653002/diff/140001/cc/ipc/mojo_compositor...
File cc/ipc/mojo_compositor_frame_sink.mojom (right):

https://codereview.chromium.org/2430653002/diff/140001/cc/ipc/mojo_compositor...
cc/ipc/mojo_compositor_frame_sink.mojom:20: // For successful swaps, the
implementation must call
this is top level documentation for how to use the DisplayCompositor. It is
"the" DisplayCompositor API. More better docs please.

https://codereview.chromium.org/2430653002/diff/140001/cc/ipc/mojo_compositor...
cc/ipc/mojo_compositor_frame_sink.mojom:33: DidReceiveCompositorFrameAck();
why do we have this given UBF? Let the compositor simply ask for when it wants
CFRs? This seems unnecessary.

https://codereview.chromium.org/2430653002/diff/140001/content/browser/compos...
File content/browser/compositor/mus_browser_compositor_output_surface.h (right):

https://codereview.chromium.org/2430653002/diff/140001/content/browser/compos...
content/browser/compositor/mus_browser_compositor_output_surface.h:20: class
CompositorFrameSink;
alphabetical order

https://codereview.chromium.org/2430653002/diff/140001/content/browser/compos...
content/browser/compositor/mus_browser_compositor_output_surface.h:47: //
ui::CompositorFrameSinkClient:
either it's in ui or cc. So this line is wrong?

https://codereview.chromium.org/2430653002/diff/140001/services/ui/demo/bitma...
File services/ui/demo/bitmap_uploader.cc (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/demo/bitma...
services/ui/demo/bitmap_uploader.cc:209: void
BitmapUploader::SetTreeActivationCallback(const base::Closure& callback) {}
these probably need TODOs. :-)

https://codereview.chromium.org/2430653002/diff/140001/services/ui/demo/bitma...
File services/ui/demo/bitmap_uploader.h (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/demo/bitma...
services/ui/demo/bitmap_uploader.h:61: // CompositorFrameSinkClient
implementation.
we've made being a client a bit more complicated. :-)

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
File services/ui/public/cpp/tests/test_window_tree.cc (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
services/ui/public/cpp/tests/test_window_tree.cc:89:
mojo::InterfaceRequest<cc::mojom::MojoCompositorFrameSink> surface,
does a mojom:: need to be a MojoCompositorFrameSink?

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
File services/ui/public/cpp/window_surface.cc (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
services/ui/public/cpp/window_surface.cc:55:
DCHECK(thread_checker_->CalledOnValidThread());
What are the thread rules? Why doesn't the mojo binding constraint force you to
the right thread?

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
File services/ui/public/cpp/window_surface.h (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
services/ui/public/cpp/window_surface.h:47:
mojo::InterfacePtrInfo<cc::mojom::MojoCompositorFrameSink> surface_info,
how about a using to make these types less ridiculous?

https://codereview.chromium.org/2430653002/diff/140001/services/ui/ws/server_...
File services/ui/ws/server_window_surface.cc (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/ws/server_...
services/ui/ws/server_window_surface.cc:46: void
ServerWindowSurface::SetNeedsBeginFrame(bool needs_begin_frame) {
if it works without it, then why is it needed? What's optional for clients needs
to be explained in the .mojom

https://codereview.chromium.org/2430653002/diff/140001/services/ui/ws/server_...
services/ui/ws/server_window_surface.cc:84: void
ServerWindowSurface::DidReceiveCompositorFrameAck() {
You are deleting ServerWindowSurface in a subsequent CL right?

https://codereview.chromium.org/2430653002/diff/140001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
(right):

https://codereview.chromium.org/2430653002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp:319:
// FIXME: Implement this.
TODO(fsamuel)

[Fady Samuel, 2016-10-20 23:03:51]

The CQ bit was checked by fsamuel@chromium.org to run a CQ dry run

[Fady Samuel, 2016-10-20 23:04:11]

PTAL Rob!

https://codereview.chromium.org/2430653002/diff/140001/cc/ipc/mojo_compositor...
File cc/ipc/mojo_compositor_frame_sink.mojom (right):

https://codereview.chromium.org/2430653002/diff/140001/cc/ipc/mojo_compositor...
cc/ipc/mojo_compositor_frame_sink.mojom:20: // For successful swaps, the
implementation must call
On 2016/10/20 21:33:17, rjkroege wrote:
> this is top level documentation for how to use the DisplayCompositor. It is
> "the" DisplayCompositor API. More better docs please.

Added more details.

https://codereview.chromium.org/2430653002/diff/140001/cc/ipc/mojo_compositor...
cc/ipc/mojo_compositor_frame_sink.mojom:33: DidReceiveCompositorFrameAck();
On 2016/10/20 21:33:17, rjkroege wrote:
> why do we have this given UBF? Let the compositor simply ask for when it wants
> CFRs? This seems unnecessary.
> 

Added TODO.

https://codereview.chromium.org/2430653002/diff/140001/content/browser/compos...
File content/browser/compositor/mus_browser_compositor_output_surface.h (right):

https://codereview.chromium.org/2430653002/diff/140001/content/browser/compos...
content/browser/compositor/mus_browser_compositor_output_surface.h:20: class
CompositorFrameSink;
On 2016/10/20 21:33:17, rjkroege wrote:
> alphabetical order

Done.

https://codereview.chromium.org/2430653002/diff/140001/content/browser/compos...
content/browser/compositor/mus_browser_compositor_output_surface.h:47: //
ui::CompositorFrameSinkClient:
On 2016/10/20 21:33:17, rjkroege wrote:
> either it's in ui or cc. So this line is wrong?

heh, cc:: fixed.

https://codereview.chromium.org/2430653002/diff/140001/services/ui/demo/bitma...
File services/ui/demo/bitmap_uploader.cc (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/demo/bitma...
services/ui/demo/bitmap_uploader.cc:209: void
BitmapUploader::SetTreeActivationCallback(const base::Closure& callback) {}
On 2016/10/20 21:33:18, rjkroege wrote:
> these probably need TODOs. :-)

Done.

https://codereview.chromium.org/2430653002/diff/140001/services/ui/demo/bitma...
File services/ui/demo/bitmap_uploader.h (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/demo/bitma...
services/ui/demo/bitmap_uploader.h:61: // CompositorFrameSinkClient
implementation.
On 2016/10/20 21:33:18, rjkroege wrote:
> we've made being a client a bit more complicated. :-)

But hopefully more correct once all these things are implemented.

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
File services/ui/public/cpp/tests/test_window_tree.cc (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
services/ui/public/cpp/tests/test_window_tree.cc:89:
mojo::InterfaceRequest<cc::mojom::MojoCompositorFrameSink> surface,
On 2016/10/20 21:33:18, rjkroege wrote:
> does a mojom:: need to be a MojoCompositorFrameSink?

Once all CompositorFrameSinks transition to mojo we can drop the Mojo.

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
File services/ui/public/cpp/window_surface.cc (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
services/ui/public/cpp/window_surface.cc:55:
DCHECK(thread_checker_->CalledOnValidThread());
On 2016/10/20 21:33:18, rjkroege wrote:
> What are the thread rules? Why doesn't the mojo binding constraint force you
to
> the right thread?

In use-mus-in-renderer, we create a WindowSurface on the main thread and use it
on the compositor thread IIRC. This thread_checker makes sure we don't
accidentally use it on two threads.

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
File services/ui/public/cpp/window_surface.h (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/public/cpp...
services/ui/public/cpp/window_surface.h:47:
mojo::InterfacePtrInfo<cc::mojom::MojoCompositorFrameSink> surface_info,
On 2016/10/20 21:33:18, rjkroege wrote:
> how about a using to make these types less ridiculous?

Apparently mojo autogenerates one for you. I'll do it in my other CL to avoid
creating a rebase mess.

https://codereview.chromium.org/2430653002/diff/140001/services/ui/ws/server_...
File services/ui/ws/server_window_surface.cc (right):

https://codereview.chromium.org/2430653002/diff/140001/services/ui/ws/server_...
services/ui/ws/server_window_surface.cc:46: void
ServerWindowSurface::SetNeedsBeginFrame(bool needs_begin_frame) {
On 2016/10/20 21:33:18, rjkroege wrote:
> if it works without it, then why is it needed? What's optional for clients
needs
> to be explained in the .mojom

We don't use unified BeginFrame in Mus yet. staraz@ is working on that. Each
client ticks independently currently. This will be used by staraz's patch.

https://codereview.chromium.org/2430653002/diff/140001/services/ui/ws/server_...
services/ui/ws/server_window_surface.cc:84: void
ServerWindowSurface::DidReceiveCompositorFrameAck() {
On 2016/10/20 21:33:18, rjkroege wrote:
> You are deleting ServerWindowSurface in a subsequent CL right?

Not quite. ServerWindowSurface => ServerWindowCompositorFrameSink.

https://codereview.chromium.org/2430653002/diff/140001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
(right):

https://codereview.chromium.org/2430653002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp:319:
// FIXME: Implement this.
On 2016/10/20 21:33:18, rjkroege wrote:
> TODO(fsamuel)

Done.

[commit-bot: I haz the power, 2016-10-20 23:04:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rjkroege, 2016-10-20 23:53:47]

lgtm

[Fady Samuel, 2016-10-20 23:56:06]

fsamuel@chromium.org changed reviewers:
+ jam@chromium.org, sky@chromium.org, tsepez@chromium.org

[Fady Samuel, 2016-10-20 23:56:07]

+tsepez@ for mojom
+sky@ for services/ui
+jam for the rest.

[commit-bot: I haz the power, 2016-10-21 01:06:41]

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

[commit-bot: I haz the power, 2016-10-21 01:06:42]

Dry run: This issue passed the CQ dry run.

[sky, 2016-10-21 03:18:36]

https://codereview.chromium.org/2430653002/diff/160001/services/ui/ws/window_...
File services/ui/ws/window_tree_client_unittest.cc (right):

https://codereview.chromium.org/2430653002/diff/160001/services/ui/ws/window_...
services/ui/ws/window_tree_client_unittest.cc:2149:
cc::mojom::MojoCompositorFrameSinkPtr surface_ptr;
Isn't the Mojo in 'cc::mojom::Mojo redundant with the namespace? Why not just
cc::mojom::CompositorFrameSinkPtr?

[Fady Samuel, 2016-10-21 03:25:16]

https://codereview.chromium.org/2430653002/diff/160001/services/ui/ws/window_...
File services/ui/ws/window_tree_client_unittest.cc (right):

https://codereview.chromium.org/2430653002/diff/160001/services/ui/ws/window_...
services/ui/ws/window_tree_client_unittest.cc:2149:
cc::mojom::MojoCompositorFrameSinkPtr surface_ptr;
On 2016/10/21 03:18:36, sky wrote:
> Isn't the Mojo in 'cc::mojom::Mojo redundant with the namespace? Why not just
> cc::mojom::CompositorFrameSinkPtr?

I agree it's redundant. I'll have a chat with cc folks who suggested this name
separately.

[jam, 2016-10-21 15:57:35]

On 2016/10/20 23:56:07, Fady Samuel wrote:
> +tsepez@ for mojom
> +sky@ for services/ui
> +jam for the rest.

you need an owner for webkit
since rjk already reviewed and there are two other reviewers, can you list out
which directories you want me to look at?

[sky, 2016-10-21 17:03:00]

LGTM

[Fady Samuel, 2016-10-21 17:21:13]

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

[Fady Samuel, 2016-10-21 17:21:14]

+junov@ for offscreen canvas things
+jam@ for content/ things.

[jam, 2016-10-21 17:23:06]

On 2016/10/21 17:21:14, Fady Samuel wrote:
> +junov@ for offscreen canvas things
> +jam@ for content/ things.

rs lgtm for content, i defer to others for the actual review there

[Justin Novosad, 2016-10-21 17:56:36]

lgtm for webkit

[Tom Sepez, 2016-10-21 18:00:31]

LGTM for mojom.

[Fady Samuel, 2016-10-21 18:01:18]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-10-21 18:02:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 18:35:42]

Description was changed from

==========
Mus+Ash: Towards Unifying CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code 
between Mus+Ash and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses 
ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces 
RequestSurface
4. WindowSurface/ServerWindowSurface use 
MojoCompositorFrameSink interface and surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Towards Unifying CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code 
between Mus+Ash and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses 
ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces 
RequestSurface
4. WindowSurface/ServerWindowSurface use 
MojoCompositorFrameSink interface and surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-10-21 18:35:44]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-10-21 18:42:51]

Description was changed from

==========
Mus+Ash: Towards Unifying CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code 
between Mus+Ash and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses 
ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces 
RequestSurface
4. WindowSurface/ServerWindowSurface use 
MojoCompositorFrameSink interface and surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Towards Unifying CompositorFrameSink terminology

This CL takes a few steps along the way to unify compositor code
between Mus+Ash and the browser.

1. BitmapUploader uses ui::CompositorFrameSink.
2. MusBrowserCompositorOutputSurface uses
ui::CompositorFrameSink.
3. Introduces ui::Window::RequestCompositorFrameSink that replaces
RequestSurface
4. WindowSurface/ServerWindowSurface use
MojoCompositorFrameSink interface and surface.mojom is deleted.

TODO: Merge WindowSurface and WindowCompositorFrameSink.

BUG=656975
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/86c4e98bfe7dbd67d30ce09b4a23d58b0a147c5a
Cr-Commit-Position: refs/heads/master@{#426851}
==========

[commit-bot: I haz the power, 2016-10-21 18:42:52]

Patchset 9 (id:??) landed as
https://crrev.com/86c4e98bfe7dbd67d30ce09b4a23d58b0a147c5a
Cr-Commit-Position: refs/heads/master@{#426851}