0

[PDF] Avoid copying PaintReadyRect in production code

Add move ctor / operator= to PaintReadyRect, and use it in production
code. The copy ctor / operator= still has to remain for use in tests,
where the code that does aggregate construction gets confused if there
is only a move ctor. Also change PaintAggregator::GetReadyRects() to
TakeReadyRects(), to move PaintReadyRects around instead of copying
them.

Change-Id: I9266c4a6858ff6c91533438e9aed2f07ddf0e6b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5938729
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1370267}
This commit is contained in:
Lei Zhang
2024-10-17 21:40:57 +00:00
committed by Chromium LUCI CQ
parent 086fec1d07
commit dc0e67daec
5 changed files with 35 additions and 23 deletions

@ -7,6 +7,8 @@
#include <stddef.h>
#include <stdint.h>
#include <utility>
#include "base/check.h"
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/rect.h"
@ -130,16 +132,16 @@ PaintAggregator::PaintUpdate PaintAggregator::GetPendingUpdate() {
return ret;
}
void PaintAggregator::SetIntermediateResults(
const std::vector<PaintReadyRect>& ready,
const std::vector<gfx::Rect>& pending) {
update_.ready_rects.insert(update_.ready_rects.end(), ready.begin(),
ready.end());
update_.paint_rects = pending;
void PaintAggregator::SetIntermediateResults(std::vector<PaintReadyRect> ready,
std::vector<gfx::Rect> pending) {
for (auto& rect : ready) {
update_.ready_rects.insert(update_.ready_rects.end(), std::move(rect));
}
update_.paint_rects = std::move(pending);
}
std::vector<PaintReadyRect> PaintAggregator::GetReadyRects() const {
return update_.ready_rects;
std::vector<PaintReadyRect> PaintAggregator::TakeReadyRects() {
return std::move(update_.ready_rects);
}
void PaintAggregator::InvalidateRect(const gfx::Rect& rect) {

@ -66,14 +66,16 @@ class PaintAggregator {
PaintUpdate GetPendingUpdate();
// Sets the result of a call to the plugin to paint. This includes rects that
// are finished painting (ready), and ones that are still in-progress
// (pending).
void SetIntermediateResults(const std::vector<PaintReadyRect>& ready,
const std::vector<gfx::Rect>& pending);
// Sets the result of a call to the plugin to paint.
//
// - `ready` includes rects that are finished painting.
// - `pending` includes rects that are still in-progress.
void SetIntermediateResults(std::vector<PaintReadyRect> ready,
std::vector<gfx::Rect> pending);
// Returns the rectangles that are ready to be painted.
std::vector<PaintReadyRect> GetReadyRects() const;
// Returns the rectangles that are ready to be painted. Caller takes
// ownership.
std::vector<PaintReadyRect> TakeReadyRects();
// The given rect should be repainted.
void InvalidateRect(const gfx::Rect& rect);

@ -243,8 +243,9 @@ void PaintManager::DoPaint() {
std::vector<PaintReadyRect> ready_now;
if (pending_rects.empty()) {
aggregator_.SetIntermediateResults(ready_rects, pending_rects);
ready_now = aggregator_.GetReadyRects();
aggregator_.SetIntermediateResults(std::move(ready_rects),
std::move(pending_rects));
ready_now = aggregator_.TakeReadyRects();
aggregator_.ClearPendingUpdate();
// First, apply any scroll amount less than the surface's size.
@ -259,7 +260,7 @@ void PaintManager::DoPaint() {
view_size_changed_waiting_for_paint_ = false;
} else {
std::vector<PaintReadyRect> ready_later;
for (const auto& ready_rect : ready_rects) {
for (auto& ready_rect : ready_rects) {
// Don't flush any part (i.e. scrollbars) if we're resizing the browser,
// as that'll lead to flashes. Until we flush, the browser will use the
// previous image, but if we flush, it'll revert to using the blank image.
@ -267,14 +268,15 @@ void PaintManager::DoPaint() {
// default background color instead of the pepper default of black.
if (ready_rect.flush_now() &&
(!view_size_changed_waiting_for_paint_ || first_paint_)) {
ready_now.push_back(ready_rect);
ready_now.push_back(std::move(ready_rect));
} else {
ready_later.push_back(ready_rect);
ready_later.push_back(std::move(ready_rect));
}
}
// Take the rectangles, except the ones that need to be flushed right away,
// and save them so that everything is flushed at once.
aggregator_.SetIntermediateResults(ready_later, pending_rects);
aggregator_.SetIntermediateResults(std::move(ready_later),
std::move(pending_rects));
if (ready_now.empty()) {
EnsureCallbackPending();

@ -23,6 +23,10 @@ PaintReadyRect::PaintReadyRect(const gfx::Rect& rect,
CHECK(image_);
}
PaintReadyRect::PaintReadyRect(PaintReadyRect&&) noexcept = default;
PaintReadyRect& PaintReadyRect::operator=(PaintReadyRect&&) noexcept = default;
PaintReadyRect::PaintReadyRect(const PaintReadyRect& other) = default;
PaintReadyRect& PaintReadyRect::operator=(const PaintReadyRect& other) =

@ -19,8 +19,10 @@ class PaintReadyRect {
public:
PaintReadyRect(const gfx::Rect& rect, sk_sp<SkImage> image);
PaintReadyRect(const gfx::Rect& rect, sk_sp<SkImage> image, bool flush_now);
PaintReadyRect(const PaintReadyRect& other);
PaintReadyRect& operator=(const PaintReadyRect& other);
PaintReadyRect(PaintReadyRect&&) noexcept;
PaintReadyRect& operator=(PaintReadyRect&&) noexcept;
PaintReadyRect(const PaintReadyRect&);
PaintReadyRect& operator=(const PaintReadyRect&);
~PaintReadyRect();
const gfx::Rect& rect() const { return rect_; }