Cleanup FrameGenerator, ServerWindowCompositorFrameSink |
||||||||
Issue descriptionFrameGenerator and ServerWindowCompositorFrameSink seem to have gotten a bit crufty: 1. FrameGenerator gets continuous BeginFrames but it really doesn't need them. FrameGenerator should only request a BeginFrame if something has changed: a. Screen resolution. b. Device scale factor c. Window manager surface ID has changed. Once a new CompositorFrame is submitted we should ask Mus-Gpu to stop sending BeginFrames. It's wasteful. 2. We probably shouldn't be creating a FrameGenerator until we have an AcceleratedWidget anyway. It has nothing useful to do until then. 3. FrameGenerator::OnSurfaceCreated isn't receiving all the necessary info. It should get a full SurfaceInfo. If SurfaceInfo has changed then it should request BeginFrame. It shouldn't get an OnSurfaceCreated for other windows. Taking in a ServerWindow seems silly. 4. delegate_->GetActiveWindow() is confusing and unnecessary once 3 is implemented. OnSurfaceCreated will only be called when the "active window" changes. 5. FrameGenerator likely doesn't need to be a ServerWindowTracker anymore. OnWindowDestroying can go away. 6. ServerWindowCompositorFrameSinkManager::GetLatestSurfaceId is only used by FrameGenerator and FrameGenerator should know about the window manager's SurfaceId from OnSurfaceCreated so this is redundant and can go away. 7. ServerWindowCompositorFrameSinkManager::GetLatestFrameSize is only used in tests and can go away. 8. ServerWindowCompositorFrameSinkManager::window() does not appear to be used.
,
Jan 23 2017
,
Jan 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/401c5932118fb24071348b6a6ee5ce1638a7d982 commit 401c5932118fb24071348b6a6ee5ce1638a7d982 Author: samans <samans@chromium.org> Date: Wed Jan 25 01:30:42 2017 FrameGenerator should receive SurfaceInfo and use it in frame generation FrameGenerator::OnSurfaceCreated should get a full SurfaceInfo object, as opposed to a surface id, and use that in frame generation instead of depending on a window object. FrameGenerator should not receive begin frames unless the SurfaceInfo object or device_scale_factor_ changes. Fixed a bug in SurfaceFactory, where the frame size was not calculated properly. BUG= 683732 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2651843002 Cr-Commit-Position: refs/heads/master@{#445901} [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/cc/surfaces/surface_factory.cc [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/cc/surfaces/surface_factory_unittest.cc [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/display.cc [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/display.h [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/frame_generator.cc [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/frame_generator.h [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/frame_generator_delegate.h [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/frame_generator_unittest.cc [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/platform_display_default.cc [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/platform_display_default.h [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/platform_display_delegate.h [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/test_utils.cc [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/test_utils.h [modify] https://crrev.com/401c5932118fb24071348b6a6ee5ce1638a7d982/services/ui/ws/window_server.cc
,
Jan 26 2017
,
Feb 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a821081c2d29d5b8ece6215742fbc55b26a659b commit 8a821081c2d29d5b8ece6215742fbc55b26a659b Author: fsamuel <fsamuel@chromium.org> Date: Tue Feb 07 20:43:14 2017 Cleanup ServerWindowCompositorFrameSinkManager and FrameGenerator There was a bunch of cruft left over from Surface ID propagation. This CL deletes a bunch of it. BUG= 683732 Review-Url: https://codereview.chromium.org/2683553004 Cr-Commit-Position: refs/heads/master@{#448725} [modify] https://crrev.com/8a821081c2d29d5b8ece6215742fbc55b26a659b/services/ui/ws/frame_generator.cc [modify] https://crrev.com/8a821081c2d29d5b8ece6215742fbc55b26a659b/services/ui/ws/frame_generator.h [modify] https://crrev.com/8a821081c2d29d5b8ece6215742fbc55b26a659b/services/ui/ws/server_window_compositor_frame_sink_manager.cc [modify] https://crrev.com/8a821081c2d29d5b8ece6215742fbc55b26a659b/services/ui/ws/server_window_compositor_frame_sink_manager.h
,
Feb 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c46bbe24124bdef62daa0e396bb4baa2681fd817 commit c46bbe24124bdef62daa0e396bb4baa2681fd817 Author: samans <samans@chromium.org> Date: Thu Feb 23 01:30:11 2017 FrameGenerator should not be created until an AcceleratedWidget is available FrameGenerator does not have anything useful to do until an AcceleratedWidget is provided. This CL delays the creation of FrameGenerator until a widget is available. We also remove the unit test because it's not doing anything useful right now. BUG= 683732 Review-Url: https://codereview.chromium.org/2694043003 Cr-Commit-Position: refs/heads/master@{#452329} [modify] https://crrev.com/c46bbe24124bdef62daa0e396bb4baa2681fd817/services/ui/ws/BUILD.gn [modify] https://crrev.com/c46bbe24124bdef62daa0e396bb4baa2681fd817/services/ui/ws/frame_generator.cc [modify] https://crrev.com/c46bbe24124bdef62daa0e396bb4baa2681fd817/services/ui/ws/frame_generator.h [delete] https://crrev.com/919b5796526c5c9764ae73ab5f5f50829597556f/services/ui/ws/frame_generator_unittest.cc [modify] https://crrev.com/c46bbe24124bdef62daa0e396bb4baa2681fd817/services/ui/ws/platform_display_default.cc [modify] https://crrev.com/c46bbe24124bdef62daa0e396bb4baa2681fd817/services/ui/ws/platform_display_default.h
,
Feb 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cfa63cc0384a8ab0d8dbea6e67543d21716485dd commit cfa63cc0384a8ab0d8dbea6e67543d21716485dd Author: hajimehoshi <hajimehoshi@chromium.org> Date: Thu Feb 23 10:07:33 2017 Revert of FrameGenerator should not be created until an AcceleratedWidget is available (patchset #6 id:100001 of https://codereview.chromium.org/2694043003/ ) Reason for revert: This might cause the crash (crbug/695371) Original issue's description: > FrameGenerator should not be created until an AcceleratedWidget is available > > FrameGenerator does not have anything useful to do until an > AcceleratedWidget is provided. This CL delays the creation of > FrameGenerator until a widget is available. > > We also remove the unit test because it's not doing anything useful > right now. > > BUG= 683732 > > Review-Url: https://codereview.chromium.org/2694043003 > Cr-Commit-Position: refs/heads/master@{#452329} > Committed: https://chromium.googlesource.com/chromium/src/+/c46bbe24124bdef62daa0e396bb4baa2681fd817 TBR=fsamuel@chromium.org,sky@chromium.org,samans@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 683732 Review-Url: https://codereview.chromium.org/2717453002 Cr-Commit-Position: refs/heads/master@{#452446} [modify] https://crrev.com/cfa63cc0384a8ab0d8dbea6e67543d21716485dd/services/ui/ws/BUILD.gn [modify] https://crrev.com/cfa63cc0384a8ab0d8dbea6e67543d21716485dd/services/ui/ws/frame_generator.cc [modify] https://crrev.com/cfa63cc0384a8ab0d8dbea6e67543d21716485dd/services/ui/ws/frame_generator.h [add] https://crrev.com/cfa63cc0384a8ab0d8dbea6e67543d21716485dd/services/ui/ws/frame_generator_unittest.cc [modify] https://crrev.com/cfa63cc0384a8ab0d8dbea6e67543d21716485dd/services/ui/ws/platform_display_default.cc [modify] https://crrev.com/cfa63cc0384a8ab0d8dbea6e67543d21716485dd/services/ui/ws/platform_display_default.h
,
Feb 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fa2337ad665fad794a067127e6e4bca8d30764c6 commit fa2337ad665fad794a067127e6e4bca8d30764c6 Author: hajimehoshi <hajimehoshi@chromium.org> Date: Thu Feb 23 11:08:07 2017 Reland of FrameGenerator should not be created until an AcceleratedWidget is available (patchset #1 id:1 of https://codereview.chromium.org/2717453002/ ) Reason for revert: This was not the culprit of crbug.com/695375 as the build bot results said. Sorry for confusion. https://build.chromium.org/p/chromium.win/buildstatus?builder=Win7%20Tests%20%281%29&number=64018 Original issue's description: > Revert of FrameGenerator should not be created until an AcceleratedWidget is available (patchset #6 id:100001 of https://codereview.chromium.org/2694043003/ ) > > Reason for revert: > This might cause the crash (crbug/695371) > > Original issue's description: > > FrameGenerator should not be created until an AcceleratedWidget is available > > > > FrameGenerator does not have anything useful to do until an > > AcceleratedWidget is provided. This CL delays the creation of > > FrameGenerator until a widget is available. > > > > We also remove the unit test because it's not doing anything useful > > right now. > > > > BUG= 683732 > > > > Review-Url: https://codereview.chromium.org/2694043003 > > Cr-Commit-Position: refs/heads/master@{#452329} > > Committed: https://chromium.googlesource.com/chromium/src/+/c46bbe24124bdef62daa0e396bb4baa2681fd817 > > TBR=fsamuel@chromium.org,sky@chromium.org,samans@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 683732 > > Review-Url: https://codereview.chromium.org/2717453002 > Cr-Commit-Position: refs/heads/master@{#452446} > Committed: https://chromium.googlesource.com/chromium/src/+/cfa63cc0384a8ab0d8dbea6e67543d21716485dd TBR=fsamuel@chromium.org,sky@chromium.org,samans@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 683732 Review-Url: https://codereview.chromium.org/2715533006 Cr-Commit-Position: refs/heads/master@{#452457} [modify] https://crrev.com/fa2337ad665fad794a067127e6e4bca8d30764c6/services/ui/ws/BUILD.gn [modify] https://crrev.com/fa2337ad665fad794a067127e6e4bca8d30764c6/services/ui/ws/frame_generator.cc [modify] https://crrev.com/fa2337ad665fad794a067127e6e4bca8d30764c6/services/ui/ws/frame_generator.h [delete] https://crrev.com/0c9658dcf96d88ad497cd4c43765dfad8b93dd1c/services/ui/ws/frame_generator_unittest.cc [modify] https://crrev.com/fa2337ad665fad794a067127e6e4bca8d30764c6/services/ui/ws/platform_display_default.cc [modify] https://crrev.com/fa2337ad665fad794a067127e6e4bca8d30764c6/services/ui/ws/platform_display_default.h
,
Feb 23 2017
One other nice cleanup (potential bug fix?) that I noticed is FrameGenerator has a FrameGeneratorDelegate whose sole purpose is now just IsInHighContrastMode(). Perhaps, if FrameGenerator instead had a FrameGenerator::SetInHighContrastMode() then the delegate becomes unnecessary. What's interesting as well, is I think high contrast mode can probably be turned on and off and we don't trigger the generation of a new frame when high contrast mode changes. If high contrast mode changes, then that should trigger a BeginFrame and then FrameGenerator should pick that up and add the RenderPassDrawQuad with the invert filter. FrameGeneratorDelegate goes away, so this might actually be a net negative lines of code patch too! Yay!
,
Feb 23 2017
Before I forget, another change that needs to happen in FrameGenerator, most likely for external window mode, is it needs to be told when the visibility of the PlatformWindow changes (say on minimize). Currently we always set the cc::Display's visiblity (in GpuDisplayCompositorFrameSink, soon GpuRootCompositorFrameSink) to true. What we should instead to is make visibility another toggle on FrameGenerator that then gets passed along DisplayPrivate.
,
Feb 23 2017
A couple more observations: 1. We only check if the frame size has changed to allocate a new LocalSurfaceId. We should also check the device scale factor. 2. We read the current frame size from the render_pass_list in FrameGenerator::OnBeginFrame. This seems silly, FrameGenerator already knows the frame size because it generated the CompositorFrame. Maybe we shouldn't be looking at the render_pass_list and instead just look at the root_window_->bounds() directly? 3. I really think we should find a way to delete ServerWindowCompositorFrameSinkManager::GetLatestFrameSize and SetLatestSurfaceInfo. GetLatestFrameSize is only used in tests to ensure a client has drawn. This is a lot of added complexity for a test.
,
Feb 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/45975708707a4b35561d074e0315c517800f6bed commit 45975708707a4b35561d074e0315c517800f6bed Author: xing.xu <xing.xu@intel.com> Date: Thu Feb 23 18:48:18 2017 Remove latest_submitted_surface_info latest_submitted_surface_info in CompositorFrameSinkData is only for test. BUG= 683732 Review-Url: https://codereview.chromium.org/2711043002 Cr-Commit-Position: refs/heads/master@{#452563} [modify] https://crrev.com/45975708707a4b35561d074e0315c517800f6bed/services/ui/ws/server_window_compositor_frame_sink_manager.cc [modify] https://crrev.com/45975708707a4b35561d074e0315c517800f6bed/services/ui/ws/server_window_compositor_frame_sink_manager.h [modify] https://crrev.com/45975708707a4b35561d074e0315c517800f6bed/services/ui/ws/window_server.cc [modify] https://crrev.com/45975708707a4b35561d074e0315c517800f6bed/services/ui/ws/window_server_test_impl.cc
,
Feb 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9edf32fa33654215032bae96032066bf7e662a9f commit 9edf32fa33654215032bae96032066bf7e662a9f Author: fsamuel <fsamuel@chromium.org> Date: Thu Feb 23 20:58:30 2017 FrameGenerator: Don't issue BeginFrames if we have nothing to draw Updating the device scale factor before the window manager's CompositorFrame is ready caused a premature BeginFrame in FrameGenerator. This would cause an invalid CompositorFrame to be generated. BUG= 683732 Review-Url: https://codereview.chromium.org/2718523002 Cr-Commit-Position: refs/heads/master@{#452623} [modify] https://crrev.com/9edf32fa33654215032bae96032066bf7e662a9f/services/ui/ws/frame_generator.cc
,
Mar 2 2017
I think before we make further changes on FrameGenerator, we need to make it mockable in unit tests. I propose: 1. Rename WindowCompositorFrameSink => ClientCompositorFrameSink. 2. Move ClientCompositorFrameSink to components/display_compositor/public/cpp. 3. FrameGenerator takes in a cc::CompositorFrameSink. By default, a ClientCompositorFrameSink. 4. A unit test plugs a TestCompositorFrameSink into FrameGenerator. We should unit test device scale factor changes and WindowManager SurfaceInfo changes to start. We can then get rid of FrameGeneratorDelegate::IsInHighContrastMode, in favor of FrameGenerator::SetInHighContrastMode and then unit test that.
,
Mar 2 2017
,
Mar 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b commit e4ff7160c06948baf8dd3d3141c00c8cee40fc3b Author: staraz <staraz@chromium.org> Date: Wed Mar 08 01:07:45 2017 Rename WindowCompositorFrameSink To ClientCompositorFrameSink This name is more natural since it's the CompositorFrameSink running on the client side. BUG= 683732 Review-Url: https://codereview.chromium.org/2732283002 Cr-Commit-Position: refs/heads/master@{#455316} [modify] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/content/browser/compositor/mus_browser_compositor_output_surface.cc [modify] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/content/browser/compositor/mus_browser_compositor_output_surface.h [modify] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/content/renderer/mus/renderer_window_tree_client.cc [modify] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/content/renderer/mus/renderer_window_tree_client.h [modify] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/services/ui/public/cpp/BUILD.gn [modify] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/services/ui/public/cpp/OWNERS [rename] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/services/ui/public/cpp/client_compositor_frame_sink.cc [rename] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/services/ui/public/cpp/client_compositor_frame_sink.h [modify] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/ui/aura/mus/DEPS [modify] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/ui/aura/mus/window_port_mus.cc [modify] https://crrev.com/e4ff7160c06948baf8dd3d3141c00c8cee40fc3b/ui/aura/mus/window_port_mus.h
,
Mar 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/72df4334a57fdd13b8dbbc7cd48895fcd79b3a63 commit 72df4334a57fdd13b8dbbc7cd48895fcd79b3a63 Author: staraz <staraz@chromium.org> Date: Sat Mar 11 17:48:41 2017 Add DisplayClientCompositorFrameSink class. DisplayClientCompositorFrameSink can be understood as a ClientCompositorFrameSink that also has a DisplayPrivate interface pointer. This is part of the effort to make FrameGenerator mockable and unit test-able. DisplayClientCompositorFrameSink will be used by FrameGenerator in the future where FrameGenerator takes a cc::CompositorFrameSink* (DisplayClientCompositorFramesink by default) upon creation. We can then unit test FrameGenerator with a TestCompositorFrameSink. BUG= 683732 Review-Url: https://codereview.chromium.org/2738923002 Cr-Commit-Position: refs/heads/master@{#456282} [modify] https://crrev.com/72df4334a57fdd13b8dbbc7cd48895fcd79b3a63/services/ui/ws/BUILD.gn [add] https://crrev.com/72df4334a57fdd13b8dbbc7cd48895fcd79b3a63/services/ui/ws/display_client_compositor_frame_sink.cc [add] https://crrev.com/72df4334a57fdd13b8dbbc7cd48895fcd79b3a63/services/ui/ws/display_client_compositor_frame_sink.h [modify] https://crrev.com/72df4334a57fdd13b8dbbc7cd48895fcd79b3a63/services/ui/ws/frame_generator.cc [modify] https://crrev.com/72df4334a57fdd13b8dbbc7cd48895fcd79b3a63/services/ui/ws/frame_generator.h [modify] https://crrev.com/72df4334a57fdd13b8dbbc7cd48895fcd79b3a63/services/ui/ws/platform_display_default.cc [modify] https://crrev.com/72df4334a57fdd13b8dbbc7cd48895fcd79b3a63/services/ui/ws/platform_display_default.h
,
Mar 11 2017
Sweet! Now that FrameGenerator uses a cc::CompositorFrameSink, it ought to be fairly straightforward to write unit tests for FrameGenerator. In particular, a mock CompositorFrameSink could also dispense a test BeginFrameSource. We can manually tick BeginFrames in unit tests. We can receive CompositorFrames in the mock and verify that the right CompositorFrames have been generated. Some useful test ideas: 1. We should verify that changes the device scale factor triggers a BeginFrame but only if there is a valid SurfaceInfo set. 2. Changing the SurfaceInfo should trigger a BeginFrame. We should verify that a CompositorFrame refers to the window manager's surface via referenced_surfaces. Changes that need to happen and ultimately need tests: 1. Changing high contrast mode should trigger a BeginFrame. We should verify that an appropriate RenderPass is created as a result. 2. We should track window visibility. A hidden window should not get BeginFrames. Eventually to support mus-gpu restarts, we should detect when we lose a CompositorFrameSink and request a new one. That can happen later... maybe that can be a separate bug?
,
Mar 12 2017
I think we can modify TestCompositorFrameSink::DidReceiveCompositorFrameAck() to keep track of the number of CompositorFrames received, but how do we know if a FrameGenerator is requesting BeginFrames?
,
Mar 12 2017
We don't necessarily need to use TestCompositorFrameSink if it doesn't give us the hooks we need for FrameGenerator. We can write a new CompositorFrameSink for FrameGenerator testing if necessary. Take a look at what we did in compositor_frame_sink_support_unittest: https://cs.chromium.org/chromium/src/cc/surfaces/compositor_frame_sink_support_unittest.cc?type=cs&q=compositor_frame_sink_support_unittest.cc&l=161 We dispense a FakeExternalBeginFrameSource. We can then fire off BeginFrames as we do here: https://cs.chromium.org/chromium/src/cc/surfaces/compositor_frame_sink_support_unittest.cc?type=cs&l=471
,
Mar 14 2017
I think I can use a simpler TestCompositorFrameSink along with FakeExternalBeginFrameSource. Once we manually tick the FakeExternalBeginFrameSource, CFS should only receives a CompositorFrame if SurfaceInfo has changed OR (device scale factor has changed AND FrameGeneraotr has a valid SurfaceInfo). fsamuel@: any thoughts?
,
Mar 14 2017
Well, you could either reuse TestCompositorFrameSink or write your own test version. TestCompositorFrameSink is actually kind of complicated, so maybe writing a simpler one is easier. Not sure. Use your judgement.
,
Mar 14 2017
For what it's worth.. Usually our test fixtures are complicated because they let tests use them with minimal work, and I prefer adding things to a single test fixture to adding many test fixtures that do slightly different things but are not as discoverable/shareable. But ya, use your judgement.
,
Mar 17 2017
#18: Currently, FrameGenerator calls delegate_->IsInHighContrastMode() to know if it's in high contrast mode. Do you want to add a FrameGenerator::OnHighContrastModeSet() so WindowServer calls that to trigger a BeginFrame in SetHighContrastMode()? (There's also a TODO saying setting high contrast mode doesn't seem like a window server concept: https://cs.chromium.org/chromium/src/services/ui/ws/window_server.cc?l=284) Also, FrameGenerator early returns in OnBeginFrame(0 if the root_window_ is not visible. Do you want that check to move to SetNeedsBeginFrames()?
,
Mar 17 2017
staraz@: FrameGenerator::SetInHighConstrastMode(bool) yea. It should behave like SetDeviceScaleFactor. I like the idea of checking visibility in SetNeedsBeginFrames! Thanks. Furthermore, changing visibility should trigger a BeginFrame.
,
Mar 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e89706698b46b5391e000fbd7175fd63cdf1a64 commit 8e89706698b46b5391e000fbd7175fd63cdf1a64 Author: staraz <staraz@chromium.org> Date: Sat Mar 18 04:12:39 2017 Add FrameGenerator Unit Tests Added FrameGeneratorTest and several mock classes to facilitate unit tests for FrameGenerator. Also added some explanatory comments to DisplayClientCompositorFrameSink. BUG= 683732 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2755573002 Cr-Commit-Position: refs/heads/master@{#457954} [modify] https://crrev.com/8e89706698b46b5391e000fbd7175fd63cdf1a64/services/ui/ws/BUILD.gn [modify] https://crrev.com/8e89706698b46b5391e000fbd7175fd63cdf1a64/services/ui/ws/display_client_compositor_frame_sink.h [modify] https://crrev.com/8e89706698b46b5391e000fbd7175fd63cdf1a64/services/ui/ws/frame_generator.cc [add] https://crrev.com/8e89706698b46b5391e000fbd7175fd63cdf1a64/services/ui/ws/frame_generator_unittest.cc
,
Mar 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0855d091dfde14cb00275864c1534651441f7180 commit 0855d091dfde14cb00275864c1534651441f7180 Author: staraz <staraz@chromium.org> Date: Tue Mar 21 15:23:10 2017 [mus]Add FrameGenerator::SetHighContrastMode(bool) Added FrameGenerator::high_contrast_mode_enabled_ flag. The flags of all FrameGenerators are updated by WindowServer when high contrast mode is toggled by AccessibilityManager for the current active user. Toggling high contrast mode triggers a BeginFrame for the FrameGenerator. FrameGeneratorTest.SetHighContrastMode turns on high contrast mode and verifies that the CompositorFrame received from FrameGenerator has a InvertFilter with invert amount of 1.0. BUG= 683732 TBR=msw@chromium.org Review-Url: https://codereview.chromium.org/2759933007 Cr-Commit-Position: refs/heads/master@{#458421} [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/display.cc [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/display.h [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/display_manager.cc [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/display_manager.h [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/frame_generator.cc [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/frame_generator.h [delete] https://crrev.com/a3d6db6921ab508bc66d04a8141b872325a7b5c2/services/ui/ws/frame_generator_delegate.h [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/frame_generator_unittest.cc [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/platform_display_default.cc [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/platform_display_default.h [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/platform_display_delegate.h [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/test_utils.cc [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/test_utils.h [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/window_server.cc [modify] https://crrev.com/0855d091dfde14cb00275864c1534651441f7180/services/ui/ws/window_server.h
,
Mar 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/394caf0ffa05d6c7ada7560a3379f4807215bfb4 commit 394caf0ffa05d6c7ada7560a3379f4807215bfb4 Author: staraz <staraz@chromium.org> Date: Thu Mar 23 18:39:27 2017 Remove FrameGenerator::root_window_ FrameGenerator should be as simple as possible. PlatformDisplayDefault implements ServerWindowObserver and notify its FrameGenerator when the visibility or bounds of the root window changes. FrameGenerator updates window visibility and bounds when it gets notified and requests BeginFrames accordingly. BUG= 683732 Review-Url: https://codereview.chromium.org/2763143002 Cr-Commit-Position: refs/heads/master@{#459157} [modify] https://crrev.com/394caf0ffa05d6c7ada7560a3379f4807215bfb4/services/ui/ws/frame_generator.cc [modify] https://crrev.com/394caf0ffa05d6c7ada7560a3379f4807215bfb4/services/ui/ws/frame_generator.h [modify] https://crrev.com/394caf0ffa05d6c7ada7560a3379f4807215bfb4/services/ui/ws/frame_generator_unittest.cc [modify] https://crrev.com/394caf0ffa05d6c7ada7560a3379f4807215bfb4/services/ui/ws/platform_display_default.cc [modify] https://crrev.com/394caf0ffa05d6c7ada7560a3379f4807215bfb4/services/ui/ws/platform_display_default.h
,
Mar 24 2017
I'm pretty happy with the cleanups that have been done since I filed this bug. FrameGenerator is finally (fairly) easy to understand and well tested. Thanks Saman! Thanks Alex! I'm marking as FIXED!
,
Jun 13 2017
,
Feb 26 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by fsam...@chromium.org
, Jan 22 2017