Skip to content

Commit

Permalink
[GLImage] Restrict GLImageMemory creation
Browse files Browse the repository at this point in the history
GLImageMemory is now created in production only from the relevant
SharedImage backing. This CL hardcodes this restriction to ensure that
it does not regress as we seek to eliminate GLImage.

Note that GLImage is a start-from-0 refcounted type (i.e., the default),
for which the correct conversion from base::MakeRefCounted(...) when
making the constructor private with a friend class is
base::WrapRefCounted(new T(...)).

Bug: 1310018, 1382031
Change-Id: I52851f394f99661faef891469993b89b98b5a8f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4055102
Reviewed-by: Peng Huang <penghuang@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1076757}
  • Loading branch information
colinblundell authored and Chromium LUCI CQ committed Nov 29, 2022
1 parent ce0f365 commit 549a43f
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 7 deletions.
4 changes: 2 additions & 2 deletions components/viz/test/test_image_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ namespace viz {

namespace {

class GLImageSharedMemory : public gl::GLImageMemory {
class GLImageSharedMemory : public gl::GLImageMemoryForTesting {
public:
explicit GLImageSharedMemory(const gfx::Size& size)
: gl::GLImageMemory(size) {}
: gl::GLImageMemoryForTesting(size) {}

GLImageSharedMemory(const GLImageSharedMemory&) = delete;
GLImageSharedMemory& operator=(const GLImageSharedMemory&) = delete;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ SharedMemoryImageBacking::ProduceOverlay(SharedImageManager* manager,
if (!shared_memory_wrapper_.IsValid())
return nullptr;

auto gl_image = base::MakeRefCounted<gl::GLImageMemory>(size());
auto gl_image =
base::WrapRefCounted<gl::GLImageMemory>(new gl::GLImageMemory(size()));
if (!gl_image->Initialize(shared_memory_wrapper_.GetMemory(),
ToBufferFormat(format()),
shared_memory_wrapper_.GetStride(),
Expand Down
4 changes: 2 additions & 2 deletions ui/gl/direct_composition_surface_win_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@
namespace gl {
namespace {

class GLImageRefCountedMemory : public GLImageMemory {
class GLImageRefCountedMemory : public GLImageMemoryForTesting {
public:
explicit GLImageRefCountedMemory(const gfx::Size& size)
: GLImageMemory(size) {}
: GLImageMemoryForTesting(size) {}

GLImageRefCountedMemory(const GLImageRefCountedMemory&) = delete;
GLImageRefCountedMemory& operator=(const GLImageRefCountedMemory&) = delete;
Expand Down
5 changes: 5 additions & 0 deletions ui/gl/gl_image_memory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -494,4 +494,9 @@ bool GLImageMemory::ValidFormat(gfx::BufferFormat format) {
return false;
}

GLImageMemoryForTesting::GLImageMemoryForTesting(const gfx::Size& size)
: GLImageMemory(size) {}

GLImageMemoryForTesting::~GLImageMemoryForTesting() = default;

} // namespace gl
28 changes: 26 additions & 2 deletions ui/gl/gl_image_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,17 @@
#include "ui/gfx/buffer_types.h"
#include "ui/gl/gl_export.h"

namespace gpu {
class SharedMemoryImageBacking;
}

namespace gl {
class GLContext;
class GLImageMemoryForTesting;
class GLSurface;

class GL_EXPORT GLImageMemory : public GLImage {
public:
explicit GLImageMemory(const gfx::Size& size);

GLImageMemory(const GLImageMemory&) = delete;
GLImageMemory& operator=(const GLImageMemory&) = delete;

Expand Down Expand Up @@ -55,6 +58,16 @@ class GL_EXPORT GLImageMemory : public GLImage {
~GLImageMemory() override;

private:
// Make constructor private to ensure that only specified friend classes can
// create GLImageMemory instances.
explicit GLImageMemory(const gfx::Size& size);

// GLImageMemory should be created in production only by
// SharedMemoryImageBacking. Some tests need to subclass GLImageMemory in
// anonymous namespaces, for which GLImageMemoryForTesting exists.
friend class gpu::SharedMemoryImageBacking;
friend class GLImageMemoryForTesting;

static bool ValidFormat(gfx::BufferFormat format);

const gfx::Size size_;
Expand All @@ -70,6 +83,17 @@ class GL_EXPORT GLImageMemory : public GLImage {
int memcpy_tasks_ = 0;
};

// GLImageMemoryForTesting supports test use cases for subclassing
// GLImageMemory in anonymous namespaces. This class should never be used in
// production.
class GL_EXPORT GLImageMemoryForTesting : public GLImageMemory {
protected:
explicit GLImageMemoryForTesting(const gfx::Size& size);

protected:
~GLImageMemoryForTesting() override;
};

} // namespace gl

#endif // UI_GL_GL_IMAGE_MEMORY_H_

0 comments on commit 549a43f

Please sign in to comment.