FrameSinkIds use to WindowIds in window server, Process ID/Route ID in renderer

FrameSinkIds are WindowIds in window server, Process ID/Route ID in renderer

This CL places routing information in FrameSinkIds. That way a new
surface can be routed to the appropriate FrameSinkObserver from the
display compositor.

In Chrome, the FrameSinkId contains the ProcessId/RoutingId.
FrameSinks that are not owned by renderers have a client_id = 0,
and the sink_id is set by AllocateFrameSinkId().

In Mus+Ash, each ServerWindowSurface gets a unique FrameSinkId.
The client_id is the WindowId, and the sink_id is the surface type.
Once the underlay surface goes away, the FrameSinkId will simply be
the window ID.

BUG=647852
TBR=junov@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[Fady Samuel, 2016-10-03 14:58:12]

Description was changed from

==========
FrameSinkIds use to WindowIds in window server, Process ID/Route ID in renderer

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

to

==========
FrameSinkIds use to WindowIds in window server, Process ID/Route ID in renderer

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Fady Samuel, 2016-10-04 02:11:36]

Description was changed from

==========
FrameSinkIds use to WindowIds in window server, Process ID/Route ID in renderer

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
FrameSinkIds are WindowIds in window server, Process ID/Route ID in renderer

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Fady Samuel, 2016-10-04 02:14:44]

Description was changed from

==========
FrameSinkIds are WindowIds in window server, Process ID/Route ID in renderer

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
FrameSinkIds are WindowIds in window server, Process ID/Route ID in renderer

This CL places routing information in FrameSinkIds. That way a new 
surface can be routed to the appropriate FrameSinkObserver from the
display compositor.

In Chrome, the FrameSinkId contains the ProcessId/RoutingId. 
FrameSinks that are not owned by renderers have a client_id = 0,
and the sink_id is set by AllocateFrameSinkId().

In Mus+Ash, each ServerWindowSurface gets a unique FrameSinkId.
The client_id is the WindowId, and the sink_id is the surface type.
Once the underlay surface goes away, the FrameSinkId will simply be
the window ID.


BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Fady Samuel, 2016-10-04 02:15:09]

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

[Fady Samuel, 2016-10-04 02:15:09]

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

[Fady Samuel, 2016-10-04 02:15:09]

+enne@ for content bits.
+sky@ for uni and services/ui bits.

[Fady Samuel, 2016-10-04 02:15:09]

+enne@ for content bits.
+sky@ for uni and services/ui bits.

[sky, 2016-10-04 16:47:27]

Does this mean we would expose server ids (via the surface id)? I think it would
be safer to use a separate id so that we avoid exposing the window ids.

[sky, 2016-10-04 17:03:32]

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

[sky, 2016-10-04 17:03:32]

LGTM - but this means we're exposing the unique id for windows to clients.
+tsepez for the security implications.

[Tom Sepez, 2016-10-04 17:35:10]

My recollection is that there wasn't much entropy in the unique_id (e.g. its
sequential, not a "nonce"), so that the client could already guess it.  Please
let me know if this isn't the case.

[Fady Samuel, 2016-10-04 17:39:07]

On 2016/10/04 17:35:10, Tom Sepez wrote:
> My recollection is that there wasn't much entropy in the unique_id (e.g. its
> sequential, not a "nonce"), so that the client could already guess it.  Please
> let me know if this isn't the case.

Yup, WindowIds, RenderProcessHost IDs and routing IDs are all allocated
sequentially and not random bits at all so they're all guessable.

[Tom Sepez, 2016-10-04 17:42:28]

On 2016/10/04 17:39:07, Fady Samuel wrote:
> On 2016/10/04 17:35:10, Tom Sepez wrote:
> > My recollection is that there wasn't much entropy in the unique_id (e.g. its
> > sequential, not a "nonce"), so that the client could already guess it. 
Please
> > let me know if this isn't the case.
> 
> Yup, WindowIds, RenderProcessHost IDs and routing IDs are all allocated
> sequentially and not random bits at all so they're all guessable.

LGTM then.

[sky, 2016-10-04 17:50:53]

https://codereview.chromium.org/2385193002/diff/20001/services/ui/ws/ids.h
File services/ui/ws/ids.h (right):

https://codereview.chromium.org/2385193002/diff/20001/services/ui/ws/ids.h#ne...
services/ui/ws/ids.h:56: operator uint32_t() const {
Actually, one comment. Style guide says no to implicit conversion:
https://google.github.io/styleguide/cppguide.html#Implicit_Conversions .

[enne (OOO), 2016-10-04 18:26:14]

lgtm

[Fady Samuel, 2016-10-04 19:31:57]

PTAL sky@

https://codereview.chromium.org/2385193002/diff/20001/services/ui/ws/ids.h
File services/ui/ws/ids.h (right):

https://codereview.chromium.org/2385193002/diff/20001/services/ui/ws/ids.h#ne...
services/ui/ws/ids.h:56: operator uint32_t() const {
On 2016/10/04 17:50:53, sky wrote:
> Actually, one comment. Style guide says no to implicit conversion:
> https://google.github.io/styleguide/cppguide.html#Implicit_Conversions .

Using WindowIdToTransportId instead.

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

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

[commit-bot: I haz the power, 2016-10-04 19:32:46]

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-04 19:53:25]

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

[commit-bot: I haz the power, 2016-10-04 19:53:26]

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...)

[Fady Samuel, 2016-10-04 21:37:54]

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

[Fady Samuel, 2016-10-04 21:37:55]

+jam@ for content stamp

[Fady Samuel, 2016-10-04 21:39:59]

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

[jam, 2016-10-04 21:40:30]

lgtm

[commit-bot: I haz the power, 2016-10-04 21:40:58]

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-04 22:24:24]

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

[commit-bot: I haz the power, 2016-10-04 22:24:25]

Dry run: Try jobs failed on following builders:
  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-04 22:51:47]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-10-04 22:51:48]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
sky@chromium.org, enne@chromium.org, jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2385193002/#ps80001
(title: "Make child window have unique WindowId in FrameGenerator test")

[commit-bot: I haz the power, 2016-10-04 22:52:17]

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-04 23:48:16]

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

[commit-bot: I haz the power, 2016-10-04 23:48:18]

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-10-05 03:39:59]

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

[Fady Samuel, 2016-10-05 03:40:00]

TBR'ing junov@ for sinkId plumbing into offscreencanvas.

[Fady Samuel, 2016-10-05 03:40:26]

Description was changed from

==========
FrameSinkIds are WindowIds in window server, Process ID/Route ID in renderer

This CL places routing information in FrameSinkIds. That way a new 
surface can be routed to the appropriate FrameSinkObserver from the
display compositor.

In Chrome, the FrameSinkId contains the ProcessId/RoutingId. 
FrameSinks that are not owned by renderers have a client_id = 0,
and the sink_id is set by AllocateFrameSinkId().

In Mus+Ash, each ServerWindowSurface gets a unique FrameSinkId.
The client_id is the WindowId, and the sink_id is the surface type.
Once the underlay surface goes away, the FrameSinkId will simply be
the window ID.


BUG=647852
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
FrameSinkIds are WindowIds in window server, Process ID/Route ID in renderer

This CL places routing information in FrameSinkIds. That way a new 
surface can be routed to the appropriate FrameSinkObserver from the
display compositor.

In Chrome, the FrameSinkId contains the ProcessId/RoutingId. 
FrameSinks that are not owned by renderers have a client_id = 0,
and the sink_id is set by AllocateFrameSinkId().

In Mus+Ash, each ServerWindowSurface gets a unique FrameSinkId.
The client_id is the WindowId, and the sink_id is the surface type.
Once the underlay surface goes away, the FrameSinkId will simply be
the window ID.


BUG=647852
TBR=junov@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Fady Samuel, 2016-10-05 03:40:36]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-10-05 03:40:36]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jam@chromium.org, sky@chromium.org, enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2385193002/#ps100001
(title: "Plumb SinkId into OffscreenCanvas")

[commit-bot: I haz the power, 2016-10-05 03:41:02]

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-05 04:47:04]

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

[commit-bot: I haz the power, 2016-10-05 04:47:06]

Try jobs failed on following builders:
  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-05 05:06:38]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-10-05 05:06:38]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jam@chromium.org, sky@chromium.org, enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2385193002/#ps120001
(title: "Added missing serialization")

[commit-bot: I haz the power, 2016-10-05 05:06:57]

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-05 06:29:07]

Description was changed from

==========
FrameSinkIds are WindowIds in window server, Process ID/Route ID in renderer

This CL places routing information in FrameSinkIds. That way a new 
surface can be routed to the appropriate FrameSinkObserver from the
display compositor.

In Chrome, the FrameSinkId contains the ProcessId/RoutingId. 
FrameSinks that are not owned by renderers have a client_id = 0,
and the sink_id is set by AllocateFrameSinkId().

In Mus+Ash, each ServerWindowSurface gets a unique FrameSinkId.
The client_id is the WindowId, and the sink_id is the surface type.
Once the underlay surface goes away, the FrameSinkId will simply be
the window ID.


BUG=647852
TBR=junov@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
FrameSinkIds are WindowIds in window server, Process ID/Route ID in renderer

This CL places routing information in FrameSinkIds. That way a new 
surface can be routed to the appropriate FrameSinkObserver from the
display compositor.

In Chrome, the FrameSinkId contains the ProcessId/RoutingId. 
FrameSinks that are not owned by renderers have a client_id = 0,
and the sink_id is set by AllocateFrameSinkId().

In Mus+Ash, each ServerWindowSurface gets a unique FrameSinkId.
The client_id is the WindowId, and the sink_id is the surface type.
Once the underlay surface goes away, the FrameSinkId will simply be
the window ID.


BUG=647852
TBR=junov@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-10-05 06:29:08]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-10-05 06:32:48]

Description was changed from

==========
FrameSinkIds are WindowIds in window server, Process ID/Route ID in renderer

This CL places routing information in FrameSinkIds. That way a new 
surface can be routed to the appropriate FrameSinkObserver from the
display compositor.

In Chrome, the FrameSinkId contains the ProcessId/RoutingId. 
FrameSinks that are not owned by renderers have a client_id = 0,
and the sink_id is set by AllocateFrameSinkId().

In Mus+Ash, each ServerWindowSurface gets a unique FrameSinkId.
The client_id is the WindowId, and the sink_id is the surface type.
Once the underlay surface goes away, the FrameSinkId will simply be
the window ID.


BUG=647852
TBR=junov@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
FrameSinkIds are WindowIds in window server, Process ID/Route ID in renderer

This CL places routing information in FrameSinkIds. That way a new
surface can be routed to the appropriate FrameSinkObserver from the
display compositor.

In Chrome, the FrameSinkId contains the ProcessId/RoutingId.
FrameSinks that are not owned by renderers have a client_id = 0,
and the sink_id is set by AllocateFrameSinkId().

In Mus+Ash, each ServerWindowSurface gets a unique FrameSinkId.
The client_id is the WindowId, and the sink_id is the surface type.
Once the underlay surface goes away, the FrameSinkId will simply be
the window ID.

BUG=647852
TBR=junov@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[commit-bot: I haz the power, 2016-10-05 06:32:50]

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