Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2832043002:
ui: Prevent DelegatedFrameHost from taking CompositorResizeLock forever (Closed)
|
Created:
3 years, 8 months ago by danakj Modified:
3 years, 8 months ago CC:
chromium-reviews, yusukes+watch_chromium.org, shuchen+watch_chromium.org, jbauman+watch_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, piman+watch_chromium.org, kalyank, danakj+watch_chromium.org, James Su, xjz+watch_chromium.org, miu+watch_chromium.org, enne (OOO), piman, oshima Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Descriptionui: Prevent DelegatedFrameHost from taking CompositorResizeLock forever
In the case where DelegatedFrameHost has no frame, it is not showing
a renderer tab contents, so it does not need to try prevent guttering.
However it would grab a resize lock still, in some cases where it will
never be receiving a frame from the renderer. When that happens the
lock times out after 67ms and it grabs it again.. forever. This causes
the UI to update at 15 fps instead of 60.
This fixes it by not allowing a resize lock to be grabbed when
|has_frame_| is false. And when |has_frame_| changes from true to
false, any current resize lock is dropped, freeing up the UI to
present itself.
R=fsamuel@chromium.org, nick@chromium.org
BUG=712903
Review-Url: https://codereview.chromium.org/2832043002
Cr-Commit-Position: refs/heads/master@{#466508}
Committed: https://chromium.googlesource.com/chromium/src/+/8230eff799f34889d9ec93944a4e8f3147b91c17
Patch Set 1 #Patch Set 2 : resizelocknoframe: fixtestcomments #Patch Set 3 : resizelocknoframe: submitcomment #
Messages
Total messages: 32 (22 generated)
The CQ bit was checked by danakj@chromium.org to run a CQ dry run
Description was changed from ========== ui: Prevent DelegatedFrameHost from taking CompositorResizeLock forever In the case where DelegatedFrameHost has no frame, it is not showing a renderer tab contents, so it does not need to try prevent guttering. However it would grab a resize lock still, in some cases where it will never be receiving a frame from the renderer. When that happens the lock times out after 67ms and it grabs it again.. forever. This causes the UI to update at 15 fps instead of 60. This fixes it by not allowing a resize lock to be grabbed when |has_frame_| is false. And when |has_frame_| changes from true to false, any current resize lock is dropped, freeing up the UI to present itself. R=fsamuel@chromium.org, nick@chromium.org BUG=712903 ========== to ========== ui: Prevent DelegatedFrameHost from taking CompositorResizeLock forever In the case where DelegatedFrameHost has no frame, it is not showing a renderer tab contents, so it does not need to try prevent guttering. However it would grab a resize lock still, in some cases where it will never be receiving a frame from the renderer. When that happens the lock times out after 67ms and it grabs it again.. forever. This causes the UI to update at 15 fps instead of 60. This fixes it by not allowing a resize lock to be grabbed when |has_frame_| is false. And when |has_frame_| changes from true to false, any current resize lock is dropped, freeing up the UI to present itself. R=fsamuel@chromium.org, nick@chromium.org BUG=712903 ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
fsamuel: PTAL ncarter: please give OWNERS review. I added Fady since he thinks about resize a lot lately, and piman is not available who knows this stuff the most.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...) win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
LGTM! Thanks! I ran into both cases actually with frame eviction and hidden clients that don't produce CompositorFrames in surface sync. My random thinking out loud: I fixed the hidden clients blocking CompositorFrames thing by introducing the embedded_surfaces vector in CompositorFrameMetaData that only lists clients that are in SurfaceDrawQuads and are thus visible from the embedder's perspective. For evicted frames, I remove it as a blocker from other CompositorFrames (and activate them if necessary) on eviction.
The CQ bit was checked by danakj@chromium.org to run a CQ dry run
Small fix in the test to improve comments
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by danakj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by danakj@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
nick: bump
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
danakj@chromium.org changed reviewers: + jam@chromium.org
jam@ PTAL? nick seems away
lgtm
sorry just saw this, lgtm
The CQ bit was checked by danakj@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from fsamuel@chromium.org Link to the patchset: https://codereview.chromium.org/2832043002/#ps40001 (title: "resizelocknoframe: submitcomment")
Cool thanks!
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492817505840130,
"parent_rev": "ef03d2b9a67352094516f4e866373378fb961ee8", "commit_rev":
"8230eff799f34889d9ec93944a4e8f3147b91c17"}
Message was sent while issue was closed.
Description was changed from ========== ui: Prevent DelegatedFrameHost from taking CompositorResizeLock forever In the case where DelegatedFrameHost has no frame, it is not showing a renderer tab contents, so it does not need to try prevent guttering. However it would grab a resize lock still, in some cases where it will never be receiving a frame from the renderer. When that happens the lock times out after 67ms and it grabs it again.. forever. This causes the UI to update at 15 fps instead of 60. This fixes it by not allowing a resize lock to be grabbed when |has_frame_| is false. And when |has_frame_| changes from true to false, any current resize lock is dropped, freeing up the UI to present itself. R=fsamuel@chromium.org, nick@chromium.org BUG=712903 ========== to ========== ui: Prevent DelegatedFrameHost from taking CompositorResizeLock forever In the case where DelegatedFrameHost has no frame, it is not showing a renderer tab contents, so it does not need to try prevent guttering. However it would grab a resize lock still, in some cases where it will never be receiving a frame from the renderer. When that happens the lock times out after 67ms and it grabs it again.. forever. This causes the UI to update at 15 fps instead of 60. This fixes it by not allowing a resize lock to be grabbed when |has_frame_| is false. And when |has_frame_| changes from true to false, any current resize lock is dropped, freeing up the UI to present itself. R=fsamuel@chromium.org, nick@chromium.org BUG=712903 Review-Url: https://codereview.chromium.org/2832043002 Cr-Commit-Position: refs/heads/master@{#466508} Committed: https://chromium.googlesource.com/chromium/src/+/8230eff799f34889d9ec93944a4e... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/8230eff799f34889d9ec93944a4e... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
