mus: Embedder can request and observe BeginFrame for embedded client.

mus: Embedder can request and observe BeginFrame for embedded client.

For telemetry tests, the browser needs to receive begin-frame messages
for the embedded renderer so that it can dispatch synthesized input
events. This CL introduces cc.mojom.FrameSinkObserver to support that.
The embedder installs the observer in GpuCompositorFrameSink through the
WindowTree (which verifies that the client requesting this is allowed to
do so).

BUG=721920
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

[sadrul, 2017-05-12 21:36:56]

Description was changed from

==========
mus: Embedder can request and observe BeginFrame for embedded client.

For telemetry tests, the browser needs to receive begin-frame messages
for the embedded renderer so that it can dispatch synthesized input
events. This CL introduces cc.mojom.FrameSinkObserver to support that.
The embedder installs the observer in MojoFrameSinkManager through the
WindowTree (which verifies that the client requesting this is allowed to
do so).

BUG=721920
==========

to

==========
mus: Embedder can request and observe BeginFrame for embedded client.

For telemetry tests, the browser needs to receive begin-frame messages
for the embedded renderer so that it can dispatch synthesized input
events. This CL introduces cc.mojom.FrameSinkObserver to support that.
The embedder installs the observer in MojoFrameSinkManager through the
WindowTree (which verifies that the client requesting this is allowed to
do so).

BUG=721920
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[sadrul, 2017-05-12 21:37:37]

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

[commit-bot: I haz the power, 2017-05-12 21:38:26]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-05-12 21:39:51]

sadrul@chromium.org changed reviewers:
+ danakj@chromium.org, fsamuel@chromium.org, sky@chromium.org

[sadrul, 2017-05-12 21:39:52]

sky@ Please review aura, ws changes.

danakj@ and fsamuel@ Please review viz changes.

[sadrul, 2017-05-12 22:19:39]

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

[sadrul, 2017-05-12 22:19:42]

Description was changed from

==========
mus: Embedder can request and observe BeginFrame for embedded client.

For telemetry tests, the browser needs to receive begin-frame messages
for the embedded renderer so that it can dispatch synthesized input
events. This CL introduces cc.mojom.FrameSinkObserver to support that.
The embedder installs the observer in MojoFrameSinkManager through the
WindowTree (which verifies that the client requesting this is allowed to
do so).

BUG=721920
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
mus: Embedder can request and observe BeginFrame for embedded client.

For telemetry tests, the browser needs to receive begin-frame messages
for the embedded renderer so that it can dispatch synthesized input
events. This CL introduces cc.mojom.FrameSinkObserver to support that.
The embedder installs the observer in GpuCompositorFrameSink through the
WindowTree (which verifies that the client requesting this is allowed to
do so).

BUG=721920
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[commit-bot: I haz the power, 2017-05-12 22:19:55]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-05-12 22:21:27]

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

[commit-bot: I haz the power, 2017-05-12 22:21:50]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-05-12 22:22:09]

After discussing offline with Fady, moved the new API into
mojom.MojoCompositorFrameSinkPrivate (instead of mojom.FrameSinkManager). PTAL
Thanks!

[Fady Samuel, 2017-05-12 22:42:55]

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

https://codereview.chromium.org/2878113002/diff/40001/cc/ipc/mojo_compositor_...
cc/ipc/mojo_compositor_frame_sink.mojom:87: RequestBeginFrame(FrameSinkObserver
observer);
nit: RequestBeginFrames

https://codereview.chromium.org/2878113002/diff/40001/components/viz/frame_si...
File components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc (right):

https://codereview.chromium.org/2878113002/diff/40001/components/viz/frame_si...
components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc:109: NOTREACHED()
<< "Stahp";
Better message?

https://codereview.chromium.org/2878113002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/2878113002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:2400:
weak_ptr_factory_.GetWeakPtr(), cc::BeginFrameArgs()));
Maybe propagate the BeginFrameArgs from viz?

https://codereview.chromium.org/2878113002/diff/40001/services/ui/public/inte...
File services/ui/public/interfaces/window_tree.mojom (right):

https://codereview.chromium.org/2878113002/diff/40001/services/ui/public/inte...
services/ui/public/interfaces/window_tree.mojom:117: RequestBeginFrame(uint32
window_id, cc.mojom.FrameSinkObserver observer);
RequestBeginFrames.

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

https://codereview.chromium.org/2878113002/diff/40001/services/ui/ws/server_w...
services/ui/ws/server_window_compositor_frame_sink_manager.cc:67:
cc::mojom::FrameSinkObserverPtr observer) {
if (!compositor_frame_sink_.is_bound()) {
  pending_compositor_frame_sink_request_ =
     mojo::MakeRequest(&compositor_frame_sink_);
}

https://codereview.chromium.org/2878113002/diff/40001/ui/aura/mus/window_tree...
File ui/aura/mus/window_tree_client.cc (right):

https://codereview.chromium.org/2878113002/diff/40001/ui/aura/mus/window_tree...
ui/aura/mus/window_tree_client.cc:901: void
WindowTreeClient::InstallBeginFrameObserver(
RequestBeginFrames?

[commit-bot: I haz the power, 2017-05-13 00:09:59]

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

[commit-bot: I haz the power, 2017-05-13 00:10:00]

Dry run: 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_...)

[sadrul, 2017-05-13 03:01:37]

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

[sadrul, 2017-05-13 03:01:38]

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

https://codereview.chromium.org/2878113002/diff/40001/cc/ipc/mojo_compositor_...
cc/ipc/mojo_compositor_frame_sink.mojom:87: RequestBeginFrame(FrameSinkObserver
observer);
On 2017/05/12 22:42:55, Fady Samuel wrote:
> nit: RequestBeginFrames

Done.

https://codereview.chromium.org/2878113002/diff/40001/components/viz/frame_si...
File components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc (right):

https://codereview.chromium.org/2878113002/diff/40001/components/viz/frame_si...
components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc:109: NOTREACHED()
<< "Stahp";
On 2017/05/12 22:42:55, Fady Samuel wrote:
> Better message?

Sorry, meant to cleanup. Removed the message here, and instead added comment
explaining why NOTREACHED()

https://codereview.chromium.org/2878113002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/2878113002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:2400:
weak_ptr_factory_.GetWeakPtr(), cc::BeginFrameArgs()));
On 2017/05/12 22:42:55, Fady Samuel wrote:
> Maybe propagate the BeginFrameArgs from viz?

I considered doing that. The reasons I didn't are (1) it's not going to be used,
yet, anyway, and (2) it's not clear to me whether this could include useful
information for this observer anyway.

https://codereview.chromium.org/2878113002/diff/40001/services/ui/public/inte...
File services/ui/public/interfaces/window_tree.mojom (right):

https://codereview.chromium.org/2878113002/diff/40001/services/ui/public/inte...
services/ui/public/interfaces/window_tree.mojom:117: RequestBeginFrame(uint32
window_id, cc.mojom.FrameSinkObserver observer);
On 2017/05/12 22:42:55, Fady Samuel wrote:
> RequestBeginFrames.

Done.

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

https://codereview.chromium.org/2878113002/diff/40001/services/ui/ws/server_w...
services/ui/ws/server_window_compositor_frame_sink_manager.cc:67:
cc::mojom::FrameSinkObserverPtr observer) {
On 2017/05/12 22:42:55, Fady Samuel wrote:
> if (!compositor_frame_sink_.is_bound()) {
>   pending_compositor_frame_sink_request_ =
>      mojo::MakeRequest(&compositor_frame_sink_);
> }

Done.

https://codereview.chromium.org/2878113002/diff/40001/ui/aura/mus/window_tree...
File ui/aura/mus/window_tree_client.cc (right):

https://codereview.chromium.org/2878113002/diff/40001/ui/aura/mus/window_tree...
ui/aura/mus/window_tree_client.cc:901: void
WindowTreeClient::InstallBeginFrameObserver(
On 2017/05/12 22:42:55, Fady Samuel wrote:
> RequestBeginFrames?

Done.

[sadrul, 2017-05-13 03:41:19]

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

[commit-bot: I haz the power, 2017-05-13 03:41:37]

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, 2017-05-13 05:38:56]

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

[commit-bot: I haz the power, 2017-05-13 05:38:57]

Dry run: This issue passed the CQ dry run.

[sky, 2017-05-14 16:26:58]

https://codereview.chromium.org/2878113002/diff/2/services/ui/public/interfac...
File services/ui/public/interfaces/window_tree.mojom (right):

https://codereview.chromium.org/2878113002/diff/2/services/ui/public/interfac...
services/ui/public/interfaces/window_tree.mojom:117: RequestBeginFrames(uint32
window_id, cc.mojom.FrameSinkObserver observer);
The browser and renderer already have a more direct communication channel. Can't
they communicate additional information over that channel rather than modifying
the ui service? In other words, why can't the renderer tell the browser about
frames directly?

[Fady Samuel, 2017-05-15 13:38:36]

On 2017/05/14 16:26:58, sky wrote:
>
https://codereview.chromium.org/2878113002/diff/2/services/ui/public/interfac...
> File services/ui/public/interfaces/window_tree.mojom (right):
> 
>
https://codereview.chromium.org/2878113002/diff/2/services/ui/public/interfac...
> services/ui/public/interfaces/window_tree.mojom:117: RequestBeginFrames(uint32
> window_id, cc.mojom.FrameSinkObserver observer);
> The browser and renderer already have a more direct communication channel.
Can't
> they communicate additional information over that channel rather than
modifying
> the ui service? In other words, why can't the renderer tell the browser about
> frames directly?

The renderer receives BeginFrames from viz. It could forward the BeginFrames to
the browser but then the browser needs to flush input back to the renderer in
response to that BeginFrame...I suppose that could work?

[sadrul, 2017-05-15 14:34:23]

On 2017/05/15 13:38:36, Fady Samuel wrote:
> On 2017/05/14 16:26:58, sky wrote:
> >
>
https://codereview.chromium.org/2878113002/diff/2/services/ui/public/interfac...
> > File services/ui/public/interfaces/window_tree.mojom (right):
> > 
> >
>
https://codereview.chromium.org/2878113002/diff/2/services/ui/public/interfac...
> > services/ui/public/interfaces/window_tree.mojom:117:
RequestBeginFrames(uint32
> > window_id, cc.mojom.FrameSinkObserver observer);
> > The browser and renderer already have a more direct communication channel.
> Can't
> > they communicate additional information over that channel rather than
> modifying
> > the ui service? In other words, why can't the renderer tell the browser
about
> > frames directly?
> 
> The renderer receives BeginFrames from viz. It could forward the BeginFrames
to
> the browser but then the browser needs to flush input back to the renderer in
> response to that BeginFrame...I suppose that could work?

The renderer does not know when there is pending event in the browser side
though. So we need the browser to still request the begin-frame for the renderer
(this CL), or we need a new mechanism for the browser to notify the renderer to
request a begin-frame from viz, which seems a bit awkward.

[danakj, 2017-05-15 15:21:23]

On Mon, May 15, 2017 at 10:34 AM, <sadrul@chromium.org> wrote:

> On 2017/05/15 13:38:36, Fady Samuel wrote:
> > On 2017/05/14 16:26:58, sky wrote:
> > >
> >
> https://codereview.chromium.org/2878113002/diff/2/
> services/ui/public/interfaces/window_tree.mojom
> > > File services/ui/public/interfaces/window_tree.mojom (right):
> > >
> > >
> >
> https://codereview.chromium.org/2878113002/diff/2/
> services/ui/public/interfaces/window_tree.mojom#newcode117
> > > services/ui/public/interfaces/window_tree.mojom:117:
> RequestBeginFrames(uint32
> > > window_id, cc.mojom.FrameSinkObserver observer);
> > > The browser and renderer already have a more direct communication
> channel.
> > Can't
> > > they communicate additional information over that channel rather than
> > modifying
> > > the ui service? In other words, why can't the renderer tell the browser
> about
> > > frames directly?
> >
> > The renderer receives BeginFrames from viz. It could forward the
> BeginFrames
> to
> > the browser but then the browser needs to flush input back to the
> renderer in
> > response to that BeginFrame...I suppose that could work?
>
> The renderer does not know when there is pending event in the browser side
> though. So we need the browser to still request the begin-frame for the
> renderer
> (this CL), or we need a new mechanism for the browser to notify the
> renderer to
> request a begin-frame from viz, which seems a bit awkward.
>

Brainstorm thot: What if the browser could ask to interrupt beginframes,
such that they round trip thru the browser before being passed to other
listeners? Or, maybe simpler, the browser gets a synchronous beginframe
before anyone else gets theirs for each frame?

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2017-05-15 15:50:30]

On 2017/05/15 15:21:23, danakj wrote:
> On Mon, May 15, 2017 at 10:34 AM, <mailto:sadrul@chromium.org> wrote:
> 
> > On 2017/05/15 13:38:36, Fady Samuel wrote:
> > > On 2017/05/14 16:26:58, sky wrote:
> > > >
> > >
> > https://codereview.chromium.org/2878113002/diff/2/
> > services/ui/public/interfaces/window_tree.mojom
> > > > File services/ui/public/interfaces/window_tree.mojom (right):
> > > >
> > > >
> > >
> > https://codereview.chromium.org/2878113002/diff/2/
> > services/ui/public/interfaces/window_tree.mojom#newcode117
> > > > services/ui/public/interfaces/window_tree.mojom:117:
> > RequestBeginFrames(uint32
> > > > window_id, cc.mojom.FrameSinkObserver observer);
> > > > The browser and renderer already have a more direct communication
> > channel.
> > > Can't
> > > > they communicate additional information over that channel rather than
> > > modifying
> > > > the ui service? In other words, why can't the renderer tell the browser
> > about
> > > > frames directly?
> > >
> > > The renderer receives BeginFrames from viz. It could forward the
> > BeginFrames
> > to
> > > the browser but then the browser needs to flush input back to the
> > renderer in
> > > response to that BeginFrame...I suppose that could work?
> >
> > The renderer does not know when there is pending event in the browser side
> > though. So we need the browser to still request the begin-frame for the
> > renderer
> > (this CL), or we need a new mechanism for the browser to notify the
> > renderer to
> > request a begin-frame from viz, which seems a bit awkward.
> >
> 
> Brainstorm thot: What if the browser could ask to interrupt beginframes,
> such that they round trip thru the browser before being passed to other
> listeners?

Do you mean similar to how input events are dispatched right now? Where browser
specifically turns a flag on in the server, so that viz sends begin-frames to
browser, and browser is responsible for routing them to the right renderer? But
that would also mean the browser has to wake up unnecessarily, right?

> Or, maybe simpler, the browser gets a synchronous beginframe
> before anyone else gets theirs for each frame?

Would this be just for browser, or for all embedders? Because ash would embed
browser, and browser would embed renderers. Having to block on ash and browser
before sending to renderer would add noticeable latency I think (in addition to
unnecessarily waking up ash and browser in the process)?

[danakj, 2017-05-15 15:55:58]

On Mon, May 15, 2017 at 11:50 AM, <sadrul@chromium.org> wrote:

> On 2017/05/15 15:21:23, danakj wrote:
>
> > On Mon, May 15, 2017 at 10:34 AM, <mailto:sadrul@chromium.org> wrote:
> >
> > > On 2017/05/15 13:38:36, Fady Samuel wrote:
> > > > On 2017/05/14 16:26:58, sky wrote:
> > > > >
> > > >
> > > https://codereview.chromium.org/2878113002/diff/2/
> > > services/ui/public/interfaces/window_tree.mojom
> > > > > File services/ui/public/interfaces/window_tree.mojom (right):
> > > > >
> > > > >
> > > >
> > > https://codereview.chromium.org/2878113002/diff/2/
> > > services/ui/public/interfaces/window_tree.mojom#newcode117
> > > > > services/ui/public/interfaces/window_tree.mojom:117:
> > > RequestBeginFrames(uint32
> > > > > window_id, cc.mojom.FrameSinkObserver observer);
> > > > > The browser and renderer already have a more direct communication
> > > channel.
> > > > Can't
> > > > > they communicate additional information over that channel rather
> than
> > > > modifying
> > > > > the ui service? In other words, why can't the renderer tell the
> browser
> > > about
> > > > > frames directly?
> > > >
> > > > The renderer receives BeginFrames from viz. It could forward the
> > > BeginFrames
> > > to
> > > > the browser but then the browser needs to flush input back to the
> > > renderer in
> > > > response to that BeginFrame...I suppose that could work?
> > >
> > > The renderer does not know when there is pending event in the browser
> side
> > > though. So we need the browser to still request the begin-frame for the
> > > renderer
> > > (this CL), or we need a new mechanism for the browser to notify the
> > > renderer to
> > > request a begin-frame from viz, which seems a bit awkward.
> > >
> >
> > Brainstorm thot: What if the browser could ask to interrupt beginframes,
> > such that they round trip thru the browser before being passed to other
> > listeners?
>
> Do you mean similar to how input events are dispatched right now? Where
> browser
> specifically turns a flag on in the server, so that viz sends begin-frames
> to
> browser, and browser is responsible for routing them to the right
> renderer? But
> that would also mean the browser has to wake up unnecessarily, right?
>

I was thinking more that viz just goes to the browser, then back to viz
where it follows the same path thereafter, but maybe that'd be racey.


>
>
> > Or, maybe simpler, the browser gets a synchronous beginframe
> > before anyone else gets theirs for each frame?
>
> Would this be just for browser, or for all embedders? Because ash would
> embed
> browser, and browser would embed renderers. Having to block on ash and
> browser
> before sending to renderer would add noticeable latency I think (in
> addition to
> unnecessarily waking up ash and browser in the process)?
>

Would ash need to do this for telemetry too? I was expecting only the
browser doing anything like this. Telemetry is a performance test, so we
should try to keep the system working as normal as possible is my overall
thought here.

Maybe we need to rethink how the test is built? Or maybe we break security
and send input thru viz for the test? idk.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2017-05-15 16:54:05]

On 2017/05/15 15:55:58, danakj wrote:
> On Mon, May 15, 2017 at 11:50 AM, <mailto:sadrul@chromium.org> wrote:
> 
> > On 2017/05/15 15:21:23, danakj wrote:
> >
> > > On Mon, May 15, 2017 at 10:34 AM, <mailto:sadrul@chromium.org> wrote:
> > >
> > > > On 2017/05/15 13:38:36, Fady Samuel wrote:
> > > > > On 2017/05/14 16:26:58, sky wrote:
> > > > > >
> > > > >
> > > > https://codereview.chromium.org/2878113002/diff/2/
> > > > services/ui/public/interfaces/window_tree.mojom
> > > > > > File services/ui/public/interfaces/window_tree.mojom (right):
> > > > > >
> > > > > >
> > > > >
> > > > https://codereview.chromium.org/2878113002/diff/2/
> > > > services/ui/public/interfaces/window_tree.mojom#newcode117
> > > > > > services/ui/public/interfaces/window_tree.mojom:117:
> > > > RequestBeginFrames(uint32
> > > > > > window_id, cc.mojom.FrameSinkObserver observer);
> > > > > > The browser and renderer already have a more direct communication
> > > > channel.
> > > > > Can't
> > > > > > they communicate additional information over that channel rather
> > than
> > > > > modifying
> > > > > > the ui service? In other words, why can't the renderer tell the
> > browser
> > > > about
> > > > > > frames directly?
> > > > >
> > > > > The renderer receives BeginFrames from viz. It could forward the
> > > > BeginFrames
> > > > to
> > > > > the browser but then the browser needs to flush input back to the
> > > > renderer in
> > > > > response to that BeginFrame...I suppose that could work?
> > > >
> > > > The renderer does not know when there is pending event in the browser
> > side
> > > > though. So we need the browser to still request the begin-frame for the
> > > > renderer
> > > > (this CL), or we need a new mechanism for the browser to notify the
> > > > renderer to
> > > > request a begin-frame from viz, which seems a bit awkward.
> > > >
> > >
> > > Brainstorm thot: What if the browser could ask to interrupt beginframes,
> > > such that they round trip thru the browser before being passed to other
> > > listeners?
> >
> > Do you mean similar to how input events are dispatched right now? Where
> > browser
> > specifically turns a flag on in the server, so that viz sends begin-frames
> > to
> > browser, and browser is responsible for routing them to the right
> > renderer? But
> > that would also mean the browser has to wake up unnecessarily, right?
> >
> 
> I was thinking more that viz just goes to the browser, then back to viz
> where it follows the same path thereafter, but maybe that'd be racey.
> 
> 
> >
> >
> > > Or, maybe simpler, the browser gets a synchronous beginframe
> > > before anyone else gets theirs for each frame?
> >
> > Would this be just for browser, or for all embedders? Because ash would
> > embed
> > browser, and browser would embed renderers. Having to block on ash and
> > browser
> > before sending to renderer would add noticeable latency I think (in
> > addition to
> > unnecessarily waking up ash and browser in the process)?
> >
> 
> Would ash need to do this for telemetry too? I was expecting only the
> browser doing anything like this. Telemetry is a performance test, so we
> should try to keep the system working as normal as possible is my overall
> thought here.
> 
> Maybe we need to rethink how the test is built? Or maybe we break security
> and send input thru viz for the test? idk.

We have talked to input-dev folks about how event-dispatch in telemetry tests
are different from what happens with real use, and whether/how we can make
things better. Some of the details are in this doc [1]. The hope/desire is to
have the telemetry tests working with the tests' current approach on cros with
mus first, which would make it easier to change the approach of the test so that
it works for both mus and non-mus cases. Does that sound reasonable?

[1]
https://docs.google.com/document/d/1_Apee7VRgTFKWt8Mq_1cEk_WMRYxwyhR_G0MgM-8H...

[danakj, 2017-05-15 17:27:50]

The doc's proposal is to not send input events based on begin frames but to
just send them at 60hz, so why do we need browser synchronization of begin
frames then?

On Mon, May 15, 2017 at 12:54 PM, <sadrul@chromium.org> wrote:

> On 2017/05/15 15:55:58, danakj wrote:
>
> > On Mon, May 15, 2017 at 11:50 AM, <mailto:sadrul@chromium.org> wrote:
> >
> > > On 2017/05/15 15:21:23, danakj wrote:
> > >
> > > > On Mon, May 15, 2017 at 10:34 AM, <mailto:sadrul@chromium.org>
> wrote:
> > > >
> > > > > On 2017/05/15 13:38:36, Fady Samuel wrote:
> > > > > > On 2017/05/14 16:26:58, sky wrote:
> > > > > > >
> > > > > >
> > > > > https://codereview.chromium.org/2878113002/diff/2/
> > > > > services/ui/public/interfaces/window_tree.mojom
> > > > > > > File services/ui/public/interfaces/window_tree.mojom (right):
> > > > > > >
> > > > > > >
> > > > > >
> > > > > https://codereview.chromium.org/2878113002/diff/2/
> > > > > services/ui/public/interfaces/window_tree.mojom#newcode117
> > > > > > > services/ui/public/interfaces/window_tree.mojom:117:
> > > > > RequestBeginFrames(uint32
> > > > > > > window_id, cc.mojom.FrameSinkObserver observer);
> > > > > > > The browser and renderer already have a more direct
> communication
> > > > > channel.
> > > > > > Can't
> > > > > > > they communicate additional information over that channel
> rather
> > > than
> > > > > > modifying
> > > > > > > the ui service? In other words, why can't the renderer tell the
> > > browser
> > > > > about
> > > > > > > frames directly?
> > > > > >
> > > > > > The renderer receives BeginFrames from viz. It could forward the
> > > > > BeginFrames
> > > > > to
> > > > > > the browser but then the browser needs to flush input back to the
> > > > > renderer in
> > > > > > response to that BeginFrame...I suppose that could work?
> > > > >
> > > > > The renderer does not know when there is pending event in the
> browser
> > > side
> > > > > though. So we need the browser to still request the begin-frame
> for the
> > > > > renderer
> > > > > (this CL), or we need a new mechanism for the browser to notify the
> > > > > renderer to
> > > > > request a begin-frame from viz, which seems a bit awkward.
> > > > >
> > > >
> > > > Brainstorm thot: What if the browser could ask to interrupt
> beginframes,
> > > > such that they round trip thru the browser before being passed to
> other
> > > > listeners?
> > >
> > > Do you mean similar to how input events are dispatched right now? Where
> > > browser
> > > specifically turns a flag on in the server, so that viz sends
> begin-frames
> > > to
> > > browser, and browser is responsible for routing them to the right
> > > renderer? But
> > > that would also mean the browser has to wake up unnecessarily, right?
> > >
> >
> > I was thinking more that viz just goes to the browser, then back to viz
> > where it follows the same path thereafter, but maybe that'd be racey.
> >
> >
> > >
> > >
> > > > Or, maybe simpler, the browser gets a synchronous beginframe
> > > > before anyone else gets theirs for each frame?
> > >
> > > Would this be just for browser, or for all embedders? Because ash would
> > > embed
> > > browser, and browser would embed renderers. Having to block on ash and
> > > browser
> > > before sending to renderer would add noticeable latency I think (in
> > > addition to
> > > unnecessarily waking up ash and browser in the process)?
> > >
> >
> > Would ash need to do this for telemetry too? I was expecting only the
> > browser doing anything like this. Telemetry is a performance test, so we
> > should try to keep the system working as normal as possible is my overall
> > thought here.
> >
> > Maybe we need to rethink how the test is built? Or maybe we break
> security
> > and send input thru viz for the test? idk.
>
> We have talked to input-dev folks about how event-dispatch in telemetry
> tests
> are different from what happens with real use, and whether/how we can make
> things better. Some of the details are in this doc [1]. The hope/desire is
> to
> have the telemetry tests working with the tests' current approach on cros
> with
> mus first, which would make it easier to change the approach of the test
> so that
> it works for both mus and non-mus cases. Does that sound reasonable?
>
> [1]
> https://docs.google.com/document/d/1_Apee7VRgTFKWt8Mq_
> 1cEk_WMRYxwyhR_G0MgM-8H9c/edit
>
> https://codereview.chromium.org/2878113002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2017-05-15 17:54:52]

On 2017/05/15 17:27:50, danakj wrote:
> The doc's proposal is to not send input events based on begin frames but to
> just send them at 60hz, so why do we need browser synchronization of begin
> frames then?

We discussed this a little in the comments [1]. I tried testing locally with
sending events at 60Hz, and it does add some noise to the data, unfortunately.
So short-term, we likely need to align with (begin-frame + offset).

[1]
https://docs.google.com/a/chromium.org/document/d/1_Apee7VRgTFKWt8Mq_1cEk_WMR...

[danakj, 2017-05-15 18:04:55]

On Mon, May 15, 2017 at 1:54 PM, <sadrul@chromium.org> wrote:

> On 2017/05/15 17:27:50, danakj wrote:
> > The doc's proposal is to not send input events based on begin frames but
> to
> > just send them at 60hz, so why do we need browser synchronization of
> begin
> > frames then?
>
> We discussed this a little in the comments [1]. I tried testing locally
> with
> sending events at 60Hz, and it does add some noise to the data,
> unfortunately.
> So short-term, we likely need to align with (begin-frame + offset).
>

What if you just get the vsync info and generate them at the same rate that
beginframes are happening, without using actual begin frames?


>
> [1]
> https://docs.google.com/a/chromium.org/document/d/1_Apee7VRgTFKWt8Mq_1cEk_
> WMRYxwyhR_G0MgM-8H9c/edit?disco=AAAABDtvJGw
>
>
>
> https://codereview.chromium.org/2878113002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2017-06-15 17:35:16]

I changed how we dispatch input events in telemetry, so it's not dependent on
begin-frames anymore. The last change landed a couple of weeks back, and seems
to have stuck. So we shouldn't need this change for mus anymore. Closing.