Fixes bug in ServerWindowCompositorFrameSinkManager

Fixes bug in ServerWindowCompositorFrameSinkManager

It was assuming there is always a root window, which isn't the
case. GetRootWindow() only returns non-null when attached to a
display, so if a window has a parent but isn't yet attached to a
display then GetRoot returns null.

BUG=674575
TEST=none
R=fsamuel@chromium.org

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

[sky, 2016-12-15 21:49:05]

I would like a test for this, but I'm not sure how to exercise this from a test.
Any suggestions?

[sky, 2016-12-15 21:49:11]

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

[commit-bot: I haz the power, 2016-12-15 21:49:57]

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-12-15 21:53:12]

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

[commit-bot: I haz the power, 2016-12-15 21:53:13]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[sky, 2016-12-15 22:30:21]

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

[commit-bot: I haz the power, 2016-12-15 22:31:25]

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-12-15 23:48:15]

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

[commit-bot: I haz the power, 2016-12-15 23:48:16]

Dry run: This issue passed the CQ dry run.

[Fady Samuel, 2016-12-16 16:59:49]

lgtm, we might be able to write a test in frame_generator_unittest.cc but all
this code is changing significantly with Mikhail's changes so I'm not sure it's
worth it.

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

https://codereview.chromium.org/2580703005/diff/40001/services/ui/ws/server_w...
services/ui/ws/server_window_compositor_frame_sink_manager.cc:165: if
(window_->parent()) {
This check seems redundant then if we're checking for the root window.

[sky, 2016-12-16 17:07:07]

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

https://codereview.chromium.org/2580703005/diff/40001/services/ui/ws/server_w...
services/ui/ws/server_window_compositor_frame_sink_manager.cc:165: if
(window_->parent()) {
On 2016/12/16 16:59:49, Fady Samuel wrote:
> This check seems redundant then if we're checking for the root window.

If we don't check the parent it means if a frame sink is created for the root
it'll be added as a child of itself. Do we want that?

[Fady Samuel, 2016-12-16 17:07:59]

On 2016/12/16 17:07:07, sky wrote:
>
https://codereview.chromium.org/2580703005/diff/40001/services/ui/ws/server_w...
> File services/ui/ws/server_window_compositor_frame_sink_manager.cc (left):
> 
>
https://codereview.chromium.org/2580703005/diff/40001/services/ui/ws/server_w...
> services/ui/ws/server_window_compositor_frame_sink_manager.cc:165: if
> (window_->parent()) {
> On 2016/12/16 16:59:49, Fady Samuel wrote:
> > This check seems redundant then if we're checking for the root window.
> 
> If we don't check the parent it means if a frame sink is created for the root
> it'll be added as a child of itself. Do we want that?

Ohh, good point. No we don't. Leave it then :-) Thanks.

[sky, 2016-12-16 17:15:00]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2016-12-16 17:15:36]

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-12-16 17:22:14]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481908500607500,
"parent_rev": "bf84bb4def7deb2b5a1049d56563e2d103e23dad", "commit_rev":
"8d657e7468142c15eda37b3200e8ed9caee7c80a"}

[commit-bot: I haz the power, 2016-12-16 17:22:42]

Description was changed from

==========
Fixes bug in ServerWindowCompositorFrameSinkManager

It was assuming there is always a root window, which isn't the
case. GetRootWindow() only returns non-null when attached to a
display, so if a window has a parent but isn't yet attached to a
display then GetRoot returns null.

BUG=674575
TEST=none
R=fsamuel@chromium.org
==========

to

==========
Fixes bug in ServerWindowCompositorFrameSinkManager

It was assuming there is always a root window, which isn't the
case. GetRootWindow() only returns non-null when attached to a
display, so if a window has a parent but isn't yet attached to a
display then GetRoot returns null.

BUG=674575
TEST=none
R=fsamuel@chromium.org

Review-Url: https://codereview.chromium.org/2580703005
==========

[commit-bot: I haz the power, 2016-12-16 17:22:44]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-16 17:25:19]

Description was changed from

==========
Fixes bug in ServerWindowCompositorFrameSinkManager

It was assuming there is always a root window, which isn't the
case. GetRootWindow() only returns non-null when attached to a
display, so if a window has a parent but isn't yet attached to a
display then GetRoot returns null.

BUG=674575
TEST=none
R=fsamuel@chromium.org

Review-Url: https://codereview.chromium.org/2580703005
==========

to

==========
Fixes bug in ServerWindowCompositorFrameSinkManager

It was assuming there is always a root window, which isn't the
case. GetRootWindow() only returns non-null when attached to a
display, so if a window has a parent but isn't yet attached to a
display then GetRoot returns null.

BUG=674575
TEST=none
R=fsamuel@chromium.org

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

[commit-bot: I haz the power, 2016-12-16 17:25:20]

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