Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(7)

Issue 2750223005: Preserve FrameSinkSourceMapping nodes that have path to root (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 2 weeks ago by Fady Samuel
Modified:
2 months, 1 week ago
Reviewers:
enne
CC:
cc-bugs_chromium.org, chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Preserve FrameSinkSourceMapping nodes that have path to root In the Mus window server, every ServerWindow has a corresponding unique FrameSinkId whether or not it has a CompositorFrameSink. Some (most) ServerWindows do not submit CompositorFrames because they are local windows within some ancestor that does submit a CompositorFrame. We keep the FrameSinkSourceMapping in sync with the ServerWindow hierarchy to simplify reparenting of windows across displays. However, prior to this patch, if a FrameSinkSourceMapping loses all its children and does not have a CompositorFrameSink (a SurfaceFactoryClient) registered, then the node will be removed. When the window server adds a child to the ServerWindow corresponding to the deleted FrameSinkSourceMapping that child becomes orphaned and an appropriate BeginFrameSource does not propagate to it. This manifests when all windows are closed in Mus+Ash and a new window is opened. That window will not show any content because it will not receive BeginFrames. This CL fixes this issue by preserving the FrameSinkSourceMapping as long as it has a BeginFrameSource. BUG=701626 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2750223005 Cr-Commit-Position: refs/heads/master@{#457948} Committed: https://chromium.googlesource.com/chromium/src/+/8f0964617b1d3e6abdc4f61e8f1d4b8bf0f6c672

Patch Set 1 #

Patch Set 2 : UnregisterBeginFrameSource, fix debug build of unit test #

Total comments: 3

Patch Set 3 : Added additional unit test #

Messages

Total messages: 25 (16 generated)
Fady Samuel
PTAL Enne! Minor change + unit test. Fixes a Mus+Ash bug! Thanks!
2 months, 2 weeks ago (2017-03-16 22:59:26 UTC) #4
Fady Samuel
PTAL Enne. I fixed a DCHECK by unregistering the BeginFrameSource in the unit test.
2 months, 2 weeks ago (2017-03-16 23:50:16 UTC) #9
enne
https://codereview.chromium.org/2750223005/diff/20001/cc/surfaces/surface_manager.cc File cc/surfaces/surface_manager.cc (right): https://codereview.chromium.org/2750223005/diff/20001/cc/surfaces/surface_manager.cc#newcode585 cc/surfaces/surface_manager.cc:585: if (!iter->second.has_children() && !clients_.count(parent_frame_sink_id) && I'm not sure that ...
2 months, 1 week ago (2017-03-17 17:00:38 UTC) #13
enne
Ok, nevermind, convinced myself that this should be ok. The node will get removed, but ...
2 months, 1 week ago (2017-03-17 18:39:41 UTC) #14
Fady Samuel
Thanks Enne. CQ'ing. https://codereview.chromium.org/2750223005/diff/20001/cc/surfaces/surface_manager.cc File cc/surfaces/surface_manager.cc (right): https://codereview.chromium.org/2750223005/diff/20001/cc/surfaces/surface_manager.cc#newcode585 cc/surfaces/surface_manager.cc:585: if (!iter->second.has_children() && !clients_.count(parent_frame_sink_id) && On ...
2 months, 1 week ago (2017-03-17 18:42:35 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2750223005/40001
2 months, 1 week ago (2017-03-17 21:16:53 UTC) #18
commit-bot: I haz the power
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_rel_ng/builds/386682)
2 months, 1 week ago (2017-03-17 22:56:04 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2750223005/40001
2 months, 1 week ago (2017-03-18 02:41:42 UTC) #22
commit-bot: I haz the power
2 months, 1 week ago (2017-03-18 03:26:37 UTC) #25
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/8f0964617b1d3e6abdc4f61e8f1d...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06