diff options
4 files changed, 87 insertions, 78 deletions
diff --git a/chromium/third_party/blink/renderer/platform/blob/blob_bytes_provider.cc b/chromium/third_party/blink/renderer/platform/blob/blob_bytes_provider.cc index 7aa139649f7..85915bb9602 100644 --- a/chromium/third_party/blink/renderer/platform/blob/blob_bytes_provider.cc +++ b/chromium/third_party/blink/renderer/platform/blob/blob_bytes_provider.cc @@ -31,13 +31,10 @@ class BlobBytesStreamer { public: BlobBytesStreamer(Vector<scoped_refptr<RawData>> data, - mojo::ScopedDataPipeProducerHandle pipe, - scoped_refptr<base::SequencedTaskRunner> task_runner) + mojo::ScopedDataPipeProducerHandle pipe) : data_(std::move(data)), pipe_(std::move(pipe)), - watcher_(FROM_HERE, - mojo::SimpleWatcher::ArmingPolicy::AUTOMATIC, - std::move(task_runner)) { + watcher_(FROM_HERE, mojo::SimpleWatcher::ArmingPolicy::AUTOMATIC) { watcher_.Watch(pipe_.get(), MOJO_HANDLE_SIGNAL_WRITABLE, MOJO_WATCH_CONDITION_SATISFIED, WTF::BindRepeating(&BlobBytesStreamer::OnWritable, @@ -45,6 +42,8 @@ class BlobBytesStreamer { } void OnWritable(MojoResult result, const mojo::HandleSignalsState& state) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + if (result == MOJO_RESULT_CANCELLED || result == MOJO_RESULT_FAILED_PRECONDITION) { delete this; @@ -84,15 +83,18 @@ class BlobBytesStreamer { private: // The index of the item currently being written. - size_t current_item_ = 0; + size_t current_item_ GUARDED_BY_CONTEXT(sequence_checker_) = 0; // The offset into the current item of the first byte not yet written to the // data pipe. - size_t current_item_offset_ = 0; + size_t current_item_offset_ GUARDED_BY_CONTEXT(sequence_checker_) = 0; // The data being written. - Vector<scoped_refptr<RawData>> data_; + Vector<scoped_refptr<RawData>> data_ GUARDED_BY_CONTEXT(sequence_checker_); + + mojo::ScopedDataPipeProducerHandle pipe_ + GUARDED_BY_CONTEXT(sequence_checker_); + mojo::SimpleWatcher watcher_ GUARDED_BY_CONTEXT(sequence_checker_); - mojo::ScopedDataPipeProducerHandle pipe_; - mojo::SimpleWatcher watcher_; + SEQUENCE_CHECKER(sequence_checker_); }; // This keeps the process alive while blobs are being transferred. @@ -118,31 +120,8 @@ void DecreaseChildProcessRefCount() { constexpr size_t BlobBytesProvider::kMaxConsolidatedItemSizeInBytes; -// static -BlobBytesProvider* BlobBytesProvider::CreateAndBind( - mojo::PendingReceiver<mojom::blink::BytesProvider> receiver) { - auto task_runner = base::ThreadPool::CreateSequencedTaskRunner( - {base::MayBlock(), base::TaskPriority::USER_VISIBLE}); - auto provider = base::WrapUnique(new BlobBytesProvider(task_runner)); - auto* result = provider.get(); - // TODO(mek): Consider binding BytesProvider on the IPC thread instead, only - // using the MayBlock taskrunner for actual file operations. - PostCrossThreadTask( - *task_runner, FROM_HERE, - CrossThreadBindOnce( - [](std::unique_ptr<BlobBytesProvider> provider, - mojo::PendingReceiver<mojom::blink::BytesProvider> receiver) { - mojo::MakeSelfOwnedReceiver(std::move(provider), - std::move(receiver)); - }, - std::move(provider), std::move(receiver))); - return result; -} - -// static -std::unique_ptr<BlobBytesProvider> BlobBytesProvider::CreateForTesting( - scoped_refptr<base::SequencedTaskRunner> task_runner) { - return base::WrapUnique(new BlobBytesProvider(std::move(task_runner))); +BlobBytesProvider::BlobBytesProvider() { + IncreaseChildProcessRefCount(); } BlobBytesProvider::~BlobBytesProvider() { @@ -150,6 +129,8 @@ BlobBytesProvider::~BlobBytesProvider() { } void BlobBytesProvider::AppendData(scoped_refptr<RawData> data) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + if (!data_.IsEmpty()) { uint64_t last_offset = offsets_.IsEmpty() ? 0 : offsets_.back(); offsets_.push_back(last_offset + data_.back()->length()); @@ -158,6 +139,8 @@ void BlobBytesProvider::AppendData(scoped_refptr<RawData> data) { } void BlobBytesProvider::AppendData(base::span<const char> data) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + if (data_.IsEmpty() || data_.back()->length() + data.size() > kMaxConsolidatedItemSizeInBytes) { AppendData(RawData::Create()); @@ -165,8 +148,32 @@ void BlobBytesProvider::AppendData(base::span<const char> data) { data_.back()->MutableData()->Append(data.data(), data.size()); } +// static +void BlobBytesProvider::Bind( + std::unique_ptr<BlobBytesProvider> provider, + mojo::PendingReceiver<mojom::blink::BytesProvider> receiver) { + DCHECK_CALLED_ON_VALID_SEQUENCE(provider->sequence_checker_); + DETACH_FROM_SEQUENCE(provider->sequence_checker_); + + auto task_runner = base::ThreadPool::CreateSequencedTaskRunner( + {base::MayBlock(), base::TaskPriority::USER_VISIBLE}); + // TODO(mek): Consider binding BytesProvider on the IPC thread instead, only + // using the MayBlock taskrunner for actual file operations. + PostCrossThreadTask( + *task_runner, FROM_HERE, + CrossThreadBindOnce( + [](std::unique_ptr<BlobBytesProvider> provider, + mojo::PendingReceiver<mojom::blink::BytesProvider> receiver) { + DCHECK_CALLED_ON_VALID_SEQUENCE(provider->sequence_checker_); + mojo::MakeSelfOwnedReceiver(std::move(provider), + std::move(receiver)); + }, + std::move(provider), std::move(receiver))); +} + void BlobBytesProvider::RequestAsReply(RequestAsReplyCallback callback) { - DCHECK(task_runner_->RunsTasksInCurrentSequence()); + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + // TODO(mek): Once better metrics are created we could experiment with ways // to reduce the number of copies of data that are made here. Vector<uint8_t> result; @@ -177,9 +184,10 @@ void BlobBytesProvider::RequestAsReply(RequestAsReplyCallback callback) { void BlobBytesProvider::RequestAsStream( mojo::ScopedDataPipeProducerHandle pipe) { - DCHECK(task_runner_->RunsTasksInCurrentSequence()); + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + // BlobBytesStreamer will self delete when done. - new BlobBytesStreamer(std::move(data_), std::move(pipe), task_runner_); + new BlobBytesStreamer(std::move(data_), std::move(pipe)); } void BlobBytesProvider::RequestAsFile(uint64_t source_offset, @@ -187,7 +195,7 @@ void BlobBytesProvider::RequestAsFile(uint64_t source_offset, base::File file, uint64_t file_offset, RequestAsFileCallback callback) { - DCHECK(task_runner_->RunsTasksInCurrentSequence()); + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (!file.IsValid()) { std::move(callback).Run(base::nullopt); @@ -255,10 +263,4 @@ void BlobBytesProvider::RequestAsFile(uint64_t source_offset, std::move(callback).Run(info.last_modified); } -BlobBytesProvider::BlobBytesProvider( - scoped_refptr<base::SequencedTaskRunner> task_runner) - : task_runner_(std::move(task_runner)) { - IncreaseChildProcessRefCount(); -} - } // namespace blink diff --git a/chromium/third_party/blink/renderer/platform/blob/blob_bytes_provider.h b/chromium/third_party/blink/renderer/platform/blob/blob_bytes_provider.h index 59b019a6ce9..62ce89d1c4c 100644 --- a/chromium/third_party/blink/renderer/platform/blob/blob_bytes_provider.h +++ b/chromium/third_party/blink/renderer/platform/blob/blob_bytes_provider.h @@ -6,6 +6,7 @@ #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_BLOB_BLOB_BYTES_PROVIDER_H_ #include "base/sequenced_task_runner.h" +#include "base/sequence_checker.h" #include "third_party/blink/public/mojom/blob/blob_registry.mojom-blink-forward.h" #include "third_party/blink/public/mojom/blob/data_element.mojom-blink.h" #include "third_party/blink/renderer/platform/blob/blob_data.h" @@ -25,19 +26,17 @@ class PLATFORM_EXPORT BlobBytesProvider : public mojom::blink::BytesProvider { // data appended to the same item. static constexpr size_t kMaxConsolidatedItemSizeInBytes = 15 * 1024; - // Creates a new instance, and binds it on a new SequencedTaskRunner. The - // returned instance should only be considered valid as long as the request - // passed in to this method is still known to be valid. - static BlobBytesProvider* CreateAndBind( - mojo::PendingReceiver<mojom::blink::BytesProvider>); - static std::unique_ptr<BlobBytesProvider> CreateForTesting( - scoped_refptr<base::SequencedTaskRunner>); - + BlobBytesProvider(); ~BlobBytesProvider() override; void AppendData(scoped_refptr<RawData>); void AppendData(base::span<const char>); + // Binds `provider` to `receiver` on a threadpool task runner, transferring + // ownership. + static void Bind(std::unique_ptr<BlobBytesProvider> provider, + mojo::PendingReceiver<mojom::blink::BytesProvider> receiver); + // BytesProvider implementation: void RequestAsReply(RequestAsReplyCallback) override; void RequestAsStream(mojo::ScopedDataPipeProducerHandle) override; @@ -50,17 +49,13 @@ class PLATFORM_EXPORT BlobBytesProvider : public mojom::blink::BytesProvider { private: FRIEND_TEST_ALL_PREFIXES(BlobBytesProviderTest, Consolidation); - BlobBytesProvider(scoped_refptr<base::SequencedTaskRunner>); - - // The task runner this class is bound on, as well as what is used by the - // RequestAsStream method to monitor the data pipe. - scoped_refptr<base::SequencedTaskRunner> task_runner_; - - Vector<scoped_refptr<RawData>> data_; + Vector<scoped_refptr<RawData>> data_ GUARDED_BY_CONTEXT(sequence_checker_); // |offsets_| always contains exactly one fewer item than |data_| (except when // |data_| itself is empty). // offsets_[x] is equal to the sum of data_[i].length for all i <= x. - Vector<uint64_t> offsets_; + Vector<uint64_t> offsets_ GUARDED_BY_CONTEXT(sequence_checker_); + + SEQUENCE_CHECKER(sequence_checker_); }; } // namespace blink diff --git a/chromium/third_party/blink/renderer/platform/blob/blob_data.cc b/chromium/third_party/blink/renderer/platform/blob/blob_data.cc index a381dbc3140..feeae4779a5 100644 --- a/chromium/third_party/blink/renderer/platform/blob/blob_data.cc +++ b/chromium/third_party/blink/renderer/platform/blob/blob_data.cc @@ -106,6 +106,12 @@ BlobData::BlobData(FileCompositionStatus composition) BlobData::~BlobData() = default; Vector<mojom::blink::DataElementPtr> BlobData::ReleaseElements() { + if (last_bytes_provider_) { + DCHECK(last_bytes_provider_receiver_); + BlobBytesProvider::Bind(std::move(last_bytes_provider_), + std::move(last_bytes_provider_receiver_)); + } + return std::move(elements_); } @@ -140,16 +146,6 @@ std::unique_ptr<BlobData> BlobData::CreateForFileSystemURLWithUnknownSize( return data; } -void BlobData::DetachFromCurrentThread() { - content_type_ = content_type_.IsolatedCopy(); - for (auto& element : elements_) { - if (element->is_file_filesystem()) { - auto& file_element = element->get_file_filesystem(); - file_element->url = file_element->url.Copy(); - } - } -} - void BlobData::SetContentType(const String& content_type) { if (IsValidBlobType(content_type)) content_type_ = content_type; @@ -273,6 +269,7 @@ void BlobData::AppendDataInternal(base::span<const char> data, if (!elements_.IsEmpty() && elements_.back()->is_bytes()) { // Append bytes to previous element. DCHECK(last_bytes_provider_); + DCHECK(last_bytes_provider_receiver_); const auto& bytes_element = elements_.back()->get_bytes(); bytes_element->length += data.size(); if (should_embed_bytes && bytes_element->embedded_data) { @@ -283,9 +280,18 @@ void BlobData::AppendDataInternal(base::span<const char> data, bytes_element->embedded_data = base::nullopt; } } else { + if (last_bytes_provider_) { + // If `last_bytes_provider_` is set, but the previous element is not a + // bytes element, a new BytesProvider will be created and we need to + // make sure to bind the previous one first. + DCHECK(last_bytes_provider_receiver_); + BlobBytesProvider::Bind(std::move(last_bytes_provider_), + std::move(last_bytes_provider_receiver_)); + } mojo::PendingRemote<BytesProvider> bytes_provider_remote; - last_bytes_provider_ = BlobBytesProvider::CreateAndBind( - bytes_provider_remote.InitWithNewPipeAndPassReceiver()); + last_bytes_provider_ = std::make_unique<BlobBytesProvider>(); + last_bytes_provider_receiver_ = + bytes_provider_remote.InitWithNewPipeAndPassReceiver(); auto bytes_element = DataElementBytes::New( data.size(), base::nullopt, std::move(bytes_provider_remote)); diff --git a/chromium/third_party/blink/renderer/platform/blob/blob_data.h b/chromium/third_party/blink/renderer/platform/blob/blob_data.h index 697c6ce501e..864198847d4 100644 --- a/chromium/third_party/blink/renderer/platform/blob/blob_data.h +++ b/chromium/third_party/blink/renderer/platform/blob/blob_data.h @@ -43,6 +43,7 @@ #include "base/thread_annotations.h" #include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/struct_ptr.h" +#include "third_party/blink/public/mojom/blob/data_element.mojom-blink-forward.h" #include "third_party/blink/renderer/platform/weborigin/kurl.h" #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h" #include "third_party/blink/renderer/platform/wtf/forward.h" @@ -117,13 +118,10 @@ class PLATFORM_EXPORT BlobData { const KURL& file_system_url, const base::Optional<base::Time>& expected_modification_time); - // Detaches from current thread so that it can be passed to another thread. - void DetachFromCurrentThread(); - const String& ContentType() const { return content_type_; } void SetContentType(const String&); - const Vector<mojom::blink::DataElementPtr>& Elements() const { + const Vector<mojom::blink::DataElementPtr>& ElementsForTesting() const { return elements_; } Vector<mojom::blink::DataElementPtr> ReleaseElements(); @@ -165,7 +163,15 @@ class PLATFORM_EXPORT BlobData { Vector<mojom::blink::DataElementPtr> elements_; size_t current_memory_population_ = 0; - BlobBytesProvider* last_bytes_provider_ = nullptr; + + // These two members are used to combine multiple consecutive 'bytes' elements + // (as created by `AppendBytes`, `AppendData` or `AppendText`) into a single + // element. When one has a value the other also has a value. Before using + // `elements_` to actually create a blob, `last_bytes_provider_` should be + // bound to `last_bytes_provider_receiver_`. + std::unique_ptr<BlobBytesProvider> last_bytes_provider_; + mojo::PendingReceiver<mojom::blink::BytesProvider> + last_bytes_provider_receiver_; DISALLOW_COPY_AND_ASSIGN(BlobData); }; |