Skip to content

Commit

Permalink
fix: implement ses.getBlobData() for NetworkService (#20041) (#20105)
Browse files Browse the repository at this point in the history
* pass data pipe to JS

* implement reading buffer

* re-enable ses.getBlobData test

* remove AtomBlobReader
  • Loading branch information
zcbenz authored and deepak1556 committed Sep 4, 2019
1 parent 4ce4318 commit 2d16034
Show file tree
Hide file tree
Showing 9 changed files with 259 additions and 27 deletions.
4 changes: 2 additions & 2 deletions filenames.gni
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ filenames = {
"shell/browser/api/atom_api_content_tracing.cc",
"shell/browser/api/atom_api_cookies.cc",
"shell/browser/api/atom_api_cookies.h",
"shell/browser/api/atom_api_data_pipe_holder.cc",
"shell/browser/api/atom_api_data_pipe_holder.h",
"shell/browser/api/atom_api_debugger.cc",
"shell/browser/api/atom_api_debugger.h",
"shell/browser/api/atom_api_dialog.cc",
Expand Down Expand Up @@ -125,8 +127,6 @@ filenames = {
"shell/browser/auto_updater.cc",
"shell/browser/auto_updater.h",
"shell/browser/auto_updater_mac.mm",
"shell/browser/atom_blob_reader.cc",
"shell/browser/atom_blob_reader.h",
"shell/browser/atom_browser_client.cc",
"shell/browser/atom_browser_client.h",
"shell/browser/atom_browser_context.cc",
Expand Down
185 changes: 185 additions & 0 deletions shell/browser/api/atom_api_data_pipe_holder.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
// Copyright (c) 2019 GitHub, Inc.
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.

#include "shell/browser/api/atom_api_data_pipe_holder.h"

#include <utility>
#include <vector>

#include "base/memory/weak_ptr.h"
#include "base/strings/string_number_conversions.h"
#include "mojo/public/cpp/system/data_pipe.h"
#include "mojo/public/cpp/system/simple_watcher.h"
#include "net/base/net_errors.h"
#include "shell/common/key_weak_map.h"
#include "shell/common/promise_util.h"

#include "shell/common/node_includes.h"

namespace electron {

namespace api {

namespace {

// Incremental ID.
int g_next_id = 0;

// Map that manages all the DataPipeHolder objects.
KeyWeakMap<std::string> g_weak_map;

// Utility class to read from data pipe.
class DataPipeReader {
public:
DataPipeReader(util::Promise promise,
network::mojom::DataPipeGetterPtr data_pipe_getter)
: promise_(std::move(promise)),
data_pipe_getter_(std::move(data_pipe_getter)),
handle_watcher_(FROM_HERE,
mojo::SimpleWatcher::ArmingPolicy::MANUAL,
base::SequencedTaskRunnerHandle::Get()) {
// Get a new data pipe and start.
mojo::DataPipe data_pipe;
data_pipe_getter_->Read(std::move(data_pipe.producer_handle),
base::BindOnce(&DataPipeReader::ReadCallback,
weak_factory_.GetWeakPtr()));
data_pipe_ = std::move(data_pipe.consumer_handle);
handle_watcher_.Watch(data_pipe_.get(), MOJO_HANDLE_SIGNAL_READABLE,
base::BindRepeating(&DataPipeReader::OnHandleReadable,
weak_factory_.GetWeakPtr()));
}

~DataPipeReader() = default;

private:
// Callback invoked by DataPipeGetter::Read.
void ReadCallback(int32_t status, uint64_t size) {
if (status != net::OK) {
OnFailure();
return;
}
buffer_.resize(size);
head_ = &buffer_.front();
remaining_size_ = size;
handle_watcher_.ArmOrNotify();
}

// Called by |handle_watcher_| when data is available or the pipe was closed,
// and there's a pending Read() call.
void OnHandleReadable(MojoResult result) {
if (result != MOJO_RESULT_OK) {
OnFailure();
return;
}

// Read.
uint32_t length = remaining_size_;
result = data_pipe_->ReadData(head_, &length, MOJO_READ_DATA_FLAG_NONE);
if (result == MOJO_RESULT_OK) { // success
remaining_size_ -= length;
head_ += length;
if (remaining_size_ == 0)
OnSuccess();
} else if (result == MOJO_RESULT_SHOULD_WAIT) { // IO pending
handle_watcher_.ArmOrNotify();
} else { // error
OnFailure();
}
}

void OnFailure() {
promise_.RejectWithErrorMessage("Could not get blob data");
delete this;
}

void OnSuccess() {
// Pass the buffer to JS.
//
// Note that the lifetime of the native buffer belongs to us, and we will
// free memory when JS buffer gets garbage collected.
v8::Locker locker(promise_.isolate());
v8::HandleScope handle_scope(promise_.isolate());
v8::Local<v8::Value> buffer =
node::Buffer::New(promise_.isolate(), &buffer_.front(), buffer_.size(),
&DataPipeReader::FreeBuffer, this)
.ToLocalChecked();
promise_.Resolve(buffer);

// Destroy data pipe.
handle_watcher_.Cancel();
data_pipe_.reset();
data_pipe_getter_ = nullptr;
}

static void FreeBuffer(char* data, void* self) {
delete static_cast<DataPipeReader*>(self);
}

util::Promise promise_;

network::mojom::DataPipeGetterPtr data_pipe_getter_;
mojo::ScopedDataPipeConsumerHandle data_pipe_;
mojo::SimpleWatcher handle_watcher_;

// Stores read data.
std::vector<char> buffer_;

// The head of buffer.
char* head_ = nullptr;

// Remaining data to read.
uint64_t remaining_size_ = 0;

base::WeakPtrFactory<DataPipeReader> weak_factory_{this};

DISALLOW_COPY_AND_ASSIGN(DataPipeReader);
};

} // namespace

gin::WrapperInfo DataPipeHolder::kWrapperInfo = {gin::kEmbedderNativeGin};

DataPipeHolder::DataPipeHolder(const network::DataElement& element)
: id_(base::NumberToString(++g_next_id)),
data_pipe_(element.CloneDataPipeGetter()) {}

DataPipeHolder::~DataPipeHolder() = default;

v8::Local<v8::Promise> DataPipeHolder::ReadAll(v8::Isolate* isolate) {
util::Promise promise(isolate);
v8::Local<v8::Promise> handle = promise.GetHandle();
if (!data_pipe_) {
promise.RejectWithErrorMessage("Could not get blob data");
return handle;
}

new DataPipeReader(std::move(promise), std::move(data_pipe_));
return handle;
}

// static
gin::Handle<DataPipeHolder> DataPipeHolder::Create(
v8::Isolate* isolate,
const network::DataElement& element) {
auto handle = gin::CreateHandle(isolate, new DataPipeHolder(element));
g_weak_map.Set(isolate, handle->id(),
handle->GetWrapper(isolate).ToLocalChecked());
return handle;
}

// static
gin::Handle<DataPipeHolder> DataPipeHolder::From(v8::Isolate* isolate,
const std::string& id) {
v8::MaybeLocal<v8::Object> object = g_weak_map.Get(isolate, id);
if (!object.IsEmpty()) {
gin::Handle<DataPipeHolder> handle;
if (gin::ConvertFromV8(isolate, object.ToLocalChecked(), &handle))
return handle;
}
return gin::Handle<DataPipeHolder>();
}

} // namespace api

} // namespace electron
53 changes: 53 additions & 0 deletions shell/browser/api/atom_api_data_pipe_holder.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright (c) 2019 GitHub, Inc.
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.

#ifndef SHELL_BROWSER_API_ATOM_API_DATA_PIPE_HOLDER_H_
#define SHELL_BROWSER_API_ATOM_API_DATA_PIPE_HOLDER_H_

#include <string>

#include "gin/handle.h"
#include "gin/wrappable.h"
#include "services/network/public/cpp/data_element.h"
#include "services/network/public/mojom/data_pipe_getter.mojom.h"

namespace electron {

namespace api {

// Retains reference to the data pipe.
class DataPipeHolder : public gin::Wrappable<DataPipeHolder> {
public:
static gin::WrapperInfo kWrapperInfo;

static gin::Handle<DataPipeHolder> Create(
v8::Isolate* isolate,
const network::DataElement& element);
static gin::Handle<DataPipeHolder> From(v8::Isolate* isolate,
const std::string& id);

// Read all data at once.
//
// TODO(zcbenz): This is apparently not suitable for really large data, but
// no one has complained about it yet.
v8::Local<v8::Promise> ReadAll(v8::Isolate* isolate);

// The unique ID that can be used to receive the object.
const std::string& id() const { return id_; }

private:
explicit DataPipeHolder(const network::DataElement& element);
~DataPipeHolder() override;

std::string id_;
network::mojom::DataPipeGetterPtr data_pipe_;

DISALLOW_COPY_AND_ASSIGN(DataPipeHolder);
};

} // namespace api

} // namespace electron

#endif // SHELL_BROWSER_API_ATOM_API_DATA_PIPE_HOLDER_H_
16 changes: 8 additions & 8 deletions shell/browser/api/atom_api_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "services/network/network_service.h"
#include "services/network/public/cpp/features.h"
#include "shell/browser/api/atom_api_cookies.h"
#include "shell/browser/api/atom_api_data_pipe_holder.h"
#include "shell/browser/api/atom_api_download_item.h"
#include "shell/browser/api/atom_api_net_log.h"
#include "shell/browser/api/atom_api_protocol_ns.h"
Expand Down Expand Up @@ -498,15 +499,14 @@ std::string Session::GetUserAgent() {

v8::Local<v8::Promise> Session::GetBlobData(v8::Isolate* isolate,
const std::string& uuid) {
util::Promise promise(isolate);
v8::Local<v8::Promise> handle = promise.GetHandle();
gin::Handle<DataPipeHolder> holder = DataPipeHolder::From(isolate, uuid);
if (holder.IsEmpty()) {
util::Promise promise(isolate);
promise.RejectWithErrorMessage("Could not get blob data handle");
return promise.GetHandle();
}

AtomBlobReader* blob_reader = browser_context()->GetBlobReader();
base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::IO},
base::BindOnce(&AtomBlobReader::StartReading,
base::Unretained(blob_reader), uuid, std::move(promise)));
return handle;
return holder->ReadAll(isolate);
}

void Session::CreateInterruptedDownload(const mate::Dictionary& options) {
Expand Down
1 change: 0 additions & 1 deletion shell/browser/api/atom_api_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "electron/buildflags/buildflags.h"
#include "native_mate/handle.h"
#include "shell/browser/api/trackable_object.h"
#include "shell/browser/atom_blob_reader.h"
#include "shell/browser/net/resolve_proxy_helper.h"
#include "shell/common/promise_util.h"

Expand Down
10 changes: 0 additions & 10 deletions shell/browser/atom_browser_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "net/base/escape.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/wrapper_shared_url_loader_factory.h"
#include "shell/browser/atom_blob_reader.h"
#include "shell/browser/atom_browser_client.h"
#include "shell/browser/atom_browser_main_parts.h"
#include "shell/browser/atom_download_manager_delegate.h"
Expand Down Expand Up @@ -256,15 +255,6 @@ std::string AtomBrowserContext::GetUserAgent() const {
return user_agent_;
}

AtomBlobReader* AtomBrowserContext::GetBlobReader() {
if (!blob_reader_.get()) {
content::ChromeBlobStorageContext* blob_context =
content::ChromeBlobStorageContext::GetFor(this);
blob_reader_.reset(new AtomBlobReader(blob_context));
}
return blob_reader_.get();
}

predictors::PreconnectManager* AtomBrowserContext::GetPreconnectManager() {
if (!preconnect_manager_.get()) {
preconnect_manager_.reset(new predictors::PreconnectManager(nullptr, this));
Expand Down
3 changes: 0 additions & 3 deletions shell/browser/atom_browser_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class AtomExtensionSystem;

namespace electron {

class AtomBlobReader;
class AtomBrowserContext;
class AtomDownloadManagerDelegate;
class AtomPermissionManager;
Expand Down Expand Up @@ -90,7 +89,6 @@ class AtomBrowserContext
std::string GetUserAgent() const;
bool CanUseHttpCache() const;
int GetMaxCacheSize() const;
AtomBlobReader* GetBlobReader();
ResolveProxyHelper* GetResolveProxyHelper();
predictors::PreconnectManager* GetPreconnectManager();
scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory();
Expand Down Expand Up @@ -163,7 +161,6 @@ class AtomBrowserContext
std::unique_ptr<AtomDownloadManagerDelegate> download_manager_delegate_;
std::unique_ptr<WebViewManager> guest_manager_;
std::unique_ptr<AtomPermissionManager> permission_manager_;
std::unique_ptr<AtomBlobReader> blob_reader_;
std::unique_ptr<MediaDeviceIDSalt> media_device_id_salt_;
scoped_refptr<ResolveProxyHelper> resolve_proxy_helper_;
scoped_refptr<storage::SpecialStoragePolicy> storage_policy_;
Expand Down
12 changes: 10 additions & 2 deletions shell/common/native_mate_converters/net_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "net/cert/x509_util.h"
#include "net/http/http_response_headers.h"
#include "services/network/public/cpp/resource_request.h"
#include "shell/browser/api/atom_api_data_pipe_holder.h"
#include "shell/browser/net/cert_verifier_client.h"
#include "shell/common/native_mate_converters/gurl_converter.h"
#include "shell/common/native_mate_converters/string16_converter.h"
Expand Down Expand Up @@ -248,9 +249,16 @@ v8::Local<v8::Value> Converter<network::ResourceRequestBody>::ToV8(
element.length())
.ToLocalChecked());
break;
case network::mojom::DataElementType::kBlob:
upload_data.Set("blobUUID", element.blob_uuid());
case network::mojom::DataElementType::kDataPipe: {
// TODO(zcbenz): After the NetworkService refactor, the old blobUUID API
// becomes unecessarily complex, we should deprecate the getBlobData API
// and return the DataPipeHolder wrapper directly.
auto holder = electron::api::DataPipeHolder::Create(isolate, element);
upload_data.Set("blobUUID", holder->id());
// The lifetime of data pipe is bound to the uploadData object.
upload_data.Set("dataPipe", gin::ConvertToV8(isolate, holder));
break;
}
default:
NOTREACHED() << "Found unsupported data element";
}
Expand Down
2 changes: 1 addition & 1 deletion spec-main/api-session-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ describe('session module', () => {
})
})

describe.skip('ses.getBlobData()', () => {
describe('ses.getBlobData()', () => {
const scheme = 'cors-blob'
const protocol = session.defaultSession.protocol
const url = `${scheme}://host`
Expand Down

0 comments on commit 2d16034

Please sign in to comment.