Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1772393002:
Frames received after guest detaches should be displayed. (Closed)
|
Created:
4 years, 9 months ago by wjmaclean Modified:
4 years, 9 months ago CC:
chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, jam, nasko+codewatch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFrames received after guest detaches should be displayed.
In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently
detect if the guest has been detached, and if it has we clear the
compositor surface and exit without processing the frame we've been
given.
Based on the repro app in the associated bug, it seems that if this
happens, it prevents subsequent frames from being sent, presumably
on account of the most recent frame remaining unsatisfied.
This CL modifies the logic to complete processing of the frame, and
then clearing the surface. This appears to prevent the behaviour
observed in the bug.
BUG=581440
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
Committed: https://crrev.com/17f1c1ded552cbcd11cac1b7985593ea2c847808
Cr-Commit-Position: refs/heads/master@{#380134}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Remove unnecessary variable. #Messages
Total messages: 27 (14 generated)
Description was changed from ========== Frames received after guest detaches should be displayed. In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently detect if the guest has been detached, and if it has we clear the compositor surface and exit without processing the frame we've been given. Based on the repro app in the associated bug, it seems that if this happens, it prevents subsequent frames from being sent, presumably on account of the most recent frame remaining unsatisfied. This CL modified the logic to complete processing of the frame, and then clearing the surface. This appears to prevent the behaviour observed in the bug. BUG=581440 ========== to ========== Frames received after guest detaches should be displayed. In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently detect if the guest has been detached, and if it has we clear the compositor surface and exit without processing the frame we've been given. Based on the repro app in the associated bug, it seems that if this happens, it prevents subsequent frames from being sent, presumably on account of the most recent frame remaining unsatisfied. This CL modified the logic to complete processing of the frame, and then clearing the surface. This appears to prevent the behaviour observed in the bug. BUG=581440 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ==========
The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1772393002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1772393002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
Description was changed from ========== Frames received after guest detaches should be displayed. In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently detect if the guest has been detached, and if it has we clear the compositor surface and exit without processing the frame we've been given. Based on the repro app in the associated bug, it seems that if this happens, it prevents subsequent frames from being sent, presumably on account of the most recent frame remaining unsatisfied. This CL modified the logic to complete processing of the frame, and then clearing the surface. This appears to prevent the behaviour observed in the bug. BUG=581440 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ========== to ========== Frames received after guest detaches should be displayed. In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently detect if the guest has been detached, and if it has we clear the compositor surface and exit without processing the frame we've been given. Based on the repro app in the associated bug, it seems that if this happens, it prevents subsequent frames from being sent, presumably on account of the most recent frame remaining unsatisfied. This CL modified the logic to complete processing of the frame, and then clearing the surface. This appears to prevent the behaviour observed in the bug. BUG=581440 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ==========
wjmaclean@chromium.org changed reviewers: + jbauman@chromium.org, kenrb@chromium.org
Hi ... could you both please take a quick look at a *small* CL and let me know if it seems reasonable?
https://codereview.chromium.org/1772393002/diff/1/content/browser/frame_host/... File content/browser/frame_host/render_widget_host_view_guest.cc (right): https://codereview.chromium.org/1772393002/diff/1/content/browser/frame_host/... content/browser/frame_host/render_widget_host_view_guest.cc:213: bool should_clear_compositor_surface_on_exit = !guest_ || !guest_->attached(); We don't actually need to declare a var for this if we don't want to, rather we can just check !guest_ || !guest_->attached() in the if-statement at the end.
lgtm. If we just throw away the frame then ViewMsg_SwapCompositorFrameAck won't be sent, so this makes sense. On 2016/03/08 16:13:07, wjmaclean wrote: > https://codereview.chromium.org/1772393002/diff/1/content/browser/frame_host/... > File content/browser/frame_host/render_widget_host_view_guest.cc (right): > > https://codereview.chromium.org/1772393002/diff/1/content/browser/frame_host/... > content/browser/frame_host/render_widget_host_view_guest.cc:213: bool > should_clear_compositor_surface_on_exit = !guest_ || !guest_->attached(); > We don't actually need to declare a var for this if we don't want to, rather we > can just check !guest_ || !guest_->attached() in the if-statement at the end. Yeah, I don't think we need to have a variable for this.
The CQ bit was checked by wjmaclean@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1772393002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1772393002/20001
wjmaclean@chromium.org changed reviewers: + nasko@chromium.org
nasko@ - PTAL?
Description was changed from ========== Frames received after guest detaches should be displayed. In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently detect if the guest has been detached, and if it has we clear the compositor surface and exit without processing the frame we've been given. Based on the repro app in the associated bug, it seems that if this happens, it prevents subsequent frames from being sent, presumably on account of the most recent frame remaining unsatisfied. This CL modified the logic to complete processing of the frame, and then clearing the surface. This appears to prevent the behaviour observed in the bug. BUG=581440 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ========== to ========== Frames received after guest detaches should be displayed. In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently detect if the guest has been detached, and if it has we clear the compositor surface and exit without processing the frame we've been given. Based on the repro app in the associated bug, it seems that if this happens, it prevents subsequent frames from being sent, presumably on account of the most recent frame remaining unsatisfied. This CL modifies the logic to complete processing of the frame, and then clearing the surface. This appears to prevent the behaviour observed in the bug. BUG=581440 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
jochen@chromium.org changed reviewers: + jochen@chromium.org
lgtm
The CQ bit was checked by wjmaclean@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jbauman@chromium.org Link to the patchset: https://codereview.chromium.org/1772393002/#ps20001 (title: "Remove unnecessary variable.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1772393002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1772393002/20001
Message was sent while issue was closed.
Description was changed from ========== Frames received after guest detaches should be displayed. In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently detect if the guest has been detached, and if it has we clear the compositor surface and exit without processing the frame we've been given. Based on the repro app in the associated bug, it seems that if this happens, it prevents subsequent frames from being sent, presumably on account of the most recent frame remaining unsatisfied. This CL modifies the logic to complete processing of the frame, and then clearing the surface. This appears to prevent the behaviour observed in the bug. BUG=581440 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ========== to ========== Frames received after guest detaches should be displayed. In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently detect if the guest has been detached, and if it has we clear the compositor surface and exit without processing the frame we've been given. Based on the repro app in the associated bug, it seems that if this happens, it prevents subsequent frames from being sent, presumably on account of the most recent frame remaining unsatisfied. This CL modifies the logic to complete processing of the frame, and then clearing the surface. This appears to prevent the behaviour observed in the bug. BUG=581440 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Frames received after guest detaches should be displayed. In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently detect if the guest has been detached, and if it has we clear the compositor surface and exit without processing the frame we've been given. Based on the repro app in the associated bug, it seems that if this happens, it prevents subsequent frames from being sent, presumably on account of the most recent frame remaining unsatisfied. This CL modifies the logic to complete processing of the frame, and then clearing the surface. This appears to prevent the behaviour observed in the bug. BUG=581440 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation ========== to ========== Frames received after guest detaches should be displayed. In RenderWidgetHostViewGuest::OnSwapCompositorFrame() we currently detect if the guest has been detached, and if it has we clear the compositor surface and exit without processing the frame we've been given. Based on the repro app in the associated bug, it seems that if this happens, it prevents subsequent frames from being sent, presumably on account of the most recent frame remaining unsatisfied. This CL modifies the logic to complete processing of the frame, and then clearing the surface. This appears to prevent the behaviour observed in the bug. BUG=581440 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation Committed: https://crrev.com/17f1c1ded552cbcd11cac1b7985593ea2c847808 Cr-Commit-Position: refs/heads/master@{#380134} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/17f1c1ded552cbcd11cac1b7985593ea2c847808 Cr-Commit-Position: refs/heads/master@{#380134}
Message was sent while issue was closed.
lgtm |
