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

Issue 965123002: Crash fix for desktop capture size calculations, and some minor things. (Closed)

Created:
5 years, 9 months ago by miu
Modified:
5 years, 9 months ago
Reviewers:
hshi1
CC:
chromium-reviews, darin-cc_chromium.org, feature-media-reviews_chromium.org, jam, miu+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Crash fix for desktop capture size calculations, and some minor things. When stress-testing desktop resize (by running a CrOS Ash desktop in a window), a problem with the capture frame size calculations was revealed: The max frame size was being updated for each change in desktop size. This meant that once width or height were reduced, they could never be increased again. When the width or height were reduced to zero, Chrome would crash. This change fixes the size calculation (the max size is made constant from construction), adds a few extra sanity-checks to prevent crashes on OOM or zero frame sizes, and also adds re-scaling of the rendered mouse cursor bitmap whenever the desktop size has changed. BUG=462799 TEST=Resized CrOS Ash-desktop-in-a-window on my local dev workstation to confirm crash is resolved. Committed: https://crrev.com/db73faeca6f1a3a6cb2923afc2e0bc3ea995aee7 Cr-Commit-Position: refs/heads/master@{#318958}

Patch Set 1 #

Patch Set 2 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+61 lines, -47 lines) Patch
M content/browser/media/capture/content_video_capture_device_core.h View 1 2 chunks +7 lines, -6 lines 0 comments Download
M content/browser/media/capture/content_video_capture_device_core.cc View 1 5 chunks +28 lines, -35 lines 0 comments Download
M content/browser/media/capture/desktop_capture_device_aura.cc View 6 chunks +26 lines, -6 lines 0 comments Download

Messages

Total messages: 12 (5 generated)
miu
hshi: Could you PTAL?
5 years, 9 months ago (2015-02-28 03:03:17 UTC) #2
hshi1
lgtm
5 years, 9 months ago (2015-03-03 20:03:41 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/965123002/1
5 years, 9 months ago (2015-03-03 21:05:33 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: android_aosp on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/builds/64101) ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, ...
5 years, 9 months ago (2015-03-03 21:10:45 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/965123002/20001
5 years, 9 months ago (2015-03-03 21:26:18 UTC) #10
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 9 months ago (2015-03-03 23:12:04 UTC) #11
commit-bot: I haz the power
5 years, 9 months ago (2015-03-03 23:12:51 UTC) #12
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/db73faeca6f1a3a6cb2923afc2e0bc3ea995aee7
Cr-Commit-Position: refs/heads/master@{#318958}

Powered by Google App Engine
This is Rietveld 408576698