From 8f6f474306d91602fe0cdafddf0dcb3c5c324556 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Fri, 8 Mar 2019 15:57:33 -0800 Subject: [PATCH] feat: promisify session.getBlobData() --- atom/browser/api/atom_api_session.cc | 11 ++++++----- atom/browser/api/atom_api_session.h | 3 +-- atom/browser/atom_blob_reader.cc | 27 +++++++++++++-------------- atom/browser/atom_blob_reader.h | 6 ++---- docs/api/promisification.md | 2 +- docs/api/session.md | 8 ++++++++ lib/browser/api/session.js | 1 + spec/api-session-spec.js | 4 ++-- spec/package-lock.json | 22 ++++++++++++++++------ 9 files changed, 50 insertions(+), 34 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 665d306d1b901..7c52ecebd9468 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -660,16 +660,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(const std::string& uuid) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + 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 82c159104c9c9..f6b4f206a638c 100644 --- a/atom/browser/api/atom_api_session.h +++ b/atom/browser/api/atom_api_session.h @@ -82,8 +82,7 @@ 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(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 cfb89bc8de1c0..dd6471efa8918 100644 --- a/atom/browser/atom_blob_reader.cc +++ b/atom/browser/atom_blob_reader.cc @@ -28,12 +28,12 @@ void FreeNodeBufferData(char* data, void* hint) { delete[] data; } -void RunCallbackInUI(const AtomBlobReader::CompletionCallback& callback, - char* blob_data, - int size) { +void RunPromiseInUI(const atom::util::CopyablePromise& promise, + char* blob_data, + int size) { DCHECK_CURRENTLY_ON(BrowserThread::UI); + v8::Isolate* isolate = promise.GetPromise().isolate(); - v8::Isolate* isolate = v8::Isolate::GetCurrent(); v8::Locker locker(isolate); v8::HandleScope handle_scope(isolate); if (blob_data) { @@ -41,9 +41,9 @@ void RunCallbackInUI(const AtomBlobReader::CompletionCallback& callback, node::Buffer::New(isolate, blob_data, static_cast(size), &FreeNodeBufferData, nullptr) .ToLocalChecked(); - callback.Run(buffer); + promise.GetPromise().Resolve(buffer); } else { - callback.Run(v8::Null(isolate)); + promise.GetPromise().RejectWithErrorMessage("Could not get blob data"); } } @@ -54,22 +54,21 @@ 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, + atom::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)); + atom::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); + BlobReadHelper* blob_read_helper = new BlobReadHelper( + std::move(blob_reader), + base::Bind(&RunPromiseInUI, atom::util::CopyablePromise(promise))); blob_read_helper->Read(); } diff --git a/atom/browser/atom_blob_reader.h b/atom/browser/atom_blob_reader.h index 871bb77ee240a..5a605de0da995 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,13 +36,10 @@ 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. diff --git a/docs/api/promisification.md b/docs/api/promisification.md index 9838b378f123a..d514b241a63c3 100644 --- a/docs/api/promisification.md +++ b/docs/api/promisification.md @@ -13,7 +13,6 @@ When a majority of affected functions are migrated, this flag will be enabled by - [dialog.showCertificateTrustDialog([browserWindow, ]options, callback)](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showCertificateTrustDialog) - [inAppPurchase.purchaseProduct(productID, quantity, callback)](https://github.com/electron/electron/blob/master/docs/api/in-app-purchase.md#purchaseProduct) - [inAppPurchase.getProducts(productIDs, callback)](https://github.com/electron/electron/blob/master/docs/api/in-app-purchase.md#getProducts) -- [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 689fc0d7eab94..0cc4af3f4adcf 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 baf54b9b147fb..a09ed0c6b0927 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 f37c7a11462f1..0ba7c1888b8e8 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) => { + const result = session.defaultSession.getBlobData(uuid).then(result => { assert.strictEqual(result.toString(), postData) done() }) diff --git a/spec/package-lock.json b/spec/package-lock.json index 3ba22e9417a2f..77f07fc31c017 100644 --- a/spec/package-lock.json +++ b/spec/package-lock.json @@ -8,7 +8,6 @@ "version": "github:nornagon/node-abstractsocket#7d9c770f9ffef14373349034f8820ff059879845", "from": "github:nornagon/node-abstractsocket#v8-compat", "dev": true, - "optional": true, "requires": { "bindings": "^1.2.1", "nan": "^2.0.9" @@ -67,7 +66,6 @@ "resolved": "https://registry.npmjs.org/bindings/-/bindings-1.4.0.tgz", "integrity": "sha512-7znEVX22Djn+nYjxCWKDne0RRloa9XfYa84yk3s+HkE3LpDYZmhArYr9O9huBoHY3/oXispx5LorIX7Sl2CgSQ==", "dev": true, - "optional": true, "requires": { "file-uri-to-path": "1.0.0" } @@ -285,6 +283,19 @@ "put": "0.0.6", "safe-buffer": "^5.1.1", "xml2js": "^0.4.17" + }, + "dependencies": { + "abstract-socket": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/abstract-socket/-/abstract-socket-2.0.0.tgz", + "integrity": "sha1-2DyT598w0n4j8+gqdj5/XnjZFvk=", + "dev": true, + "optional": true, + "requires": { + "bindings": "^1.2.1", + "nan": "^2.0.9" + } + } } }, "debug": { @@ -459,8 +470,7 @@ "version": "1.0.0", "resolved": "https://registry.npmjs.org/file-uri-to-path/-/file-uri-to-path-1.0.0.tgz", "integrity": "sha512-0Zt+s3L7Vf1biwWZ29aARiVYLx7iMGnEUl9x33fbB/j3jR81u/O2LbqK+Bm1CDSNDKVtJ/YjwY7TUd5SkeLQLw==", - "dev": true, - "optional": true + "dev": true }, "find-up": { "version": "3.0.0", @@ -778,7 +788,7 @@ }, "minimist": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "optional": true }, @@ -1151,7 +1161,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "optional": true, "requires": {