Skip to content

Commit

Permalink
Move Mailbox from cc to gpu, and its traits to gpu/ipc
Browse files Browse the repository at this point in the history
Mailbox is safer to IPC than coaxing it into a string, because otherwise we need
to validate the size when coming from untrusted sources.

This is to enable passing Mailbox through IPC without having to depend on cc.
Among others I will need it to move Pepper to mailboxes, and pepper can't depend
on cc.

BUG=164095


Review URL: https://chromiumcodereview.appspot.com/12378053

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@186006 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
piman@chromium.org committed Mar 4, 2013
1 parent 35fb693 commit 1aca009
Show file tree
Hide file tree
Showing 22 changed files with 130 additions and 87 deletions.
1 change: 1 addition & 0 deletions cc/DEPS
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
include_rules = [
"+gpu/GLES2",
"+gpu/command_buffer/common/mailbox.h",
"+media",
"+skia/ext",
"+third_party/skia/include",
Expand Down
1 change: 1 addition & 0 deletions cc/cc.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@
'dependencies': [
'<(DEPTH)/base/base.gyp:base',
'<(DEPTH)/base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
'<(DEPTH)/gpu/gpu.gyp:gpu',
'<(DEPTH)/skia/skia.gyp:skia',
'<(DEPTH)/media/media.gyp:media',
'<(DEPTH)/ui/gl/gl.gyp:gl',
Expand Down
1 change: 1 addition & 0 deletions cc/cc_tests.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@
'type': '<(gtest_target_type)',
'dependencies': [
'../base/base.gyp:test_support_base',
'../gpu/gpu.gyp:gpu',
'../media/media.gyp:media',
'../skia/skia.gyp:skia',
'../testing/gmock.gyp:gmock',
Expand Down
4 changes: 2 additions & 2 deletions cc/gl_frame_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

#include "base/basictypes.h"
#include "cc/cc_export.h"
#include "cc/transferable_resource.h"
#include "gpu/command_buffer/common/mailbox.h"
#include "ui/gfx/size.h"

namespace cc {
Expand All @@ -19,7 +19,7 @@ class CC_EXPORT GLFrameData {
GLFrameData();
~GLFrameData();

Mailbox mailbox;
gpu::Mailbox mailbox;
uint32 sync_point;
gfx::Size size;
};
Expand Down
2 changes: 1 addition & 1 deletion cc/resource_provider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ TEST_P(ResourceProviderTest, TransferMailboxResources)
context()->bindTexture(GL_TEXTURE_2D, texture);
uint8_t data[4] = {1, 2, 3, 4};
context()->texImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, &data);
Mailbox mailbox;
gpu::Mailbox mailbox;
context()->genMailboxCHROMIUM(mailbox.name);
context()->produceTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
unsigned syncPoint = context()->insertSyncPoint();
Expand Down
8 changes: 4 additions & 4 deletions cc/texture_layer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,11 @@ struct CommonMailboxObjects {
m_releaseMailbox2 = base::Bind(&MockMailboxCallback::Release,
base::Unretained(&m_mockCallback),
m_mailboxName2);
Mailbox m1;
m1.setName(reinterpret_cast<const int8*>(m_mailboxName1.data()));
gpu::Mailbox m1;
m1.SetName(reinterpret_cast<const int8*>(m_mailboxName1.data()));
m_mailbox1 = TextureMailbox(m1, m_releaseMailbox1, m_syncPoint1);
Mailbox m2;
m2.setName(reinterpret_cast<const int8*>(m_mailboxName2.data()));
gpu::Mailbox m2;
m2.SetName(reinterpret_cast<const int8*>(m_mailboxName2.data()));
m_mailbox2 = TextureMailbox(m2, m_releaseMailbox2, m_syncPoint2);
}

Expand Down
22 changes: 11 additions & 11 deletions cc/texture_mailbox.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,33 +19,33 @@ TextureMailbox::TextureMailbox(
DCHECK(mailbox_name.empty() == mailbox_callback.is_null());
if (!mailbox_name.empty()) {
CHECK(mailbox_name.size() == sizeof(name_.name));
name_.setName(reinterpret_cast<const int8*>(mailbox_name.data()));
name_.SetName(reinterpret_cast<const int8*>(mailbox_name.data()));
}
}

TextureMailbox::TextureMailbox(
const Mailbox& mailbox_name,
const gpu::Mailbox& mailbox_name,
const ReleaseCallback& mailbox_callback)
: callback_(mailbox_callback),
sync_point_(0) {
DCHECK(mailbox_name.isZero() == mailbox_callback.is_null());
name_.setName(mailbox_name.name);
DCHECK(mailbox_name.IsZero() == mailbox_callback.is_null());
name_.SetName(mailbox_name.name);
}

TextureMailbox::TextureMailbox(
const Mailbox& mailbox_name,
const gpu::Mailbox& mailbox_name,
const ReleaseCallback& mailbox_callback,
unsigned sync_point)
: callback_(mailbox_callback),
sync_point_(sync_point) {
DCHECK(mailbox_name.isZero() == mailbox_callback.is_null());
name_.setName(mailbox_name.name);
DCHECK(mailbox_name.IsZero() == mailbox_callback.is_null());
name_.SetName(mailbox_name.name);
}

TextureMailbox::~TextureMailbox() {
}

bool TextureMailbox::Equals(const Mailbox& other) const {
bool TextureMailbox::Equals(const gpu::Mailbox& other) const {
return !memcmp(data(), other.name, sizeof(name_.name));
}

Expand All @@ -54,16 +54,16 @@ bool TextureMailbox::Equals(const TextureMailbox& other) const {
}

bool TextureMailbox::IsEmpty() const {
return name_.isZero();
return name_.IsZero();
}

void TextureMailbox::RunReleaseCallback(unsigned sync_point) const {
if (!callback_.is_null())
callback_.Run(sync_point);
}

void TextureMailbox::SetName(const Mailbox& other) {
name_.setName(other.name);
void TextureMailbox::SetName(const gpu::Mailbox& other) {
name_.SetName(other.name);
}

} // namespace cc
14 changes: 7 additions & 7 deletions cc/texture_mailbox.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "base/basictypes.h"
#include "base/callback.h"
#include "cc/cc_export.h"
#include "cc/transferable_resource.h"
#include "gpu/command_buffer/common/mailbox.h"

namespace cc {

Expand All @@ -20,27 +20,27 @@ class CC_EXPORT TextureMailbox {
TextureMailbox();
TextureMailbox(const std::string& mailbox_name,
const ReleaseCallback& callback);
TextureMailbox(const Mailbox& mailbox_name,
TextureMailbox(const gpu::Mailbox& mailbox_name,
const ReleaseCallback& callback);
TextureMailbox(const Mailbox& mailbox_name,
TextureMailbox(const gpu::Mailbox& mailbox_name,
const ReleaseCallback& callback,
unsigned sync_point);

~TextureMailbox();

const ReleaseCallback& callback() const { return callback_; }
const int8* data() const { return name_.name; }
bool Equals(const Mailbox&) const;
bool Equals(const gpu::Mailbox&) const;
bool Equals(const TextureMailbox&) const;
bool IsEmpty() const;
const Mailbox& name() const { return name_; }
const gpu::Mailbox& name() const { return name_; }
void ResetSyncPoint() { sync_point_ = 0; }
void RunReleaseCallback(unsigned sync_point) const;
void SetName(const Mailbox&);
void SetName(const gpu::Mailbox&);
unsigned sync_point() const { return sync_point_; }

private:
Mailbox name_;
gpu::Mailbox name_;
ReleaseCallback callback_;
unsigned sync_point_;
};
Expand Down
17 changes: 0 additions & 17 deletions cc/transferable_resource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,6 @@

namespace cc {

Mailbox::Mailbox() {
memset(name, 0, sizeof(name));
}

bool Mailbox::isZero() const {
for (int i = 0; i < arraysize(name); ++i) {
if (name[i])
return false;
}
return true;
}

void Mailbox::setName(const int8* n) {
DCHECK(isZero() || !memcmp(name, n, sizeof(name)));
memcpy(name, n, sizeof(name));
}

TransferableResource::TransferableResource()
: id(0),
sync_point(0),
Expand Down
10 changes: 2 additions & 8 deletions cc/transferable_resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,11 @@

#include "base/basictypes.h"
#include "cc/cc_export.h"
#include "gpu/command_buffer/common/mailbox.h"
#include "ui/gfx/size.h"

namespace cc {

struct CC_EXPORT Mailbox {
Mailbox();
bool isZero() const;
void setName(const int8* name);
int8 name[64];
};

struct CC_EXPORT TransferableResource {
TransferableResource();
~TransferableResource();
Expand All @@ -29,7 +23,7 @@ struct CC_EXPORT TransferableResource {
uint32 format;
uint32 filter;
gfx::Size size;
Mailbox mailbox;
gpu::Mailbox mailbox;
};

typedef std::vector<TransferableResource> TransferableResourceArray;
Expand Down
4 changes: 2 additions & 2 deletions content/browser/renderer_host/render_widget_host_view_aura.cc
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ bool PointerEventActivates(const ui::Event& event) {
void SendCompositorFrameAck(
int32 route_id,
int renderer_host_id,
const cc::Mailbox& received_mailbox,
const gpu::Mailbox& received_mailbox,
const gfx::Size& received_size,
bool skip_frame,
const scoped_refptr<ui::Texture>& texture_to_produce) {
Expand Down Expand Up @@ -1252,7 +1252,7 @@ void RenderWidgetHostViewAura::AcceleratedSurfaceBuffersSwapped(

void RenderWidgetHostViewAura::OnSwapCompositorFrame(
const cc::CompositorFrame& frame) {
if (!frame.gl_frame_data || frame.gl_frame_data->mailbox.isZero())
if (!frame.gl_frame_data || frame.gl_frame_data->mailbox.IsZero())
return;

BufferPresentedCallback ack_callback = base::Bind(
Expand Down
20 changes: 0 additions & 20 deletions content/common/cc_messages.cc
Original file line number Diff line number Diff line change
Expand Up @@ -536,26 +536,6 @@ void ParamTraits<cc::RenderPass>::Log(
l->append("])");
}

void ParamTraits<cc::Mailbox>::Write(Message* m, const param_type& p) {
m->WriteBytes(p.name, sizeof(p.name));
}

bool ParamTraits<cc::Mailbox>::Read(const Message* m,
PickleIterator* iter,
param_type* p) {
const char* bytes = NULL;
if (!m->ReadBytes(iter, &bytes, sizeof(p->name)))
return false;
DCHECK(bytes);
memcpy(p->name, bytes, sizeof(p->name));
return true;
}

void ParamTraits<cc::Mailbox>::Log(const param_type& p, std::string* l) {
for (size_t i = 0; i < sizeof(p.name); ++i)
*l += base::StringPrintf("%02x", p.name[i]);
}

namespace {
enum CompositorFrameType {
NO_FRAME,
Expand Down
9 changes: 1 addition & 8 deletions content/common/cc_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "cc/video_layer_impl.h"
#include "cc/yuv_video_draw_quad.h"
#include "content/common/content_export.h"
#include "gpu/ipc/gpu_command_buffer_traits.h"
#include "ipc/ipc_message_macros.h"
#include "third_party/WebKit/Source/Platform/chromium/public/WebFilterOperation.h"

Expand Down Expand Up @@ -74,14 +75,6 @@ struct CONTENT_EXPORT ParamTraits<cc::RenderPass> {
static void Log(const param_type& p, std::string* l);
};

template<>
struct CONTENT_EXPORT ParamTraits<cc::Mailbox> {
typedef cc::Mailbox param_type;
static void Write(Message* m, const param_type& p);
static bool Read(const Message* m, PickleIterator* iter, param_type* p);
static void Log(const param_type& p, std::string* l);
};

template<>
struct CONTENT_EXPORT ParamTraits<cc::CompositorFrame> {
typedef cc::CompositorFrame param_type;
Expand Down
4 changes: 2 additions & 2 deletions content/common/cc_messages_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -508,15 +508,15 @@ TEST_F(CCMessagesTest, Resources) {
arbitrary_resource1.format = 7;
arbitrary_resource1.filter = 53;
arbitrary_resource1.size = gfx::Size(37189, 123123);
arbitrary_resource1.mailbox.setName(arbitrary_mailbox1);
arbitrary_resource1.mailbox.SetName(arbitrary_mailbox1);

TransferableResource arbitrary_resource2;
arbitrary_resource2.id = 789132;
arbitrary_resource1.sync_point = arbitrary_uint2;
arbitrary_resource2.format = 30;
arbitrary_resource1.filter = 47;
arbitrary_resource2.size = gfx::Size(89123, 23789);
arbitrary_resource2.mailbox.setName(arbitrary_mailbox2);
arbitrary_resource2.mailbox.SetName(arbitrary_mailbox2);

DelegatedFrameData frame_in;
frame_in.resource_list.push_back(arbitrary_resource1);
Expand Down
1 change: 1 addition & 0 deletions content/content_tests.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,7 @@
'content_renderer',
'content_resources.gyp:content_resources',
'../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
'../gpu/gpu.gyp:gpu',
'../gpu/gpu.gyp:gpu_unittest_utils',
'../ipc/ipc.gyp:test_support_ipc',
'../jingle/jingle.gyp:jingle_glue_test_util',
Expand Down
6 changes: 3 additions & 3 deletions content/renderer/gpu/mailbox_output_surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

using cc::CompositorFrame;
using cc::GLFrameData;
using cc::Mailbox;
using gpu::Mailbox;
using WebKit::WebGraphicsContext3D;

namespace content {
Expand Down Expand Up @@ -113,7 +113,7 @@ void MailboxOutputSurface::SendFrameToParentCompositor(

DCHECK(!size_.IsEmpty());
DCHECK(size_ == current_backing_.size);
DCHECK(!current_backing_.mailbox.isZero());
DCHECK(!current_backing_.mailbox.IsZero());

context3d_->framebufferTexture2D(
GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0);
Expand All @@ -132,7 +132,7 @@ void MailboxOutputSurface::SendFrameToParentCompositor(
}

void MailboxOutputSurface::OnSwapAck(const cc::CompositorFrameAck& ack) {
if (!ack.gl_frame_data->mailbox.isZero()) {
if (!ack.gl_frame_data->mailbox.IsZero()) {
DCHECK(!ack.gl_frame_data->size.IsEmpty());
uint32 texture_id = context3d_->createTexture();
TransferableFrame texture(
Expand Down
4 changes: 2 additions & 2 deletions content/renderer/gpu/mailbox_output_surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ class MailboxOutputSurface : public CompositorOutputSurface {
: texture_id(0) {}

TransferableFrame(uint32 texture_id,
const cc::Mailbox& mailbox,
const gpu::Mailbox& mailbox,
const gfx::Size size)
: texture_id(texture_id),
mailbox(mailbox),
size(size) {}

uint32 texture_id;
cc::Mailbox mailbox;
gpu::Mailbox mailbox;
gfx::Size size;
};
TransferableFrame current_backing_;
Expand Down
30 changes: 30 additions & 0 deletions gpu/command_buffer/common/mailbox.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "../common/mailbox.h"

#include <string.h>

#include "../common/logging.h"

namespace gpu {

Mailbox::Mailbox() {
memset(name, 0, sizeof(name));
}

bool Mailbox::IsZero() const {
for (size_t i = 0; i < arraysize(name); ++i) {
if (name[i])
return false;
}
return true;
}

void Mailbox::SetName(const int8* n) {
GPU_DCHECK(IsZero() || !memcmp(name, n, sizeof(name)));
memcpy(name, n, sizeof(name));
}

} // namespace gpu
Loading

0 comments on commit 1aca009

Please sign in to comment.