Skip to content

Commit

Permalink
Convert BytesProvider to new Mojo types
Browse files Browse the repository at this point in the history 
This CL converts BytesProviderRequest and BytesProviderPtr
to new Mojo types.
It updates struct DataElementBytes from data_element.mojom
and methods and members to new Mojo types.

Bug: 955171, 978694
Change-Id: I4a1f7a2cb73581ec23596178154fc9282c25e6f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1774424
Commit-Queue: Julie Kim <jkim@igalia.com>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691982}
  • Loading branch information
@jkim-julie
jkim-julie authored and Commit Bot committed Aug 30, 2019
1 parent 55730c2 commit 7ba89a9
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 89 deletions.
8 changes: 4 additions & 4 deletions storage/browser/blob/blob_registry_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ class BlobRegistryImpl::BlobUnderConstruction {

private:
// Holds onto a blink::mojom::DataElement struct and optionally a bound
// blink::mojom::BytesProviderPtr or blink::mojom::BlobPtr, if the element
// encapsulates a large byte array or a blob.
// mojo::Remote<blink::mojom::BytesProvider> or blink::mojom::BlobPtr, if the
// element encapsulates a large byte array or a blob.
struct ElementEntry {
explicit ElementEntry(blink::mojom::DataElementPtr e)
: element(std::move(e)) {
Expand All @@ -75,7 +75,7 @@ class BlobRegistryImpl::BlobUnderConstruction {
ElementEntry& operator=(ElementEntry&& other) = default;

blink::mojom::DataElementPtr element;
blink::mojom::BytesProviderPtr bytes_provider;
mojo::Remote<blink::mojom::BytesProvider> bytes_provider;
blink::mojom::BlobPtr blob;
};

Expand Down Expand Up @@ -228,7 +228,7 @@ void BlobRegistryImpl::BlobUnderConstruction::StartTransportation() {
base::BindOnce(&BlobUnderConstruction::ReceivedBlobUUID,
weak_ptr_factory_.GetWeakPtr(), blob_count++));
} else if (element->is_bytes()) {
entry.bytes_provider.set_connection_error_handler(base::BindOnce(
entry.bytes_provider.set_disconnect_handler(base::BindOnce(
&BlobUnderConstruction::MarkAsBroken, weak_ptr_factory_.GetWeakPtr(),
BlobStatus::ERR_SOURCE_DIED_IN_TRANSIT, ""));
}
Expand Down
51 changes: 27 additions & 24 deletions storage/browser/blob/blob_registry_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "mojo/core/embedder/embedder.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "mojo/public/cpp/system/data_pipe_utils.h"
#include "storage/browser/blob/blob_data_builder.h"
Expand Down Expand Up @@ -47,9 +48,10 @@ const uint64_t kTestBlobStorageMaxDiskSpace = 4000;
const uint64_t kTestBlobStorageMinFileSizeBytes = 10;
const uint64_t kTestBlobStorageMaxFileSizeBytes = 100;

void BindBytesProvider(std::unique_ptr<MockBytesProvider> impl,
blink::mojom::BytesProviderRequest request) {
mojo::MakeStrongBinding(std::move(impl), std::move(request));
void BindBytesProvider(
std::unique_ptr<MockBytesProvider> impl,
mojo::PendingReceiver<blink::mojom::BytesProvider> receiver) {
mojo::MakeSelfOwnedReceiver(std::move(impl), std::move(receiver));
}

} // namespace
Expand Down Expand Up @@ -140,24 +142,25 @@ class BlobRegistryImplTest : public testing::Test {
loop.Run();
}

blink::mojom::BytesProviderPtrInfo CreateBytesProvider(
mojo::PendingRemote<blink::mojom::BytesProvider> CreateBytesProvider(
const std::string& bytes) {
if (!bytes_provider_runner_) {
bytes_provider_runner_ = base::CreateSequencedTaskRunner(
{base::ThreadPool(), base::MayBlock()});
}
blink::mojom::BytesProviderPtrInfo result;
mojo::PendingRemote<blink::mojom::BytesProvider> result;
auto provider = std::make_unique<MockBytesProvider>(
std::vector<uint8_t>(bytes.begin(), bytes.end()), &reply_request_count_,
&stream_request_count_, &file_request_count_);
bytes_provider_runner_->PostTask(
FROM_HERE, base::BindOnce(&BindBytesProvider, std::move(provider),
MakeRequest(&result)));
result.InitWithNewPipeAndPassReceiver()));
return result;
}

void CreateBytesProvider(const std::string& bytes,
blink::mojom::BytesProviderRequest request) {
void CreateBytesProvider(
const std::string& bytes,
mojo::PendingReceiver<blink::mojom::BytesProvider> receiver) {
if (!bytes_provider_runner_) {
bytes_provider_runner_ = base::CreateSequencedTaskRunner(
{base::ThreadPool(), base::MayBlock()});
Expand All @@ -167,7 +170,7 @@ class BlobRegistryImplTest : public testing::Test {
&stream_request_count_, &file_request_count_);
bytes_provider_runner_->PostTask(
FROM_HERE, base::BindOnce(&BindBytesProvider, std::move(provider),
std::move(request)));
std::move(receiver)));
}

size_t BlobsUnderConstruction() {
Expand Down Expand Up @@ -842,13 +845,13 @@ TEST_F(BlobRegistryImplTest, Register_ValidBytesAsFile) {
TEST_F(BlobRegistryImplTest, Register_BytesProviderClosedPipe) {
const std::string kId = "id";

blink::mojom::BytesProviderPtrInfo bytes_provider_info;
MakeRequest(&bytes_provider_info);
mojo::PendingRemote<blink::mojom::BytesProvider> bytes_provider_remote;
ignore_result(bytes_provider_remote.InitWithNewPipeAndPassReceiver());

std::vector<blink::mojom::DataElementPtr> elements;
elements.push_back(
blink::mojom::DataElement::NewBytes(blink::mojom::DataElementBytes::New(
32, base::nullopt, std::move(bytes_provider_info))));
32, base::nullopt, std::move(bytes_provider_remote))));

blink::mojom::BlobPtr blob;
EXPECT_TRUE(registry_->Register(MakeRequest(&blob), kId, "", "",
Expand All @@ -867,13 +870,13 @@ TEST_F(BlobRegistryImplTest,
Register_DefereferencedWhileBuildingBeforeBreaking) {
const std::string kId = "id";

blink::mojom::BytesProviderPtrInfo bytes_provider_info;
auto request = MakeRequest(&bytes_provider_info);
mojo::PendingRemote<blink::mojom::BytesProvider> bytes_provider_remote;
auto receiver = bytes_provider_remote.InitWithNewPipeAndPassReceiver();

std::vector<blink::mojom::DataElementPtr> elements;
elements.push_back(
blink::mojom::DataElement::NewBytes(blink::mojom::DataElementBytes::New(
32, base::nullopt, std::move(bytes_provider_info))));
32, base::nullopt, std::move(bytes_provider_remote))));

blink::mojom::BlobPtr blob;
EXPECT_TRUE(registry_->Register(MakeRequest(&blob), kId, "", "",
Expand All @@ -891,7 +894,7 @@ TEST_F(BlobRegistryImplTest,
EXPECT_FALSE(context_->registry().HasEntry(kId));

// Now cause construction to fail, if it would still be going on.
request = nullptr;
receiver.reset();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(0u, BlobsUnderConstruction());
}
Expand Down Expand Up @@ -945,13 +948,13 @@ TEST_F(BlobRegistryImplTest,
const std::string kId = "id";
const std::string kData = "hello world";

blink::mojom::BytesProviderPtrInfo bytes_provider_info;
auto request = MakeRequest(&bytes_provider_info);
mojo::PendingRemote<blink::mojom::BytesProvider> bytes_provider_remote;
auto receiver = bytes_provider_remote.InitWithNewPipeAndPassReceiver();

std::vector<blink::mojom::DataElementPtr> elements;
elements.push_back(
blink::mojom::DataElement::NewBytes(blink::mojom::DataElementBytes::New(
kData.size(), base::nullopt, std::move(bytes_provider_info))));
kData.size(), base::nullopt, std::move(bytes_provider_remote))));

blink::mojom::BlobPtr blob;
EXPECT_TRUE(registry_->Register(MakeRequest(&blob), kId, "", "",
Expand All @@ -969,7 +972,7 @@ TEST_F(BlobRegistryImplTest,
EXPECT_FALSE(context_->registry().HasEntry(kId));

// Now cause construction to complete, if it would still be going on.
CreateBytesProvider(kData, std::move(request));
CreateBytesProvider(kData, std::move(receiver));
task_environment_.RunUntilIdle();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(0u, BlobsUnderConstruction());
Expand All @@ -981,13 +984,13 @@ TEST_F(BlobRegistryImplTest,
const std::string kData =
base::RandBytesAsString(kTestBlobStorageMaxBlobMemorySize + 42);

blink::mojom::BytesProviderPtrInfo bytes_provider_info;
auto request = MakeRequest(&bytes_provider_info);
mojo::PendingRemote<blink::mojom::BytesProvider> bytes_provider_remote;
auto receiver = bytes_provider_remote.InitWithNewPipeAndPassReceiver();

std::vector<blink::mojom::DataElementPtr> elements;
elements.push_back(
blink::mojom::DataElement::NewBytes(blink::mojom::DataElementBytes::New(
kData.size(), base::nullopt, std::move(bytes_provider_info))));
kData.size(), base::nullopt, std::move(bytes_provider_remote))));

blink::mojom::BlobPtr blob;
EXPECT_TRUE(registry_->Register(MakeRequest(&blob), kId, "", "",
Expand All @@ -1005,7 +1008,7 @@ TEST_F(BlobRegistryImplTest,
EXPECT_FALSE(context_->registry().HasEntry(kId));

// Now cause construction to complete, if it would still be going on.
CreateBytesProvider(kData, std::move(request));
CreateBytesProvider(kData, std::move(receiver));
task_environment_.RunUntilIdle();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(0u, BlobsUnderConstruction());
Expand Down
20 changes: 12 additions & 8 deletions storage/browser/blob/blob_transport_strategy.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ class NoneNeededTransportStrategy : public BlobTransportStrategy {
ResultCallback result_callback)
: BlobTransportStrategy(builder, std::move(result_callback)) {}

void AddBytesElement(blink::mojom::DataElementBytes* bytes,
const blink::mojom::BytesProviderPtr& data) override {
void AddBytesElement(
blink::mojom::DataElementBytes* bytes,
const mojo::Remote<blink::mojom::BytesProvider>& data) override {
DCHECK(bytes->embedded_data);
DCHECK_EQ(bytes->length, bytes->embedded_data->size());
builder_->AppendData(
Expand All @@ -48,8 +49,9 @@ class ReplyTransportStrategy : public BlobTransportStrategy {
ResultCallback result_callback)
: BlobTransportStrategy(builder, std::move(result_callback)) {}

void AddBytesElement(blink::mojom::DataElementBytes* bytes,
const blink::mojom::BytesProviderPtr& data) override {
void AddBytesElement(
blink::mojom::DataElementBytes* bytes,
const mojo::Remote<blink::mojom::BytesProvider>& data) override {
BlobDataBuilder::FutureData future_data =
builder_->AppendFutureData(bytes->length);
// base::Unretained is safe because |this| is guaranteed (by the contract
Expand Down Expand Up @@ -109,8 +111,9 @@ class DataPipeTransportStrategy : public BlobTransportStrategy {
mojo::SimpleWatcher::ArmingPolicy::AUTOMATIC,
base::SequencedTaskRunnerHandle::Get()) {}

void AddBytesElement(blink::mojom::DataElementBytes* bytes,
const blink::mojom::BytesProviderPtr& data) override {
void AddBytesElement(
blink::mojom::DataElementBytes* bytes,
const mojo::Remote<blink::mojom::BytesProvider>& data) override {
// Split up the data in |max_bytes_data_item_size| sized chunks.
std::vector<BlobDataBuilder::FutureData> future_data;
for (uint64_t source_offset = 0; source_offset < bytes->length;
Expand Down Expand Up @@ -260,8 +263,9 @@ class FileTransportStrategy : public BlobTransportStrategy {
: BlobTransportStrategy(builder, std::move(result_callback)),
limits_(limits) {}

void AddBytesElement(blink::mojom::DataElementBytes* bytes,
const blink::mojom::BytesProviderPtr& data) override {
void AddBytesElement(
blink::mojom::DataElementBytes* bytes,
const mojo::Remote<blink::mojom::BytesProvider>& data) override {
uint64_t source_offset = 0;
while (source_offset < bytes->length) {
if (current_file_size_ >= limits_.max_file_size ||
Expand Down
6 changes: 4 additions & 2 deletions storage/browser/blob/blob_transport_strategy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "base/callback.h"
#include "base/component_export.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "storage/browser/blob/blob_memory_controller.h"
#include "third_party/blink/public/mojom/blob/blob_registry.mojom.h"

Expand Down Expand Up @@ -35,8 +36,9 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobTransportStrategy {
// outlive the BlobTransportStrategy instance. If |data| is bound, this call
// may use it to acquire the bytes asynchronously rather than reading from
// |bytes|.
virtual void AddBytesElement(blink::mojom::DataElementBytes* bytes,
const blink::mojom::BytesProviderPtr& data) = 0;
virtual void AddBytesElement(
blink::mojom::DataElementBytes* bytes,
const mojo::Remote<blink::mojom::BytesProvider>& data) = 0;

// Called when quota has been allocated and transportation should begin.
// Implementations will call the |result_callback_| when transportation has
Expand Down
Loading

0 comments on commit 7ba89a9

Please sign in to comment.