feat: promisify session.getBlobData() (#17303)

atom/browser/api/atom_api_session.cc

@@ -209,7 +209,7 @@ const char kPersistPrefix[] = "persist:";
 // Referenced session objects.
 std::map<uint32_t, v8::Global<v8::Object>> 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<net::URLRequestContextGetter>& 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<v8::Promise> Session::ResolveProxy(mate::Arguments* args) {
   browser_context_->GetResolveProxyHelper()->ResolveProxy(
       url,
       base::Bind(util::CopyablePromise::ResolveCopyablePromise<std::string>,
-                 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,
+v8::Local<v8::Promise> Session::GetBlobData(v8::Isolate* isolate,
-                          const AtomBlobReader::CompletionCallback& callback) {
+                                            const std::string& uuid) {
-  if (callback.is_null())
+  util::Promise promise(isolate);
-    return;
+  v8::Local<v8::Promise> 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) {

atom/browser/api/atom_api_session.h

@@ -82,8 +82,8 @@ class Session : public mate::TrackableObject<Session>,
   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,
+  v8::Local<v8::Promise> GetBlobData(v8::Isolate* isolate,
-                   const AtomBlobReader::CompletionCallback& callback);
+                                     const std::string& uuid);
   void CreateInterruptedDownload(const mate::Dictionary& options);
   void SetPreloads(const std::vector<base::FilePath::StringType>& preloads);
   std::vector<base::FilePath::StringType> GetPreloads() const;

atom/browser/atom_blob_reader.cc

@@ -27,12 +27,10 @@ void FreeNodeBufferData(char* data, void* hint) {
   delete[] data;
 }
 
-void RunCallbackInUI(const AtomBlobReader::CompletionCallback& callback,
+void RunPromiseInUI(util::Promise promise, char* blob_data, int size) {
-                     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_t>(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(
+void AtomBlobReader::StartReading(const std::string& uuid,
-    const std::string& uuid,
+                                  util::Promise promise) {
-    const AtomBlobReader::CompletionCallback& completion_callback) {
   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},
+    util::Promise::RejectPromise(std::move(promise),
-                             base::BindOnce(callback, nullptr, 0));
+                                 "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<storage::BlobReader> blob_reader,
-    const BlobReadHelper::CompletionCallback& callback)
+    BlobReadHelper::CompletionCallback callback)
-    : blob_reader_(std::move(blob_reader)), completion_callback_(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::PostTaskWithTraits(
-                           base::BindOnce(completion_callback_, data, size));
+      FROM_HERE, {BrowserThread::UI},
+      base::BindOnce(std::move(completion_callback_), data, size));
   delete this;
 }
 

atom/browser/atom_blob_reader.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#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<void(v8::Local<v8::Value>)>;
-
   explicit AtomBlobReader(content::ChromeBlobStorageContext* blob_context);
   ~AtomBlobReader();
 
-  void StartReading(const std::string& uuid,
+  void StartReading(const std::string& uuid, atom::util::Promise promise);
-                    const AtomBlobReader::CompletionCallback& callback);
 
  private:
   // A self-destroyed helper class to read the blob data.
   // Must be accessed on IO thread.
   class BlobReadHelper {
    public:
-    using CompletionCallback = base::Callback<void(char*, int)>;
+    using CompletionCallback = base::OnceCallback<void(char*, int)>;
 
     BlobReadHelper(std::unique_ptr<storage::BlobReader> blob_reader,
-                   const BlobReadHelper::CompletionCallback& callback);
+                   BlobReadHelper::CompletionCallback callback);
     ~BlobReadHelper();
 
     void Read();

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)

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<Buffer>` - resolves with blob data.
+
 #### `ses.createInterruptedDownload(options)`
 
 * `options` Object

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)

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()
           })