Skip to content

Commit

Permalink
Blimp: Add BlobChannel Helium messages and delegate impls.
Browse files Browse the repository at this point in the history
* Add BlobChannelMessage protobuf definition.
* Add prettyprint logging for BlobChannelMessage.
* Add CreateBlimpMessage specialization for BlobChannelMessage.
* Add targeted unit tests for new Helium delegates.
* Move BlobData constness into BlobDataPtr, so that BlobData objects are mutable following their creation (necessary for passing payload buffers w/o copies).
* Refactor CreateBlobDataPtr into common location.
* Turn BlobChannelReceiver into an abstract interface for testing, moving implementation to BlobChannelReceiverImpl.

R=haibinlu@chromium.org,nyquist@chromium.org
CC=wez@chromium.org
BUG=600719

Review-Url: https://codereview.chromium.org/1970463004
Cr-Commit-Position: refs/heads/master@{#396271}
  • Loading branch information
kmarshall authored and Commit bot committed May 26, 2016
1 parent ca10709 commit 9dced90
Show file tree
Hide file tree
Showing 24 changed files with 501 additions and 91 deletions.
1 change: 1 addition & 0 deletions blimp/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ source_set("unit_tests") {

deps = [
":common",
":test_support",
"//base",
"//blimp/common/proto",
"//blimp/net:test_support",
Expand Down
7 changes: 5 additions & 2 deletions blimp/common/blob_cache/blob_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@
namespace blimp {

using BlobId = std::string;
using BlobData = base::RefCountedData<const std::string>;
using BlobDataPtr = scoped_refptr<BlobData>;
using BlobData = base::RefCountedData<std::string>;

// Immutable, ref-counted representation of blob payloads, suitable for sharing
// across threads.
using BlobDataPtr = scoped_refptr<const BlobData>;

// An interface for a cache of blobs.
class BLIMP_COMMON_EXPORT BlobCache {
Expand Down
5 changes: 1 addition & 4 deletions blimp/common/blob_cache/in_memory_blob_cache_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/memory/ref_counted.h"
#include "blimp/common/blob_cache/blob_cache.h"
#include "blimp/common/blob_cache/in_memory_blob_cache.h"
#include "blimp/common/blob_cache/test_util.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace blimp {
Expand All @@ -21,10 +22,6 @@ const char kBar[] = "bar";
const char kDeadbeef[] = "\xde\xad\xbe\xef";
const char kForbiddenCode[] = "\x4b\x1d\xc0\xd3";

BlobDataPtr CreateBlobDataPtr(const std::string& data) {
return new BlobData(data);
}

class InMemoryBlobCacheTest : public testing::Test {
public:
InMemoryBlobCacheTest() {}
Expand Down
8 changes: 8 additions & 0 deletions blimp/common/create_blimp_message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "base/logging.h"
#include "blimp/common/proto/blimp_message.pb.h"
#include "blimp/common/proto/blob_channel.pb.h"
#include "blimp/common/proto/compositor.pb.h"
#include "blimp/common/proto/input.pb.h"
#include "blimp/common/proto/render_widget.pb.h"
Expand Down Expand Up @@ -80,6 +81,13 @@ std::unique_ptr<BlimpMessage> CreateBlimpMessage(
return output;
}

std::unique_ptr<BlimpMessage> CreateBlimpMessage(
BlobChannelMessage** blob_channel_message) {
std::unique_ptr<BlimpMessage> output(new BlimpMessage);
*blob_channel_message = output->mutable_blob_channel();
return output;
}

std::unique_ptr<BlimpMessage> CreateStartConnectionMessage(
const std::string& client_token,
int protocol_version) {
Expand Down
4 changes: 4 additions & 0 deletions blimp/common/create_blimp_message.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
namespace blimp {

class BlimpMessage;
class BlobChannelMessage;
class CompositorMessage;
class EngineSettingsMessage;
class ImeMessage;
Expand Down Expand Up @@ -66,6 +67,9 @@ BLIMP_COMMON_EXPORT std::unique_ptr<BlimpMessage> CreateBlimpMessage(
BLIMP_COMMON_EXPORT std::unique_ptr<BlimpMessage> CreateBlimpMessage(
EngineSettingsMessage** engine_settings);

BLIMP_COMMON_EXPORT std::unique_ptr<BlimpMessage> CreateBlimpMessage(
BlobChannelMessage** blob_channel_message);

BLIMP_COMMON_EXPORT std::unique_ptr<BlimpMessage> CreateStartConnectionMessage(
const std::string& client_token,
int protocol_version);
Expand Down
9 changes: 9 additions & 0 deletions blimp/common/create_blimp_message_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/memory/ptr_util.h"
#include "blimp/common/create_blimp_message.h"
#include "blimp/common/proto/blimp_message.pb.h"
#include "blimp/common/proto/compositor.pb.h"
Expand Down Expand Up @@ -92,5 +93,13 @@ TEST(CreateBlimpMessageTest, StartConnectionMessage) {
message->protocol_control().start_connection().protocol_version());
}

TEST(CreateBlimpMessageTest, BlobChannelMessage) {
BlobChannelMessage* details;
std::unique_ptr<BlimpMessage> message = CreateBlimpMessage(&details);
ASSERT_TRUE(message);
EXPECT_EQ(details, &message->blob_channel());
EXPECT_EQ(BlimpMessage::kBlobChannel, message->feature_case());
}

} // namespace
} // namespace blimp
26 changes: 26 additions & 0 deletions blimp/common/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
#include "base/json/string_escape.h"
#include "base/lazy_instance.h"
#include "base/memory/ptr_util.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "blimp/common/create_blimp_message.h"
#include "blimp/common/proto/blimp_message.pb.h"

namespace blimp {
Expand Down Expand Up @@ -248,6 +250,28 @@ class TabControlLogExtractor : public LogExtractor {
}
};

// Logs fields from BLOB_CHANNEL messages.
class BlobChannelLogExtractor : public LogExtractor {
void ExtractFields(const BlimpMessage& message,
LogFields* output) const override {
switch (message.blob_channel().type_case()) {
case BlobChannelMessage::TypeCase::kTransferBlob:
AddField("subtype", "TRANSFER_BLOB", output);
AddField("id",
base::HexEncode(
message.blob_channel().transfer_blob().blob_id().data(),
message.blob_channel().transfer_blob().blob_id().size()),
output);
AddField("payload_size",
message.blob_channel().transfer_blob().payload().size(),
output);
break;
case BlobChannelMessage::TypeCase::TYPE_NOT_SET: // unknown
break;
}
}
};

// No fields are extracted from |message|.
class NullLogExtractor : public LogExtractor {
void ExtractFields(const BlimpMessage& message,
Expand All @@ -271,6 +295,8 @@ BlimpMessageLogger::BlimpMessageLogger() {
base::WrapUnique(new SettingsLogExtractor));
AddHandler("TAB_CONTROL", BlimpMessage::kTabControl,
base::WrapUnique(new TabControlLogExtractor));
AddHandler("BLOB_CHANNEL", BlimpMessage::kBlobChannel,
base::WrapUnique(new BlobChannelLogExtractor));
}

BlimpMessageLogger::~BlimpMessageLogger() {}
Expand Down
14 changes: 14 additions & 0 deletions blimp/common/logging_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@

#include "base/at_exit.h"
#include "base/strings/stringprintf.h"
#include "blimp/common/create_blimp_message.h"
#include "blimp/common/logging.h"
#include "blimp/common/proto/blimp_message.pb.h"
#include "blimp/common/proto/blob_channel.pb.h"
#include "blimp/net/test_common.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -220,6 +222,18 @@ TEST_F(LoggingTest, RenderWidget) {
deleted_msg);
}

TEST_F(LoggingTest, BlobChannel) {
BlobChannelMessage* blob_message = nullptr;
std::unique_ptr<BlimpMessage> blimp_message =
CreateBlimpMessage(&blob_message);
blob_message->mutable_transfer_blob()->set_blob_id("AAA");
blob_message->mutable_transfer_blob()->set_payload("123");

VerifyLogOutput(
"type=BLOB_CHANNEL subtype=TRANSFER_BLOB id=\"414141\" payload_size=3",
*blimp_message);
}

TEST_F(LoggingTest, Settings) {
BlimpMessage message;
message.mutable_settings()
Expand Down
1 change: 1 addition & 0 deletions blimp/common/proto/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ proto_library("proto_internal") {
sources = [
"blimp_message.proto",
"blob_cache.proto",
"blob_channel.proto",
"compositor.proto",
"ime.proto",
"input.proto",
Expand Down
2 changes: 2 additions & 0 deletions blimp/common/proto/blimp_message.proto
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ syntax = "proto2";

option optimize_for = LITE_RUNTIME;

import "blob_channel.proto";
import "compositor.proto";
import "ime.proto";
import "input.proto";
Expand Down Expand Up @@ -61,6 +62,7 @@ message BlimpMessage {
ProtocolControlMessage protocol_control = 45;
ImeMessage ime = 46;
SettingsMessage settings = 47;
BlobChannelMessage blob_channel = 48;
}
}

21 changes: 21 additions & 0 deletions blimp/common/proto/blob_channel.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2016 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.

syntax = "proto2";

option optimize_for = LITE_RUNTIME;

package blimp;

message TransferBlob {
optional string blob_id = 1;
optional bytes payload = 2;
}

message BlobChannelMessage {
oneof type {
// Engine => Client types.
TransferBlob transfer_blob = 1;
}
}
7 changes: 7 additions & 0 deletions blimp/net/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ component("net") {
"blob_channel/blob_channel_receiver.h",
"blob_channel/blob_channel_sender.cc",
"blob_channel/blob_channel_sender.h",
"blob_channel/helium_blob_receiver_delegate.cc",
"blob_channel/helium_blob_receiver_delegate.h",
"blob_channel/helium_blob_sender_delegate.cc",
"blob_channel/helium_blob_sender_delegate.h",
"browser_connection_handler.cc",
"browser_connection_handler.h",
"client_connection_manager.cc",
Expand Down Expand Up @@ -84,6 +88,8 @@ source_set("test_support") {
testonly = true

sources = [
"blob_channel/mock_blob_channel_receiver.cc",
"blob_channel/mock_blob_channel_receiver.h",
"test_common.cc",
"test_common.h",
]
Expand All @@ -110,6 +116,7 @@ source_set("unit_tests") {
"blob_channel/blob_channel_integration_test.cc",
"blob_channel/blob_channel_receiver_unittest.cc",
"blob_channel/blob_channel_sender_unittest.cc",
"blob_channel/helium_blob_channel_unittest.cc",
"browser_connection_handler_unittest.cc",
"client_connection_manager_unittest.cc",
"compressed_packet_unittest.cc",
Expand Down
44 changes: 22 additions & 22 deletions blimp/net/blob_channel/blob_channel_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "blimp/common/blob_cache/test_util.h"
#include "blimp/net/blob_channel/blob_channel_receiver.h"
#include "blimp/net/blob_channel/blob_channel_sender.h"
#include "blimp/net/blob_channel/mock_blob_channel_receiver.h"
#include "blimp/net/test_common.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand All @@ -29,30 +30,19 @@ const char kBlobPayload[] = "bar1";
// after |this| is deleted.
class SenderDelegateProxy : public BlobChannelSender::Delegate {
public:
SenderDelegateProxy() {}
explicit SenderDelegateProxy(BlobChannelReceiver* receiver)
: receiver_(receiver) {}
~SenderDelegateProxy() override {}

// Returns a receiver object that will receive proxied calls sent to |this|.
std::unique_ptr<BlobChannelReceiver::Delegate> GetReceiverDelegate() {
DCHECK(!receiver_);
receiver_ = new ReceiverDelegate;
return base::WrapUnique(receiver_);
}

private:
class ReceiverDelegate : public BlobChannelReceiver::Delegate {
public:
using BlobChannelReceiver::Delegate::OnBlobReceived;
};

// BlobChannelSender implementation.
void DeliverBlob(const BlobId& id, BlobDataPtr data) override {
base::MessageLoop::current()->PostTask(
FROM_HERE, base::Bind(&ReceiverDelegate::OnBlobReceived,
FROM_HERE, base::Bind(&BlobChannelReceiver::OnBlobReceived,
base::Unretained(receiver_), id, data));
}

ReceiverDelegate* receiver_ = nullptr;
BlobChannelReceiver* receiver_;

DISALLOW_COPY_AND_ASSIGN(SenderDelegateProxy);
};
Expand All @@ -62,18 +52,28 @@ class SenderDelegateProxy : public BlobChannelSender::Delegate {
class BlobChannelIntegrationTest : public testing::Test {
public:
BlobChannelIntegrationTest() {
std::unique_ptr<SenderDelegateProxy> sender_delegate(
new SenderDelegateProxy);
receiver_.reset(
new BlobChannelReceiver(base::WrapUnique(new InMemoryBlobCache),
sender_delegate->GetReceiverDelegate()));
sender_.reset(new BlobChannelSender(base::WrapUnique(new InMemoryBlobCache),
std::move(sender_delegate)));
BlobChannelReceiver* stored_receiver;
std::unique_ptr<MockBlobChannelReceiverDelegate> receiver_delegate(
new MockBlobChannelReceiverDelegate);
receiver_delegate_ = receiver_delegate.get();

EXPECT_CALL(*receiver_delegate, SetReceiver(_))
.WillOnce(SaveArg<0>(&stored_receiver));

receiver_ = BlobChannelReceiver::Create(
base::WrapUnique(new InMemoryBlobCache), std::move(receiver_delegate));

EXPECT_EQ(receiver_.get(), stored_receiver);

sender_.reset(new BlobChannelSender(
base::WrapUnique(new InMemoryBlobCache),
base::WrapUnique(new SenderDelegateProxy(receiver_.get()))));
}

~BlobChannelIntegrationTest() override {}

protected:
MockBlobChannelReceiverDelegate* receiver_delegate_;
std::unique_ptr<BlobChannelReceiver> receiver_;
std::unique_ptr<BlobChannelSender> sender_;
base::MessageLoop message_loop_;
Expand Down
Loading

0 comments on commit 9dced90

Please sign in to comment.