Fix crash when resizing the display

This crash happened when:
   * The cursor was located in the corner
   * The screen was resized so that the cursor position is outside of the frame.

This caused us to add out-of-bound coordinates to the frame's updated_region, which caused crashes further down the pipeline.

Bug: chromium:1323241
Test: new unittest
Test: manually reproduced crash
Change-Id: Ie71db58c8a347f00af8a3803fcd55cdcad6eafac
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261263
Reviewed-by: Alexander Cooper <alcooper@chromium.org>
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Cr-Commit-Position: refs/heads/main@{#36809}
This commit is contained in:
Jeroen Dhollander 2022-05-06 14:47:13 +02:00 committed by WebRTC LUCI CQ
parent 42a829e623
commit 77d987a724
2 changed files with 60 additions and 7 deletions

View File

@ -105,7 +105,11 @@ DesktopFrameWithCursor::DesktopFrameWithCursor(
if (!previous_cursor_rect.equals(cursor_rect_)) {
mutable_updated_region()->AddRect(cursor_rect_);
mutable_updated_region()->AddRect(previous_cursor_rect);
// Only add the previous cursor if it is inside the frame.
// (it can be outside the frame if the desktop has been resized).
if (original_frame_->rect().ContainsRect(previous_cursor_rect))
mutable_updated_region()->AddRect(previous_cursor_rect);
} else if (cursor_changed) {
mutable_updated_region()->AddRect(cursor_rect_);
}

View File

@ -15,18 +15,22 @@
#include <memory>
#include <utility>
#include <vector>
#include "modules/desktop_capture/desktop_capturer.h"
#include "modules/desktop_capture/desktop_frame.h"
#include "modules/desktop_capture/mouse_cursor.h"
#include "modules/desktop_capture/shared_desktop_frame.h"
#include "rtc_base/arraysize.h"
#include "test/gmock.h"
#include "test/gtest.h"
namespace webrtc {
namespace {
using testing::ElementsAre;
const int kFrameXCoord = 100;
const int kFrameYCoord = 200;
const int kScreenWidth = 100;
@ -75,12 +79,12 @@ uint32_t BlendPixels(uint32_t dest, uint32_t src) {
return b + (g << 8) + (r << 16) + 0xff000000;
}
DesktopFrame* CreateTestFrame() {
DesktopFrame* frame =
new BasicDesktopFrame(DesktopSize(kScreenWidth, kScreenHeight));
DesktopFrame* CreateTestFrame(int width = kScreenWidth,
int height = kScreenHeight) {
DesktopFrame* frame = new BasicDesktopFrame(DesktopSize(width, height));
uint32_t* data = reinterpret_cast<uint32_t*>(frame->data());
for (int y = 0; y < kScreenHeight; ++y) {
for (int x = 0; x < kScreenWidth; ++x) {
for (int y = 0; y < height; ++y) {
for (int x = 0; x < width; ++x) {
*(data++) = GetFakeFramePixelValue(DesktopVector(x, y));
}
}
@ -101,6 +105,15 @@ MouseCursor* CreateTestCursor(DesktopVector hotspot) {
return new MouseCursor(image.release(), hotspot);
}
std::vector<DesktopRect> GetUpdatedRegions(const DesktopFrame& frame) {
std::vector<DesktopRect> result;
for (webrtc::DesktopRegion::Iterator i(frame.updated_region()); !i.IsAtEnd();
i.Advance()) {
result.push_back(i.rect());
}
return result;
}
class FakeScreenCapturer : public DesktopCapturer {
public:
FakeScreenCapturer() {}
@ -184,10 +197,20 @@ void VerifyFrame(const DesktopFrame& frame,
} // namespace
bool operator==(const DesktopRect& left, const DesktopRect& right) {
return left.equals(right);
}
std::ostream& operator<<(std::ostream& out, const DesktopRect& rect) {
out << "{" << rect.left() << "+" << rect.top() << "-" << rect.width() << "x"
<< rect.height() << "}";
return out;
}
class DesktopAndCursorComposerTest : public ::testing::Test,
public DesktopCapturer::Callback {
public:
DesktopAndCursorComposerTest(bool include_cursor = true)
explicit DesktopAndCursorComposerTest(bool include_cursor = true)
: fake_screen_(new FakeScreenCapturer()),
fake_cursor_(include_cursor ? new FakeMouseMonitor() : nullptr),
blender_(fake_screen_, fake_cursor_) {
@ -413,6 +436,32 @@ TEST_F(DesktopAndCursorComposerNoCursorMonitorTest,
EXPECT_TRUE(frame_->updated_region().Equals(expected_region));
}
TEST_F(DesktopAndCursorComposerNoCursorMonitorTest,
UpdatedRegionDoesNotIncludeOldCursorIfOutOfBounds) {
blender_.OnMouseCursor(CreateTestCursor(DesktopVector(0, 0)));
std::unique_ptr<SharedDesktopFrame> first_frame(
SharedDesktopFrame::Wrap(CreateTestFrame(1000, 1000)));
blender_.OnMouseCursorPosition(DesktopVector(900, 900));
fake_screen_->SetNextFrame(first_frame->Share());
blender_.CaptureFrame();
// Second frame is smaller than first frame, and the first cursor is outside
// of the bounds of the new frame, so it should not be in the updated region.
std::unique_ptr<SharedDesktopFrame> second_frame(
SharedDesktopFrame::Wrap(CreateTestFrame(500, 500)));
auto second_cursor_rect =
DesktopRect::MakeXYWH(400, 400, kCursorWidth, kCursorHeight);
blender_.OnMouseCursorPosition(DesktopVector(400, 400));
fake_screen_->SetNextFrame(second_frame->Share());
blender_.CaptureFrame();
DesktopRegion expected_region;
expected_region.AddRect(second_cursor_rect);
EXPECT_THAT(GetUpdatedRegions(*frame_), ElementsAre(second_cursor_rect));
}
TEST_F(DesktopAndCursorComposerNoCursorMonitorTest,
UpdatedRegionIncludesOldAndNewCursorRectsIfShapeChanged) {
std::unique_ptr<SharedDesktopFrame> frame(