Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in

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

8 months ago by Fady Samuel
8 months ago
enne (OOO)
CC:, chromium-reviews
Target Ref:


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: Cr-Commit-Position: refs/heads/master@{#457948} Committed:

Patch Set 1 #

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

Total comments: 3

Patch Set 3 : Added additional unit test #


Total messages: 25 (16 generated)
Fady Samuel
PTAL Enne! Minor change + unit test. Fixes a Mus+Ash bug! Thanks!
8 months 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.
8 months ago (2017-03-16 23:50:16 UTC) #9
enne (OOO) File cc/surfaces/ (right): cc/surfaces/ if (!iter->second.has_children() && !clients_.count(parent_frame_sink_id) && I'm not sure that ...
8 months ago (2017-03-17 17:00:38 UTC) #13
enne (OOO)
Ok, nevermind, convinced myself that this should be ok. The node will get removed, but ...
8 months ago (2017-03-17 18:39:41 UTC) #14
Fady Samuel
Thanks Enne. CQ'ing. File cc/surfaces/ (right): cc/surfaces/ if (!iter->second.has_children() && !clients_.count(parent_frame_sink_id) && On ...
8 months ago (2017-03-17 18:42:35 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at
8 months 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 (JOB_FAILED,
8 months ago (2017-03-17 22:56:04 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at
8 months ago (2017-03-18 02:41:42 UTC) #22
commit-bot: I haz the power
8 months ago (2017-03-18 03:26:37 UTC) #25
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as

Powered by Google App Engine
This is Rietveld efc10ee0f