Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2238853003:
[Chromoting] A crash in DesktopSessionProxy::OnCaptureResult (Closed)
Description[Chromoting] A quick fix of bug 637087
IpcVideoFrameCapturer may be released before DesktopSessionAgent returns a
frame, so we should check before calling
IpcVideoFrameCapturer::OnCaptureResult() function.
BUG=637087
Committed: https://crrev.com/7f1e89c1582b6f936cd858e950ea9f2dce3e8a6a
Cr-Commit-Position: refs/heads/master@{#411929}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Review comments #Messages
Total messages: 20 (14 generated)
The CQ bit was checked by zijiehe@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 ========== [Chromoting] A quick fix of bug 637087 IpcVideoFrameCapturer may be released before DesktopSessionAgent returns a frame, so we should check before calling IpcVideoFrameCapturer::OnCaptureResult() function. BUG=637087 ========== to ========== [Chromoting] A quick fix of bug 637087 IpcVideoFrameCapturer may be released before DesktopSessionAgent returns a frame, so we should check before calling IpcVideoFrameCapturer::OnCaptureResult() function. BUG=637087 ==========
zijiehe@chromium.org changed reviewers: + joedow@chromium.org, sergeyu@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2238853003/diff/1/remoting/host/desktop_sessi... File remoting/host/desktop_session_proxy.cc (right): https://codereview.chromium.org/2238853003/diff/1/remoting/host/desktop_sessi... remoting/host/desktop_session_proxy.cc:488: // video_capturer_ has been released before DesktopSessionAgent returns this nit: 1. the capturer was deleted, not released. 2. reader may not be aware what is DesktopSessionAgent and it's not that important here. If it has to be mentioned here, then it's better to call it "the desktop process". suggest rephrasing: "Capturer was deleted while capture request was pending." Or maybe just remove this comment, as it should be clear what's happening without this comment
On 2016/08/12 04:15:18, Sergey Ulanov wrote: > lgtm > > https://codereview.chromium.org/2238853003/diff/1/remoting/host/desktop_sessi... > File remoting/host/desktop_session_proxy.cc (right): > > https://codereview.chromium.org/2238853003/diff/1/remoting/host/desktop_sessi... > remoting/host/desktop_session_proxy.cc:488: // video_capturer_ has been released > before DesktopSessionAgent returns this > nit: > 1. the capturer was deleted, not released. > 2. reader may not be aware what is DesktopSessionAgent and it's not that > important here. If it has to be mentioned here, then it's better to call it "the > desktop process". > suggest rephrasing: "Capturer was deleted while capture request was pending." Or > maybe just remove this comment, as it should be clear what's happening without > this comment Done.
The CQ bit was checked by zijiehe@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: This issue passed the CQ dry run.
The CQ bit was checked by zijiehe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org Link to the patchset: https://codereview.chromium.org/2238853003/#ps20001 (title: "Review comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] A quick fix of bug 637087 IpcVideoFrameCapturer may be released before DesktopSessionAgent returns a frame, so we should check before calling IpcVideoFrameCapturer::OnCaptureResult() function. BUG=637087 ========== to ========== [Chromoting] A quick fix of bug 637087 IpcVideoFrameCapturer may be released before DesktopSessionAgent returns a frame, so we should check before calling IpcVideoFrameCapturer::OnCaptureResult() function. BUG=637087 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] A quick fix of bug 637087 IpcVideoFrameCapturer may be released before DesktopSessionAgent returns a frame, so we should check before calling IpcVideoFrameCapturer::OnCaptureResult() function. BUG=637087 ========== to ========== [Chromoting] A quick fix of bug 637087 IpcVideoFrameCapturer may be released before DesktopSessionAgent returns a frame, so we should check before calling IpcVideoFrameCapturer::OnCaptureResult() function. BUG=637087 Committed: https://crrev.com/7f1e89c1582b6f936cd858e950ea9f2dce3e8a6a Cr-Commit-Position: refs/heads/master@{#411929} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/7f1e89c1582b6f936cd858e950ea9f2dce3e8a6a Cr-Commit-Position: refs/heads/master@{#411929} |
