Mus+Ash: Unified BeginFrame Skeleton

Mus+Ash: Unified BeginFrame Skeleton

This CL implements a skeleton ExternalBeginFrameSource in Mus+Ash.

The following are the notable changes in this patch:

1. MojoCompositorFrameSinkClient gets an OnBeginFrame method.

2. A MojoCompositorFrameSinkPrivate interface is introduced with
methods to add and remove child FrameSinkIds for BeginFrame
propagation.

3. ServerWindowCOmpositorFrameSink implements MojoCompositorFrameSinkPrivate.

4. ServerWindowCompositorFrameSinkManager no longer retains direct
ownership of ServerWindowCompositorFrameSink, but rather creates a
strong binding to the MojoCompositorFrameSinkPrivate messagepipe and
holds one end of the pipe. In a future CL,
ServerWindowCompositorFrameSinkManager will no longer create
ServerWindowCompositorFrameSink directly but through a mojo interface
on DisplayCompositor.

5. All ServerWindowCompositorFrameSinks have the root window's frame
sink as their BeginFrame parent. This is temporary until Surface ID
propagation details are all worked out. This will not work properly on
multi-monitor yet.

6. ScheduleWindowPaint is gone throughout the window server. Instead,
for now, FrameGenerator always requests a BeginFrame and calls
DrawWindowTree when it receives a BeginFrame.

7. ServerWindowCompositorFrameSink no longer needs to know about
ServerWindow or ServerWindowCompositorFrameSinkManager which takes us a
step closer to the mus-ws/mus-gpu split.

8. WindowCompositorFrameSink holds an ExternalBeginFrameSource that is
updated from Mus.

BUG=653895
TBR=junov@chromium.org for trivial offscreen canvas change.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/a6ce960a00e4b8bf5608d51b6d95eb4963271887
Cr-Commit-Position: refs/heads/master@{#430504}

[Fady Samuel, 2016-11-03 17:14:22]

Description was changed from

==========
Mus+Ash: The Beginnings of Unified BeginFrame

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

to

==========
Mus+Ash: The Beginnings of Unified BeginFrame

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

[Fady Samuel, 2016-11-03 21:45:16]

Description was changed from

==========
Mus+Ash: The Beginnings of Unified BeginFrame

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

to

==========
Mus+Ash: Unified BeginFrame Skeleton

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

[Fady Samuel, 2016-11-03 21:54:34]

Description was changed from

==========
Mus+Ash: Unified BeginFrame Skeleton

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

to

==========
Mus+Ash: Unified BeginFrame Skeleton

This CL implements a skeleton ExternalBeginFrameSource in Mus+Ash.

The following are the notable changes in this patch:

1. MojoCompositorFrameSinkClient gets an OnBeginFrame method.

2. A MojoCompositorFrameSinkPrivate interface is introduced with 
methods to add and remove child FrameSinkIds for BeginFrame 
propagation.

3. ServerWindowCOmpositorFrameSink implements MojoCompositorFrameSinkPrivate.

3. ServerWindowCompositorFrameSinkManager no longer retains direct ownership of
ServerWindowCompositorFrameSink, but rather creates a strong binding to the
MojoCompositorFrameSinkPrivate messagepipe and 
holds one end of the pipe. In a future CL, 
ServerWindowCompositorFrameSinkManager will no longer create 
ServerWindowCompositorFrameSink directly but through a mojo interface 
on DisplayCompositor.

4. All ServerWindowCompositorFrameSinks have the root window's frame sink as
their BeginFrame parent. This is temporary until Surface ID propagation details
are all worked out. This will not work properly on multi-monitor yet.

5. ScheduleWindowPaint is gone throughout the window server. Instead, for now,
FrameGenerator always requests a BeginFrame and calls 
DrawWindowTree when it receives a BeginFrame.

6. ServerWindowCompositorFrameSink no longer needs to know about ServerWindow or
ServerWindowCompositorFrameSinkManager which takes us a step closer to the
mus-ws/mus-gpu split.

7. WindowCompositorFrameSink holds an ExternalBeginFrameSource that is 
updated from Mus.

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

[Fady Samuel, 2016-11-03 21:55:27]

Description was changed from

==========
Mus+Ash: Unified BeginFrame Skeleton

This CL implements a skeleton ExternalBeginFrameSource in Mus+Ash.

The following are the notable changes in this patch:

1. MojoCompositorFrameSinkClient gets an OnBeginFrame method.

2. A MojoCompositorFrameSinkPrivate interface is introduced with 
methods to add and remove child FrameSinkIds for BeginFrame 
propagation.

3. ServerWindowCOmpositorFrameSink implements MojoCompositorFrameSinkPrivate.

3. ServerWindowCompositorFrameSinkManager no longer retains direct ownership of
ServerWindowCompositorFrameSink, but rather creates a strong binding to the
MojoCompositorFrameSinkPrivate messagepipe and 
holds one end of the pipe. In a future CL, 
ServerWindowCompositorFrameSinkManager will no longer create 
ServerWindowCompositorFrameSink directly but through a mojo interface 
on DisplayCompositor.

4. All ServerWindowCompositorFrameSinks have the root window's frame sink as
their BeginFrame parent. This is temporary until Surface ID propagation details
are all worked out. This will not work properly on multi-monitor yet.

5. ScheduleWindowPaint is gone throughout the window server. Instead, for now,
FrameGenerator always requests a BeginFrame and calls 
DrawWindowTree when it receives a BeginFrame.

6. ServerWindowCompositorFrameSink no longer needs to know about ServerWindow or
ServerWindowCompositorFrameSinkManager which takes us a step closer to the
mus-ws/mus-gpu split.

7. WindowCompositorFrameSink holds an ExternalBeginFrameSource that is 
updated from Mus.

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

to

==========
Mus+Ash: Unified BeginFrame Skeleton

This CL implements a skeleton ExternalBeginFrameSource in Mus+Ash.

The following are the notable changes in this patch:

1. MojoCompositorFrameSinkClient gets an OnBeginFrame method.

2. A MojoCompositorFrameSinkPrivate interface is introduced with 
methods to add and remove child FrameSinkIds for BeginFrame 
propagation.

3. ServerWindowCOmpositorFrameSink implements MojoCompositorFrameSinkPrivate.

4. ServerWindowCompositorFrameSinkManager no longer retains direct 
ownership of ServerWindowCompositorFrameSink, but rather creates a 
strong binding to the MojoCompositorFrameSinkPrivate messagepipe and 
holds one end of the pipe. In a future CL, 
ServerWindowCompositorFrameSinkManager will no longer create 
ServerWindowCompositorFrameSink directly but through a mojo interface 
on DisplayCompositor.

5. All ServerWindowCompositorFrameSinks have the root window's frame 
sink as their BeginFrame parent. This is temporary until Surface ID 
propagation details are all worked out. This will not work properly on 
multi-monitor yet.

6. ScheduleWindowPaint is gone throughout the window server. Instead, 
for now, FrameGenerator always requests a BeginFrame and calls 
DrawWindowTree when it receives a BeginFrame.

7. ServerWindowCompositorFrameSink no longer needs to know about 
ServerWindow or ServerWindowCompositorFrameSinkManager which takes us a 
step closer to the mus-ws/mus-gpu split.

8. WindowCompositorFrameSink holds an ExternalBeginFrameSource that is 
updated from Mus.

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

[Fady Samuel, 2016-11-03 22:01:26]

fsamuel@chromium.org changed reviewers:
+ enne@chromium.org, rjkroege@chromium.org, sadrul@chromium.org,
tsepez@chromium.org

[Fady Samuel, 2016-11-03 22:01:27]

+enne@ for cc
+rjkroege@ for services/ui
+sadrul@ for ui/aura.
+tsepez@ for ipc.

[Tom Sepez, 2016-11-03 22:25:52]

https://codereview.chromium.org/2474113002/diff/40001/cc/ipc/mojo_compositor_...
File cc/ipc/mojo_compositor_frame_sink.mojom (right):

https://codereview.chromium.org/2474113002/diff/40001/cc/ipc/mojo_compositor_...
cc/ipc/mojo_compositor_frame_sink.mojom:55: AddChildFrameSink(FrameSinkId
child_frame_sink_id);
Remind me, are these guessable?  Do we care if they aren't?  Thanks.

[Fady Samuel, 2016-11-03 22:27:29]

PTAL Tom

https://codereview.chromium.org/2474113002/diff/40001/cc/ipc/mojo_compositor_...
File cc/ipc/mojo_compositor_frame_sink.mojom (right):

https://codereview.chromium.org/2474113002/diff/40001/cc/ipc/mojo_compositor_...
cc/ipc/mojo_compositor_frame_sink.mojom:55: AddChildFrameSink(FrameSinkId
child_frame_sink_id);
On 2016/11/03 22:25:52, Tom Sepez wrote:
> Remind me, are these guessable?  Do we care if they aren't?  Thanks.

These are guessable, yes. I don't think we care because this is a private
interface usable only by a privileged process (window server/browser).

[Tom Sepez, 2016-11-03 22:31:34]

OK, LGTM.  Thanks for the explanation.

[Fady Samuel, 2016-11-03 23:58:41]

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

[commit-bot: I haz the power, 2016-11-04 00:00:00]

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-11-04 00:05:56]

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

[commit-bot: I haz the power, 2016-11-04 00:05:57]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Fady Samuel, 2016-11-04 00:26:36]

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

[commit-bot: I haz the power, 2016-11-04 00:27:48]

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-11-04 00:43:36]

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

[Fady Samuel, 2016-11-04 00:43:36]

+junov@ for offscreen canvas.

[commit-bot: I haz the power, 2016-11-04 04:19:57]

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

[commit-bot: I haz the power, 2016-11-04 04:19:58]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[rjkroege, 2016-11-05 00:59:50]

lgtm

https://codereview.chromium.org/2474113002/diff/60001/cc/ipc/begin_frame_args...
File cc/ipc/begin_frame_args_struct_traits.h (right):

https://codereview.chromium.org/2474113002/diff/60001/cc/ipc/begin_frame_args...
cc/ipc/begin_frame_args_struct_traits.h:8: #include
"cc/ipc/begin_frame_args.mojom-shared.h"
when did we decide to use - in file names? I must have archived email too
enthusiastically.

https://codereview.chromium.org/2474113002/diff/60001/services/ui/public/cpp/...
File services/ui/public/cpp/window_compositor_frame_sink.cc (right):

https://codereview.chromium.org/2474113002/diff/60001/services/ui/public/cpp/...
services/ui/public/cpp/window_compositor_frame_sink.cc:93: void
WindowCompositorFrameSink::OnBeginFrame(
this is client library code? How does change with the aura-as-client?

https://codereview.chromium.org/2474113002/diff/60001/services/ui/ws/server_w...
File services/ui/ws/server_window_compositor_frame_sink_manager.cc (right):

https://codereview.chromium.org/2474113002/diff/60001/services/ui/ws/server_w...
services/ui/ws/server_window_compositor_frame_sink_manager.cc:7: #include
"base/debug/stack_trace.h"
do you need this?

[Fady Samuel, 2016-11-05 11:56:02]

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

[Fady Samuel, 2016-11-05 11:56:03]

+sky@ for services/ui

https://codereview.chromium.org/2474113002/diff/60001/cc/ipc/begin_frame_args...
File cc/ipc/begin_frame_args_struct_traits.h (right):

https://codereview.chromium.org/2474113002/diff/60001/cc/ipc/begin_frame_args...
cc/ipc/begin_frame_args_struct_traits.h:8: #include
"cc/ipc/begin_frame_args.mojom-shared.h"
On 2016/11/05 00:59:50, rjkroege wrote:
> when did we decide to use - in file names? I must have archived email too
> enthusiastically.

This was likely introduced to deal with onion soup use cases.

https://codereview.chromium.org/2474113002/diff/60001/services/ui/public/cpp/...
File services/ui/public/cpp/window_compositor_frame_sink.cc (right):

https://codereview.chromium.org/2474113002/diff/60001/services/ui/public/cpp/...
services/ui/public/cpp/window_compositor_frame_sink.cc:93: void
WindowCompositorFrameSink::OnBeginFrame(
On 2016/11/05 00:59:50, rjkroege wrote:
> this is client library code? How does change with the aura-as-client?

We have two copies of this code. I updated both.

https://codereview.chromium.org/2474113002/diff/60001/services/ui/ws/server_w...
File services/ui/ws/server_window_compositor_frame_sink_manager.cc (right):

https://codereview.chromium.org/2474113002/diff/60001/services/ui/ws/server_w...
services/ui/ws/server_window_compositor_frame_sink_manager.cc:7: #include
"base/debug/stack_trace.h"
On 2016/11/05 00:59:50, rjkroege wrote:
> do you need this?

Done.

[Fady Samuel, 2016-11-05 12:01:04]

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

[commit-bot: I haz the power, 2016-11-05 12:01: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-11-05 13:50:34]

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

[commit-bot: I haz the power, 2016-11-05 13:50:35]

Dry run: This issue passed the CQ dry run.

[sky, 2016-11-07 16:34:08]

https://codereview.chromium.org/2474113002/diff/80001/services/ui/ws/server_w...
File services/ui/ws/server_window.cc (left):

https://codereview.chromium.org/2474113002/diff/80001/services/ui/ws/server_w...
services/ui/ws/server_window.cc:291: delegate_->OnScheduleWindowPaint(this);
How does this and other functions in this file end up triggering what was
OnScheduleWindowPaint?

[Fady Samuel, 2016-11-07 16:42:02]

PTAL sky@

https://codereview.chromium.org/2474113002/diff/80001/services/ui/ws/server_w...
File services/ui/ws/server_window.cc (left):

https://codereview.chromium.org/2474113002/diff/80001/services/ui/ws/server_w...
services/ui/ws/server_window.cc:291: delegate_->OnScheduleWindowPaint(this);
On 2016/11/07 16:34:08, sky wrote:
> How does this and other functions in this file end up triggering what was
> OnScheduleWindowPaint?

In the short term, FrameGenerator now polls the state on ServerWindows once
every 16 ms. Once the window manager embeds surface IDs, the window manager will
tick whenever it changes or something it embeds gets a new surface ID.

[sky, 2016-11-07 17:20:01]

Ok, LGTM

[vmpstr, 2016-11-07 17:37:56]

vmpstr@chromium.org changed reviewers:
+ vmpstr@chromium.org

[vmpstr, 2016-11-07 17:37:57]

cc lgtm

[Fady Samuel, 2016-11-07 17:42:06]

Description was changed from

==========
Mus+Ash: Unified BeginFrame Skeleton

This CL implements a skeleton ExternalBeginFrameSource in Mus+Ash.

The following are the notable changes in this patch:

1. MojoCompositorFrameSinkClient gets an OnBeginFrame method.

2. A MojoCompositorFrameSinkPrivate interface is introduced with 
methods to add and remove child FrameSinkIds for BeginFrame 
propagation.

3. ServerWindowCOmpositorFrameSink implements MojoCompositorFrameSinkPrivate.

4. ServerWindowCompositorFrameSinkManager no longer retains direct 
ownership of ServerWindowCompositorFrameSink, but rather creates a 
strong binding to the MojoCompositorFrameSinkPrivate messagepipe and 
holds one end of the pipe. In a future CL, 
ServerWindowCompositorFrameSinkManager will no longer create 
ServerWindowCompositorFrameSink directly but through a mojo interface 
on DisplayCompositor.

5. All ServerWindowCompositorFrameSinks have the root window's frame 
sink as their BeginFrame parent. This is temporary until Surface ID 
propagation details are all worked out. This will not work properly on 
multi-monitor yet.

6. ScheduleWindowPaint is gone throughout the window server. Instead, 
for now, FrameGenerator always requests a BeginFrame and calls 
DrawWindowTree when it receives a BeginFrame.

7. ServerWindowCompositorFrameSink no longer needs to know about 
ServerWindow or ServerWindowCompositorFrameSinkManager which takes us a 
step closer to the mus-ws/mus-gpu split.

8. WindowCompositorFrameSink holds an ExternalBeginFrameSource that is 
updated from Mus.

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

to

==========
Mus+Ash: Unified BeginFrame Skeleton

This CL implements a skeleton ExternalBeginFrameSource in Mus+Ash.

The following are the notable changes in this patch:

1. MojoCompositorFrameSinkClient gets an OnBeginFrame method.

2. A MojoCompositorFrameSinkPrivate interface is introduced with 
methods to add and remove child FrameSinkIds for BeginFrame 
propagation.

3. ServerWindowCOmpositorFrameSink implements MojoCompositorFrameSinkPrivate.

4. ServerWindowCompositorFrameSinkManager no longer retains direct 
ownership of ServerWindowCompositorFrameSink, but rather creates a 
strong binding to the MojoCompositorFrameSinkPrivate messagepipe and 
holds one end of the pipe. In a future CL, 
ServerWindowCompositorFrameSinkManager will no longer create 
ServerWindowCompositorFrameSink directly but through a mojo interface 
on DisplayCompositor.

5. All ServerWindowCompositorFrameSinks have the root window's frame 
sink as their BeginFrame parent. This is temporary until Surface ID 
propagation details are all worked out. This will not work properly on 
multi-monitor yet.

6. ScheduleWindowPaint is gone throughout the window server. Instead, 
for now, FrameGenerator always requests a BeginFrame and calls 
DrawWindowTree when it receives a BeginFrame.

7. ServerWindowCompositorFrameSink no longer needs to know about 
ServerWindow or ServerWindowCompositorFrameSinkManager which takes us a 
step closer to the mus-ws/mus-gpu split.

8. WindowCompositorFrameSink holds an ExternalBeginFrameSource that is 
updated from Mus.

BUG=653895
TBR=junov@chromium.org for trivial offscreen canvas change.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-11-07 17:42:15]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-11-07 17:42:16]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
rjkroege@chromium.org
Link to the patchset: https://codereview.chromium.org/2474113002/#ps80001
(title: "Removed unnecessary include")

[commit-bot: I haz the power, 2016-11-07 17:42:39]

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-11-07 21:13:27]

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

[commit-bot: I haz the power, 2016-11-07 21:13:28]

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

[Fady Samuel, 2016-11-07 21:41:59]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-11-07 21:42:38]

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-11-07 23:37:44]

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

[commit-bot: I haz the power, 2016-11-07 23:37:47]

Failed to apply patch for services/ui/ws/frame_generator.cc:
While running git apply --index -3 -p1;
  error: patch failed: services/ui/ws/frame_generator.cc:79
  Falling back to three-way merge...
  Applied patch to 'services/ui/ws/frame_generator.cc' with conflicts.
  U services/ui/ws/frame_generator.cc

Patch:       services/ui/ws/frame_generator.cc
Index: services/ui/ws/frame_generator.cc
diff --git a/services/ui/ws/frame_generator.cc
b/services/ui/ws/frame_generator.cc
index
c239fb04b26f3a48abd79c1cb60bea3595258662..252218ef7e77bdd1c8c42f2b3b1a5af0d129a015
100644
--- a/services/ui/ws/frame_generator.cc
+++ b/services/ui/ws/frame_generator.cc
@@ -30,7 +30,6 @@ FrameGenerator::FrameGenerator(FrameGeneratorDelegate*
delegate,
           WindowIdToTransportId(root_window->id()),
           static_cast<uint32_t>(mojom::CompositorFrameSinkType::DEFAULT)),
       root_window_(root_window),
-      draw_timer_(false, false),
       binding_(this),
       weak_factory_(this) {
   DCHECK(delegate_);
@@ -58,16 +57,16 @@ void FrameGenerator::OnGpuChannelEstablished(
         channel->gpu_memory_buffer_manager(),
         new SurfacesContextProvider(widget_, channel), std::move(request),
         binding_.CreateInterfacePtrAndBind());
+    // TODO(fsamuel): This means we're always requesting a new BeginFrame
signal
+    // even when we don't need it. Once surface ID propagation work is done,
+    // this will not be necessary because FrameGenerator will only need a
+    // BeginFrame if the window manager changes.
+    compositor_frame_sink_->SetNeedsBeginFrame(true);
   } else {
     gpu_channel_ = std::move(channel);
   }
 }
 
-void FrameGenerator::RequestRedraw(const gfx::Rect& redraw_region) {
-  dirty_rect_.Union(redraw_region);
-  WantToDraw();
-}
-
 void FrameGenerator::OnAcceleratedWidgetAvailable(
     gfx::AcceleratedWidget widget) {
   widget_ = widget;
@@ -79,51 +78,38 @@ void FrameGenerator::OnAcceleratedWidgetAvailable(
         gpu_channel_->gpu_memory_buffer_manager(),
         new SurfacesContextProvider(widget_, std::move(gpu_channel_)),
         std::move(request), binding_.CreateInterfacePtrAndBind());
+    // TODO(fsamuel): This means we're always requesting a new BeginFrame
signal
+    // even when we don't need it. Once surface ID propagation work is done,
+    // this will not be necessary because FrameGenerator will only need a
+    // BeginFrame if the window manager changes.
+    compositor_frame_sink_->SetNeedsBeginFrame(true);
   }
 }
 
-void FrameGenerator::DidReceiveCompositorFrameAck() {
-  frame_pending_ = false;
-  if (!dirty_rect_.IsEmpty())
-    WantToDraw();
-}
-
-void FrameGenerator::ReclaimResources(
-    const cc::ReturnedResourceArray& resources) {
-  // Nothing to do here because FrameGenerator CompositorFrames don't reference
-  // any resources.
-}
-
-void FrameGenerator::WantToDraw() {
-  if (draw_timer_.IsRunning() || frame_pending_)
-    return;
-
-  // TODO(rjkroege): Use vblank to kick off Draw.
-  draw_timer_.Start(
-      FROM_HERE, base::TimeDelta(),
-      base::Bind(&FrameGenerator::Draw, weak_factory_.GetWeakPtr()));
-}
+void FrameGenerator::DidReceiveCompositorFrameAck() {}
 
-void FrameGenerator::Draw() {
+void FrameGenerator::OnBeginFrame(const cc::BeginFrameArgs& begin_frame_arags)
{
   if (!delegate_->GetRootWindow()->visible())
     return;
 
   const gfx::Rect output_rect(delegate_->GetViewportMetrics().pixel_size);
-  dirty_rect_.Intersect(output_rect);
   // TODO(fsamuel): We should add a trace for generating a top level frame.
   cc::CompositorFrame frame(GenerateCompositorFrame(output_rect));
-  if (compositor_frame_sink_) {
-    frame_pending_ = true;
+  if (compositor_frame_sink_)
     compositor_frame_sink_->SubmitCompositorFrame(std::move(frame));
-  }
-  dirty_rect_ = gfx::Rect();
+}
+
+void FrameGenerator::ReclaimResources(
+    const cc::ReturnedResourceArray& resources) {
+  // Nothing to do here because FrameGenerator CompositorFrames don't reference
+  // any resources.
 }
 
 cc::CompositorFrame FrameGenerator::GenerateCompositorFrame(
     const gfx::Rect& output_rect) {
   const cc::RenderPassId render_pass_id(1, 1);
   std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create();
-  render_pass->SetNew(render_pass_id, output_rect, dirty_rect_,
+  render_pass->SetNew(render_pass_id, output_rect, output_rect,
                       gfx::Transform());
 
   DrawWindowTree(render_pass.get(), delegate_->GetRootWindow(),
gfx::Vector2d(),
@@ -133,7 +119,7 @@ cc::CompositorFrame FrameGenerator::GenerateCompositorFrame(
   frame.render_pass_list.push_back(std::move(render_pass));
   if (delegate_->IsInHighContrastMode()) {
     std::unique_ptr<cc::RenderPass> invert_pass = cc::RenderPass::Create();
-    invert_pass->SetNew(cc::RenderPassId(2, 0), output_rect, dirty_rect_,
+    invert_pass->SetNew(cc::RenderPassId(2, 0), output_rect, output_rect,
                         gfx::Transform());
     cc::SharedQuadState* shared_state =
         invert_pass->CreateAndAppendSharedQuadState();

[Fady Samuel, 2016-11-08 01:08:26]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-11-08 01:08:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org, sky@chromium.org, tsepez@chromium.org,
vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2474113002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-11-08 01:09:06]

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-11-08 03:37:16]

Description was changed from

==========
Mus+Ash: Unified BeginFrame Skeleton

This CL implements a skeleton ExternalBeginFrameSource in Mus+Ash.

The following are the notable changes in this patch:

1. MojoCompositorFrameSinkClient gets an OnBeginFrame method.

2. A MojoCompositorFrameSinkPrivate interface is introduced with 
methods to add and remove child FrameSinkIds for BeginFrame 
propagation.

3. ServerWindowCOmpositorFrameSink implements MojoCompositorFrameSinkPrivate.

4. ServerWindowCompositorFrameSinkManager no longer retains direct 
ownership of ServerWindowCompositorFrameSink, but rather creates a 
strong binding to the MojoCompositorFrameSinkPrivate messagepipe and 
holds one end of the pipe. In a future CL, 
ServerWindowCompositorFrameSinkManager will no longer create 
ServerWindowCompositorFrameSink directly but through a mojo interface 
on DisplayCompositor.

5. All ServerWindowCompositorFrameSinks have the root window's frame 
sink as their BeginFrame parent. This is temporary until Surface ID 
propagation details are all worked out. This will not work properly on 
multi-monitor yet.

6. ScheduleWindowPaint is gone throughout the window server. Instead, 
for now, FrameGenerator always requests a BeginFrame and calls 
DrawWindowTree when it receives a BeginFrame.

7. ServerWindowCompositorFrameSink no longer needs to know about 
ServerWindow or ServerWindowCompositorFrameSinkManager which takes us a 
step closer to the mus-ws/mus-gpu split.

8. WindowCompositorFrameSink holds an ExternalBeginFrameSource that is 
updated from Mus.

BUG=653895
TBR=junov@chromium.org for trivial offscreen canvas change.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Unified BeginFrame Skeleton

This CL implements a skeleton ExternalBeginFrameSource in Mus+Ash.

The following are the notable changes in this patch:

1. MojoCompositorFrameSinkClient gets an OnBeginFrame method.

2. A MojoCompositorFrameSinkPrivate interface is introduced with 
methods to add and remove child FrameSinkIds for BeginFrame 
propagation.

3. ServerWindowCOmpositorFrameSink implements MojoCompositorFrameSinkPrivate.

4. ServerWindowCompositorFrameSinkManager no longer retains direct 
ownership of ServerWindowCompositorFrameSink, but rather creates a 
strong binding to the MojoCompositorFrameSinkPrivate messagepipe and 
holds one end of the pipe. In a future CL, 
ServerWindowCompositorFrameSinkManager will no longer create 
ServerWindowCompositorFrameSink directly but through a mojo interface 
on DisplayCompositor.

5. All ServerWindowCompositorFrameSinks have the root window's frame 
sink as their BeginFrame parent. This is temporary until Surface ID 
propagation details are all worked out. This will not work properly on 
multi-monitor yet.

6. ScheduleWindowPaint is gone throughout the window server. Instead, 
for now, FrameGenerator always requests a BeginFrame and calls 
DrawWindowTree when it receives a BeginFrame.

7. ServerWindowCompositorFrameSink no longer needs to know about 
ServerWindow or ServerWindowCompositorFrameSinkManager which takes us a 
step closer to the mus-ws/mus-gpu split.

8. WindowCompositorFrameSink holds an ExternalBeginFrameSource that is 
updated from Mus.

BUG=653895
TBR=junov@chromium.org for trivial offscreen canvas change.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-11-08 03:37:18]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-11-08 03:41:42]

Description was changed from

==========
Mus+Ash: Unified BeginFrame Skeleton

This CL implements a skeleton ExternalBeginFrameSource in Mus+Ash.

The following are the notable changes in this patch:

1. MojoCompositorFrameSinkClient gets an OnBeginFrame method.

2. A MojoCompositorFrameSinkPrivate interface is introduced with 
methods to add and remove child FrameSinkIds for BeginFrame 
propagation.

3. ServerWindowCOmpositorFrameSink implements MojoCompositorFrameSinkPrivate.

4. ServerWindowCompositorFrameSinkManager no longer retains direct 
ownership of ServerWindowCompositorFrameSink, but rather creates a 
strong binding to the MojoCompositorFrameSinkPrivate messagepipe and 
holds one end of the pipe. In a future CL, 
ServerWindowCompositorFrameSinkManager will no longer create 
ServerWindowCompositorFrameSink directly but through a mojo interface 
on DisplayCompositor.

5. All ServerWindowCompositorFrameSinks have the root window's frame 
sink as their BeginFrame parent. This is temporary until Surface ID 
propagation details are all worked out. This will not work properly on 
multi-monitor yet.

6. ScheduleWindowPaint is gone throughout the window server. Instead, 
for now, FrameGenerator always requests a BeginFrame and calls 
DrawWindowTree when it receives a BeginFrame.

7. ServerWindowCompositorFrameSink no longer needs to know about 
ServerWindow or ServerWindowCompositorFrameSinkManager which takes us a 
step closer to the mus-ws/mus-gpu split.

8. WindowCompositorFrameSink holds an ExternalBeginFrameSource that is 
updated from Mus.

BUG=653895
TBR=junov@chromium.org for trivial offscreen canvas change.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Mus+Ash: Unified BeginFrame Skeleton

This CL implements a skeleton ExternalBeginFrameSource in Mus+Ash.

The following are the notable changes in this patch:

1. MojoCompositorFrameSinkClient gets an OnBeginFrame method.

2. A MojoCompositorFrameSinkPrivate interface is introduced with
methods to add and remove child FrameSinkIds for BeginFrame
propagation.

3. ServerWindowCOmpositorFrameSink implements MojoCompositorFrameSinkPrivate.

4. ServerWindowCompositorFrameSinkManager no longer retains direct
ownership of ServerWindowCompositorFrameSink, but rather creates a
strong binding to the MojoCompositorFrameSinkPrivate messagepipe and
holds one end of the pipe. In a future CL,
ServerWindowCompositorFrameSinkManager will no longer create
ServerWindowCompositorFrameSink directly but through a mojo interface
on DisplayCompositor.

5. All ServerWindowCompositorFrameSinks have the root window's frame
sink as their BeginFrame parent. This is temporary until Surface ID
propagation details are all worked out. This will not work properly on
multi-monitor yet.

6. ScheduleWindowPaint is gone throughout the window server. Instead,
for now, FrameGenerator always requests a BeginFrame and calls
DrawWindowTree when it receives a BeginFrame.

7. ServerWindowCompositorFrameSink no longer needs to know about
ServerWindow or ServerWindowCompositorFrameSinkManager which takes us a
step closer to the mus-ws/mus-gpu split.

8. WindowCompositorFrameSink holds an ExternalBeginFrameSource that is
updated from Mus.

BUG=653895
TBR=junov@chromium.org for trivial offscreen canvas change.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/a6ce960a00e4b8bf5608d51b6d95eb4963271887
Cr-Commit-Position: refs/heads/master@{#430504}
==========

[commit-bot: I haz the power, 2016-11-08 03:41:44]

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