|
|
Chromium Code Reviews|
Created:
3 years, 10 months ago by Saman Sami Modified:
3 years, 10 months ago CC:
chromium-reviews, yusukes+watch_chromium.org, shuchen+watch_chromium.org, jam, 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 Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDelegatedFrameHost should not return old resources to renderer
As you can see in RendererCompositorFrameSink::OnReclaimCompositorResources,
the renderer will ignore the resources returned by DelegatedFrameHost that
have an old compositor_frame_sink_id, so we might as well not return them.
This CL also fixes a unit test that wrongfully tested that the resources must
be returned even after context lost.
BUG=692880
TBR=jam@chromium.org
Review-Url: https://codereview.chromium.org/2710073003
Cr-Commit-Position: refs/heads/master@{#452394}
Committed: https://chromium.googlesource.com/chromium/src/+/53815530f079d15e42d21571466156b40881ed58
Patch Set 1 #
Total comments: 2
Patch Set 2 : Updated comment #
Messages
Total messages: 26 (16 generated)
The CQ bit was checked by samans@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...
Description was changed from ========== DelegatedFrameHost should not return old resources to renderer As you can see in RendererCompositorFrameSink::OnReclaimCompositorResources, the renderer will ignore the resources returned by DelegatedFrameHost that have an old compositor_frame_sink_id, so we might as well not return them. This CL also fixes a unit test that wrongfully tested that the resources must be returned even after context lost. BUG=692880 ========== to ========== DelegatedFrameHost should not return old resources to renderer As you can see in RendererCompositorFrameSink::OnReclaimCompositorResources, the renderer will ignore the resources returned by DelegatedFrameHost that have an old compositor_frame_sink_id, so we might as well not return them. This CL also fixes a unit test that wrongfully tested that the resources must be returned even after context lost. BUG=692880 ==========
samans@chromium.org changed reviewers: + fsamuel@chromium.org
fsamuel@: Please review all files.
LGTM + nit https://codereview.chromium.org/2710073003/diff/1/content/browser/renderer_ho... File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc (right): https://codereview.chromium.org/2710073003/diff/1/content/browser/renderer_ho... content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1827: // DelegateFrameHost returns compositor resources without a swap ack. This comment should be updated as well.
https://codereview.chromium.org/2710073003/diff/1/content/browser/renderer_ho... File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc (right): https://codereview.chromium.org/2710073003/diff/1/content/browser/renderer_ho... content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1827: // DelegateFrameHost returns compositor resources without a swap ack. On 2017/02/22 23:16:08, Fady Samuel wrote: > This comment should be updated as well. Done.
The CQ bit was checked by samans@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...
samans@chromium.org changed reviewers: + jbauman@chromium.org
jbauman@: Please review both files.
lgtm
samans@chromium.org changed reviewers: + jam@chromium.org
jam@: Please review both files.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
jam@google.com changed reviewers: + jam@google.com
lgtm
Description was changed from ========== DelegatedFrameHost should not return old resources to renderer As you can see in RendererCompositorFrameSink::OnReclaimCompositorResources, the renderer will ignore the resources returned by DelegatedFrameHost that have an old compositor_frame_sink_id, so we might as well not return them. This CL also fixes a unit test that wrongfully tested that the resources must be returned even after context lost. BUG=692880 ========== to ========== DelegatedFrameHost should not return old resources to renderer As you can see in RendererCompositorFrameSink::OnReclaimCompositorResources, the renderer will ignore the resources returned by DelegatedFrameHost that have an old compositor_frame_sink_id, so we might as well not return them. This CL also fixes a unit test that wrongfully tested that the resources must be returned even after context lost. BUG=692880 TBR=jam@chromium.org ==========
The CQ bit was checked by samans@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/2710073003/#ps20001 (title: "Updated comment")
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": 20001, "attempt_start_ts": 1487825584400480,
"parent_rev": "6b3d9805494f966193ad47181d39544d541e79f2", "commit_rev":
"53815530f079d15e42d21571466156b40881ed58"}
Message was sent while issue was closed.
Description was changed from ========== DelegatedFrameHost should not return old resources to renderer As you can see in RendererCompositorFrameSink::OnReclaimCompositorResources, the renderer will ignore the resources returned by DelegatedFrameHost that have an old compositor_frame_sink_id, so we might as well not return them. This CL also fixes a unit test that wrongfully tested that the resources must be returned even after context lost. BUG=692880 TBR=jam@chromium.org ========== to ========== DelegatedFrameHost should not return old resources to renderer As you can see in RendererCompositorFrameSink::OnReclaimCompositorResources, the renderer will ignore the resources returned by DelegatedFrameHost that have an old compositor_frame_sink_id, so we might as well not return them. This CL also fixes a unit test that wrongfully tested that the resources must be returned even after context lost. BUG=692880 TBR=jam@chromium.org Review-Url: https://codereview.chromium.org/2710073003 Cr-Commit-Position: refs/heads/master@{#452394} Committed: https://chromium.googlesource.com/chromium/src/+/53815530f079d15e42d215714661... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/53815530f079d15e42d215714661...
Message was sent while issue was closed.
lgtm |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
