Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(804)

Issue 1996753002: Fix DesktopSessioAgent crash when handling nullptr OnCaptureCompleted callback. (Closed)

Created:
4 years, 7 months ago by Hzj_jie
Modified:
4 years, 7 months ago
Reviewers:
Sergey Ulanov, Jamie
CC:
chromium-reviews, chromoting-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix DesktopSessioAgent crash when handling nullptr OnCaptureCompleted callback. When DesktopSessionAgent receives a nullptr DesktopFrame, it will send an empty SerializedDesktopFrame (with dimensions equal to zero) to DesktopSessionProxy, and DesktopSessionProxy will also send a nullptr DesktopFrame. BUG= Committed: https://crrev.com/68440da3c34d06a6b1c48ab866c5f3f21f270d6a Cr-Commit-Position: refs/heads/master@{#396281}

Patch Set 1 #

Total comments: 2

Patch Set 2 : DesktopSessionProxy needs to be aware of unchanged frames. #

Patch Set 3 : Return an empty SerializedDesktopFrame if input DesktopFrame is nullptr #

Total comments: 2

Patch Set 4 : Share two --pending_capture_frame_requests_ calls #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -14 lines) Patch
M remoting/host/desktop_session_agent.h View 1 1 chunk +0 lines, -3 lines 0 comments Download
M remoting/host/desktop_session_agent.cc View 1 2 1 chunk +10 lines, -10 lines 0 comments Download
M remoting/host/desktop_session_proxy.cc View 1 2 3 2 chunks +9 lines, -1 line 0 comments Download

Messages

Total messages: 22 (13 generated)
Hzj_jie
4 years, 7 months ago (2016-05-19 21:20:52 UTC) #3
Sergey Ulanov
https://codereview.chromium.org/1996753002/diff/20001/remoting/host/desktop_session_agent.cc File remoting/host/desktop_session_agent.cc (right): https://codereview.chromium.org/1996753002/diff/20001/remoting/host/desktop_session_agent.cc#newcode329 remoting/host/desktop_session_agent.cc:329: if (!frame) { I don't think we can just ...
4 years, 7 months ago (2016-05-19 22:23:52 UTC) #7
Hzj_jie
https://codereview.chromium.org/1996753002/diff/20001/remoting/host/desktop_session_agent.cc File remoting/host/desktop_session_agent.cc (right): https://codereview.chromium.org/1996753002/diff/20001/remoting/host/desktop_session_agent.cc#newcode329 remoting/host/desktop_session_agent.cc:329: if (!frame) { On 2016/05/19 22:23:52, Sergey Ulanov wrote: ...
4 years, 7 months ago (2016-05-20 00:11:15 UTC) #8
Hzj_jie
This change is ready to be reviewed.
4 years, 7 months ago (2016-05-20 21:39:15 UTC) #13
Sergey Ulanov
lgtm when my nit is addressed https://codereview.chromium.org/1996753002/diff/60001/remoting/host/desktop_session_proxy.cc File remoting/host/desktop_session_proxy.cc (right): https://codereview.chromium.org/1996753002/diff/60001/remoting/host/desktop_session_proxy.cc#newcode477 remoting/host/desktop_session_proxy.cc:477: --pending_capture_frame_requests_; Move this ...
4 years, 7 months ago (2016-05-26 12:20:10 UTC) #14
Hzj_jie
https://codereview.chromium.org/1996753002/diff/60001/remoting/host/desktop_session_proxy.cc File remoting/host/desktop_session_proxy.cc (right): https://codereview.chromium.org/1996753002/diff/60001/remoting/host/desktop_session_proxy.cc#newcode477 remoting/host/desktop_session_proxy.cc:477: --pending_capture_frame_requests_; On 2016/05/26 12:20:10, Sergey Ulanov (OOO) wrote: > ...
4 years, 7 months ago (2016-05-26 20:59:22 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996753002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996753002/80001
4 years, 7 months ago (2016-05-26 21:00:40 UTC) #18
commit-bot: I haz the power
Committed patchset #4 (id:80001)
4 years, 7 months ago (2016-05-26 21:06:52 UTC) #20
commit-bot: I haz the power
4 years, 7 months ago (2016-05-26 21:08:20 UTC) #22
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/68440da3c34d06a6b1c48ab866c5f3f21f270d6a
Cr-Commit-Position: refs/heads/master@{#396281}

Powered by Google App Engine
This is Rietveld 408576698