From aa8b66aae175fa24ec097a9aae9657f9b609873e Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 14 Mar 2019 08:11:01 -0700 Subject: [PATCH] feat: promisify session.getBlobData() (#17303) --- atom/browser/api/atom_api_session.cc | 21 +++++++++--------- atom/browser/api/atom_api_session.h | 4 ++-- atom/browser/atom_blob_reader.cc | 33 ++++++++++++++-------------- atom/browser/atom_blob_reader.h | 10 ++++----- docs/api/promisification.md | 2 +- docs/api/session.md | 8 +++++++ lib/browser/api/session.js | 1 + spec/api-session-spec.js | 4 ++-- 8 files changed, 45 insertions(+), 38 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 71a4db99b015..ad4ac1eda30f 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -209,7 +209,7 @@ const char kPersistPrefix[] = "persist:"; // Referenced session objects. std::map> g_sessions; -void ResolveOrRejectPromiseInUI(atom::util::Promise promise, int net_error) { +void ResolveOrRejectPromiseInUI(util::Promise promise, int net_error) { if (net_error != net::OK) { std::string err_msg = net::ErrorToString(net_error); util::Promise::RejectPromise(std::move(promise), std::move(err_msg)); @@ -221,7 +221,7 @@ void ResolveOrRejectPromiseInUI(atom::util::Promise promise, int net_error) { // Callback of HttpCache::GetBackend. void OnGetBackend(disk_cache::Backend** backend_ptr, Session::CacheAction action, - const atom::util::CopyablePromise& promise, + const util::CopyablePromise& promise, int result) { if (result != net::OK) { std::string err_msg = @@ -254,7 +254,7 @@ void OnGetBackend(disk_cache::Backend** backend_ptr, void DoCacheActionInIO( const scoped_refptr& context_getter, Session::CacheAction action, - atom::util::Promise promise) { + util::Promise promise) { auto* request_context = context_getter->GetURLRequestContext(); auto* http_cache = request_context->http_transaction_factory()->GetCache(); @@ -270,7 +270,7 @@ void DoCacheActionInIO( auto** backend_ptr = new BackendPtr(nullptr); net::CompletionCallback on_get_backend = base::Bind(&OnGetBackend, base::Owned(backend_ptr), action, - atom::util::CopyablePromise(promise)); + util::CopyablePromise(promise)); int rv = http_cache->GetBackend(backend_ptr, on_get_backend); if (rv != net::ERR_IO_PENDING) on_get_backend.Run(net::OK); @@ -431,7 +431,7 @@ v8::Local Session::ResolveProxy(mate::Arguments* args) { browser_context_->GetResolveProxyHelper()->ResolveProxy( url, base::Bind(util::CopyablePromise::ResolveCopyablePromise, - atom::util::CopyablePromise(promise))); + util::CopyablePromise(promise))); return handle; } @@ -659,16 +659,17 @@ std::string Session::GetUserAgent() { return browser_context_->GetUserAgent(); } -void Session::GetBlobData(const std::string& uuid, - const AtomBlobReader::CompletionCallback& callback) { - if (callback.is_null()) - return; +v8::Local Session::GetBlobData(v8::Isolate* isolate, + const std::string& uuid) { + util::Promise promise(isolate); + v8::Local handle = promise.GetHandle(); AtomBlobReader* blob_reader = browser_context()->GetBlobReader(); base::PostTaskWithTraits( FROM_HERE, {BrowserThread::IO}, base::BindOnce(&AtomBlobReader::StartReading, - base::Unretained(blob_reader), uuid, callback)); + base::Unretained(blob_reader), uuid, std::move(promise))); + return handle; } void Session::CreateInterruptedDownload(const mate::Dictionary& options) { diff --git a/atom/browser/api/atom_api_session.h b/atom/browser/api/atom_api_session.h index 82c159104c9c..3904353fd46b 100644 --- a/atom/browser/api/atom_api_session.h +++ b/atom/browser/api/atom_api_session.h @@ -82,8 +82,8 @@ class Session : public mate::TrackableObject, void AllowNTLMCredentialsForDomains(const std::string& domains); void SetUserAgent(const std::string& user_agent, mate::Arguments* args); std::string GetUserAgent(); - void GetBlobData(const std::string& uuid, - const AtomBlobReader::CompletionCallback& callback); + v8::Local GetBlobData(v8::Isolate* isolate, + const std::string& uuid); void CreateInterruptedDownload(const mate::Dictionary& options); void SetPreloads(const std::vector& preloads); std::vector GetPreloads() const; diff --git a/atom/browser/atom_blob_reader.cc b/atom/browser/atom_blob_reader.cc index e41090b2206a..9ea4c5bc375f 100644 --- a/atom/browser/atom_blob_reader.cc +++ b/atom/browser/atom_blob_reader.cc @@ -27,12 +27,10 @@ void FreeNodeBufferData(char* data, void* hint) { delete[] data; } -void RunCallbackInUI(const AtomBlobReader::CompletionCallback& callback, - char* blob_data, - int size) { +void RunPromiseInUI(util::Promise promise, char* blob_data, int size) { DCHECK_CURRENTLY_ON(BrowserThread::UI); + v8::Isolate* isolate = promise.isolate(); - v8::Isolate* isolate = v8::Isolate::GetCurrent(); v8::Locker locker(isolate); v8::HandleScope handle_scope(isolate); if (blob_data) { @@ -40,9 +38,9 @@ void RunCallbackInUI(const AtomBlobReader::CompletionCallback& callback, node::Buffer::New(isolate, blob_data, static_cast(size), &FreeNodeBufferData, nullptr) .ToLocalChecked(); - callback.Run(buffer); + promise.Resolve(buffer); } else { - callback.Run(v8::Null(isolate)); + promise.RejectWithErrorMessage("Could not get blob data"); } } @@ -53,29 +51,29 @@ AtomBlobReader::AtomBlobReader(content::ChromeBlobStorageContext* blob_context) AtomBlobReader::~AtomBlobReader() {} -void AtomBlobReader::StartReading( - const std::string& uuid, - const AtomBlobReader::CompletionCallback& completion_callback) { +void AtomBlobReader::StartReading(const std::string& uuid, + util::Promise promise) { DCHECK_CURRENTLY_ON(BrowserThread::IO); auto blob_data_handle = blob_context_->context()->GetBlobDataFromUUID(uuid); - auto callback = base::Bind(&RunCallbackInUI, completion_callback); if (!blob_data_handle) { - base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, - base::BindOnce(callback, nullptr, 0)); + util::Promise::RejectPromise(std::move(promise), + "Could not get blob data handle"); return; } auto blob_reader = blob_data_handle->CreateReader(); BlobReadHelper* blob_read_helper = - new BlobReadHelper(std::move(blob_reader), callback); + new BlobReadHelper(std::move(blob_reader), + base::BindOnce(&RunPromiseInUI, std::move(promise))); blob_read_helper->Read(); } AtomBlobReader::BlobReadHelper::BlobReadHelper( std::unique_ptr blob_reader, - const BlobReadHelper::CompletionCallback& callback) - : blob_reader_(std::move(blob_reader)), completion_callback_(callback) {} + BlobReadHelper::CompletionCallback callback) + : blob_reader_(std::move(blob_reader)), + completion_callback_(std::move(callback)) {} AtomBlobReader::BlobReadHelper::~BlobReadHelper() {} @@ -117,8 +115,9 @@ void AtomBlobReader::BlobReadHelper::DidReadBlobData( char* data = new char[size]; memcpy(data, blob_data->data(), size); - base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, - base::BindOnce(completion_callback_, data, size)); + base::PostTaskWithTraits( + FROM_HERE, {BrowserThread::UI}, + base::BindOnce(std::move(completion_callback_), data, size)); delete this; } diff --git a/atom/browser/atom_blob_reader.h b/atom/browser/atom_blob_reader.h index 871bb77ee240..ebc1ca47eeed 100644 --- a/atom/browser/atom_blob_reader.h +++ b/atom/browser/atom_blob_reader.h @@ -8,6 +8,7 @@ #include #include +#include "atom/common/promise_util.h" #include "base/callback.h" namespace content { @@ -35,23 +36,20 @@ namespace atom { // except Ctor are expected to be called on IO thread. class AtomBlobReader { public: - using CompletionCallback = base::Callback)>; - explicit AtomBlobReader(content::ChromeBlobStorageContext* blob_context); ~AtomBlobReader(); - void StartReading(const std::string& uuid, - const AtomBlobReader::CompletionCallback& callback); + void StartReading(const std::string& uuid, atom::util::Promise promise); private: // A self-destroyed helper class to read the blob data. // Must be accessed on IO thread. class BlobReadHelper { public: - using CompletionCallback = base::Callback; + using CompletionCallback = base::OnceCallback; BlobReadHelper(std::unique_ptr blob_reader, - const BlobReadHelper::CompletionCallback& callback); + BlobReadHelper::CompletionCallback callback); ~BlobReadHelper(); void Read(); diff --git a/docs/api/promisification.md b/docs/api/promisification.md index b00edac6854d..9791050ae4a0 100644 --- a/docs/api/promisification.md +++ b/docs/api/promisification.md @@ -11,7 +11,6 @@ When a majority of affected functions are migrated, this flag will be enabled by - [app.importCertificate(options, callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#importCertificate) - [dialog.showMessageBox([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showMessageBox) - [dialog.showCertificateTrustDialog([browserWindow, ]options, callback)](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showCertificateTrustDialog) -- [ses.getBlobData(identifier, callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#getBlobData) - [contents.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#executeJavaScript) - [contents.print([options], [callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#print) - [webFrame.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-frame.md#executeJavaScript) @@ -47,6 +46,7 @@ When a majority of affected functions are migrated, this flag will be enabled by - [ses.getCacheSize(callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#getCacheSize) - [ses.clearAuthCache(options[, callback])](https://github.com/electron/electron/blob/master/docs/api/session.md#clearAuthCache) - [ses.clearCache(callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#clearCache) +- [ses.getBlobData(identifier, callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#getBlobData) - [shell.openExternal(url[, options, callback])](https://github.com/electron/electron/blob/master/docs/api/shell.md#openExternal) - [webviewTag.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#capturePage) - [webviewTag.printToPDF(options, callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#printToPDF) diff --git a/docs/api/session.md b/docs/api/session.md index 689fc0d7eab9..0cc4af3f4adc 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -512,6 +512,14 @@ Returns `String` - The user agent for this session. * `callback` Function * `result` Buffer - Blob data. +**[Deprecated Soon](promisification.md)** + +#### `ses.getBlobData(identifier)` + +* `identifier` String - Valid UUID. + +Returns `Promise` - resolves with blob data. + #### `ses.createInterruptedDownload(options)` * `options` Object diff --git a/lib/browser/api/session.js b/lib/browser/api/session.js index baf54b9b147f..a09ed0c6b092 100644 --- a/lib/browser/api/session.js +++ b/lib/browser/api/session.js @@ -30,6 +30,7 @@ Session.prototype.setProxy = deprecate.promisify(Session.prototype.setProxy) Session.prototype.getCacheSize = deprecate.promisify(Session.prototype.getCacheSize) Session.prototype.clearCache = deprecate.promisify(Session.prototype.clearCache) Session.prototype.clearAuthCache = deprecate.promisify(Session.prototype.clearAuthCache) +Session.prototype.getBlobData = deprecate.promisifyMultiArg(Session.prototype.getBlobData) Cookies.prototype.flushStore = deprecate.promisify(Cookies.prototype.flushStore) Cookies.prototype.get = deprecate.promisify(Cookies.prototype.get) diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index f37c7a11462f..c6efdb17a5f9 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -774,7 +774,7 @@ describe('session module', () => { }) }) - describe('ses.getBlobData(identifier, callback)', () => { + describe('ses.getBlobData()', () => { it('returns blob data for uuid', (done) => { const scheme = 'cors-blob' const protocol = session.defaultSession.protocol @@ -811,7 +811,7 @@ describe('session module', () => { } else if (request.method === 'POST') { const uuid = request.uploadData[1].blobUUID assert(uuid) - session.defaultSession.getBlobData(uuid, (result) => { + session.defaultSession.getBlobData(uuid).then(result => { assert.strictEqual(result.toString(), postData) done() })