Browse Source

refactor: remove more uses of v8::Isolate::GetCurrent() (#28429)

Co-authored-by: Shelley Vohr <[email protected]>
trop[bot] 4 years ago
parent
commit
250438c2d9

+ 2 - 1
shell/browser/api/electron_api_crash_reporter.cc

@@ -190,8 +190,9 @@ namespace {
 
 #if defined(MAS_BUILD)
 void GetUploadedReports(
+    v8::Isolate* isolate,
     base::OnceCallback<void(v8::Local<v8::Value>)> callback) {
-  std::move(callback).Run(v8::Array::New(v8::Isolate::GetCurrent()));
+  std::move(callback).Run(v8::Array::New(isolate));
 }
 #else
 scoped_refptr<UploadList> CreateCrashUploadList() {

+ 5 - 5
shell/browser/api/electron_api_download_item.cc

@@ -84,7 +84,7 @@ DownloadItem* DownloadItem::FromDownloadItem(
 
 DownloadItem::DownloadItem(v8::Isolate* isolate,
                            download::DownloadItem* download_item)
-    : download_item_(download_item) {
+    : download_item_(download_item), isolate_(isolate) {
   download_item_->AddObserver(this);
   download_item_->SetUserData(
       kElectronApiDownloadItemKey,
@@ -101,8 +101,8 @@ DownloadItem::~DownloadItem() {
 
 bool DownloadItem::CheckAlive() const {
   if (!download_item_) {
-    gin_helper::ErrorThrower(v8::Isolate::GetCurrent())
-        .ThrowError("DownloadItem used after being destroyed");
+    gin_helper::ErrorThrower(isolate_).ThrowError(
+        "DownloadItem used after being destroyed");
     return false;
   }
   return true;
@@ -200,10 +200,10 @@ const GURL& DownloadItem::GetURL() const {
   return download_item_->GetURL();
 }
 
-v8::Local<v8::Value> DownloadItem::GetURLChain(v8::Isolate* isolate) const {
+v8::Local<v8::Value> DownloadItem::GetURLChain() const {
   if (!CheckAlive())
     return v8::Local<v8::Value>();
-  return gin::ConvertToV8(isolate, download_item_->GetUrlChain());
+  return gin::ConvertToV8(isolate_, download_item_->GetUrlChain());
 }
 
 download::DownloadItem::DownloadState DownloadItem::GetState() const {

+ 3 - 1
shell/browser/api/electron_api_download_item.h

@@ -66,7 +66,7 @@ class DownloadItem : public gin::Wrappable<DownloadItem>,
   std::string GetFilename() const;
   std::string GetContentDisposition() const;
   const GURL& GetURL() const;
-  v8::Local<v8::Value> GetURLChain(v8::Isolate*) const;
+  v8::Local<v8::Value> GetURLChain() const;
   download::DownloadItem::DownloadState GetState() const;
   bool IsDone() const;
   void SetSaveDialogOptions(const file_dialog::DialogSettings& options);
@@ -78,6 +78,8 @@ class DownloadItem : public gin::Wrappable<DownloadItem>,
   file_dialog::DialogSettings dialog_options_;
   download::DownloadItem* download_item_;
 
+  v8::Isolate* isolate_;
+
   base::WeakPtrFactory<DownloadItem> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(DownloadItem);

+ 1 - 1
shell/browser/api/electron_api_menu.cc

@@ -264,7 +264,7 @@ void Menu::OnMenuWillClose() {
 }
 
 void Menu::OnMenuWillShow() {
-  Pin(v8::Isolate::GetCurrent());
+  Pin(JavascriptEnvironment::GetIsolate());
   Emit("menu-will-show");
 }
 

+ 20 - 29
shell/browser/api/electron_api_session.cc

@@ -331,7 +331,8 @@ const void* kElectronApiSessionKey = &kElectronApiSessionKey;
 gin::WrapperInfo Session::kWrapperInfo = {gin::kEmbedderNativeGin};
 
 Session::Session(v8::Isolate* isolate, ElectronBrowserContext* browser_context)
-    : network_emulation_token_(base::UnguessableToken::Create()),
+    : isolate_(isolate),
+      network_emulation_token_(base::UnguessableToken::Create()),
       browser_context_(browser_context) {
   // Observe DownloadManager to get download notifications.
   content::BrowserContext::GetDownloadManager(browser_context)
@@ -379,10 +380,9 @@ void Session::OnDownloadCreated(content::DownloadManager* manager,
   if (item->IsSavePackageDownload())
     return;
 
-  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
-  v8::Locker locker(isolate);
-  v8::HandleScope handle_scope(isolate);
-  auto handle = DownloadItem::FromOrCreate(isolate, item);
+  v8::Locker locker(isolate_);
+  v8::HandleScope handle_scope(isolate_);
+  auto handle = DownloadItem::FromOrCreate(isolate_, item);
   if (item->GetState() == download::DownloadItem::INTERRUPTED)
     handle->SetSavePath(item->GetTargetFilePath());
   content::WebContents* web_contents =
@@ -425,8 +425,7 @@ v8::Local<v8::Promise> Session::ResolveProxy(gin::Arguments* args) {
 }
 
 v8::Local<v8::Promise> Session::GetCacheSize() {
-  auto* isolate = JavascriptEnvironment::GetIsolate();
-  gin_helper::Promise<int64_t> promise(isolate);
+  gin_helper::Promise<int64_t> promise(isolate_);
   auto handle = promise.GetHandle();
 
   content::BrowserContext::GetDefaultStoragePartition(browser_context_)
@@ -449,8 +448,7 @@ v8::Local<v8::Promise> Session::GetCacheSize() {
 }
 
 v8::Local<v8::Promise> Session::ClearCache() {
-  auto* isolate = JavascriptEnvironment::GetIsolate();
-  gin_helper::Promise<void> promise(isolate);
+  gin_helper::Promise<void> promise(isolate_);
   auto handle = promise.GetHandle();
 
   content::BrowserContext::GetDefaultStoragePartition(browser_context_)
@@ -558,8 +556,7 @@ v8::Local<v8::Promise> Session::SetProxy(gin::Arguments* args) {
 }
 
 v8::Local<v8::Promise> Session::ForceReloadProxyConfig() {
-  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
-  gin_helper::Promise<void> promise(isolate);
+  gin_helper::Promise<void> promise(isolate_);
   auto handle = promise.GetHandle();
 
   content::BrowserContext::GetDefaultStoragePartition(browser_context_)
@@ -675,8 +672,7 @@ v8::Local<v8::Promise> Session::ClearHostResolverCache(gin::Arguments* args) {
 }
 
 v8::Local<v8::Promise> Session::ClearAuthCache() {
-  auto* isolate = JavascriptEnvironment::GetIsolate();
-  gin_helper::Promise<void> promise(isolate);
+  gin_helper::Promise<void> promise(isolate_);
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
   content::BrowserContext::GetDefaultStoragePartition(browser_context_)
@@ -763,15 +759,14 @@ void Session::CreateInterruptedDownload(const gin_helper::Dictionary& options) {
   options.Get("lastModified", &last_modified);
   options.Get("eTag", &etag);
   options.Get("startTime", &start_time);
-  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
   if (path.empty() || url_chain.empty() || length == 0) {
-    isolate->ThrowException(v8::Exception::Error(gin::StringToV8(
-        isolate, "Must pass non-empty path, urlChain and length.")));
+    isolate_->ThrowException(v8::Exception::Error(gin::StringToV8(
+        isolate_, "Must pass non-empty path, urlChain and length.")));
     return;
   }
   if (offset >= length) {
-    isolate->ThrowException(v8::Exception::Error(gin::StringToV8(
-        isolate, "Must pass an offset value less than length.")));
+    isolate_->ThrowException(v8::Exception::Error(gin::StringToV8(
+        isolate_, "Must pass an offset value less than length.")));
     return;
   }
   auto* download_manager =
@@ -797,8 +792,7 @@ std::vector<base::FilePath> Session::GetPreloads() const {
 v8::Local<v8::Promise> Session::LoadExtension(
     const base::FilePath& extension_path,
     gin::Arguments* args) {
-  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
-  gin_helper::Promise<const extensions::Extension*> promise(isolate);
+  gin_helper::Promise<const extensions::Extension*> promise(isolate_);
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
   if (!extension_path.IsAbsolute()) {
@@ -833,7 +827,7 @@ v8::Local<v8::Promise> Session::LoadExtension(
             if (extension) {
               if (!error_msg.empty()) {
                 node::Environment* env =
-                    node::Environment::GetCurrent(v8::Isolate::GetCurrent());
+                    node::Environment::GetCurrent(promise.isolate());
                 EmitWarning(env, error_msg, "ExtensionLoadWarning");
               }
               promise.Resolve(extension);
@@ -856,11 +850,10 @@ v8::Local<v8::Value> Session::GetExtension(const std::string& extension_id) {
   auto* registry = extensions::ExtensionRegistry::Get(browser_context());
   const extensions::Extension* extension =
       registry->GetInstalledExtension(extension_id);
-  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
   if (extension) {
-    return gin::ConvertToV8(isolate, extension);
+    return gin::ConvertToV8(isolate_, extension);
   } else {
-    return v8::Null(isolate);
+    return v8::Null(isolate_);
   }
 }
 
@@ -872,7 +865,7 @@ v8::Local<v8::Value> Session::GetAllExtensions() {
     if (extension->location() != extensions::Manifest::COMPONENT)
       extensions_vector.emplace_back(extension.get());
   }
-  return gin::ConvertToV8(v8::Isolate::GetCurrent(), extensions_vector);
+  return gin::ConvertToV8(isolate_, extensions_vector);
 }
 
 void Session::OnExtensionLoaded(content::BrowserContext* browser_context,
@@ -967,8 +960,7 @@ void Session::Preconnect(const gin_helper::Dictionary& options,
 }
 
 v8::Local<v8::Promise> Session::CloseAllConnections() {
-  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
-  gin_helper::Promise<void> promise(isolate);
+  gin_helper::Promise<void> promise(isolate_);
   auto handle = promise.GetHandle();
 
   content::BrowserContext::GetDefaultStoragePartition(browser_context_)
@@ -1018,8 +1010,7 @@ void SetSpellCheckerDictionaryDownloadURL(gin_helper::ErrorThrower thrower,
 }
 
 v8::Local<v8::Promise> Session::ListWordsInSpellCheckerDictionary() {
-  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
-  gin_helper::Promise<std::set<std::string>> promise(isolate);
+  gin_helper::Promise<std::set<std::string>> promise(isolate_);
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
   SpellcheckService* spellcheck =

+ 2 - 0
shell/browser/api/electron_api_session.h

@@ -178,6 +178,8 @@ class Session : public gin::Wrappable<Session>,
   v8::Global<v8::Value> service_worker_context_;
   v8::Global<v8::Value> web_request_;
 
+  v8::Isolate* isolate_;
+
   // The client id to enable the network throttler.
   base::UnguessableToken network_emulation_token_;
 

+ 2 - 1
shell/browser/api/electron_api_url_loader.cc

@@ -312,7 +312,8 @@ void SimpleURLLoaderWrapper::Pin() {
 }
 
 void SimpleURLLoaderWrapper::PinBodyGetter(v8::Local<v8::Value> body_getter) {
-  pinned_chunk_pipe_getter_.Reset(v8::Isolate::GetCurrent(), body_getter);
+  pinned_chunk_pipe_getter_.Reset(JavascriptEnvironment::GetIsolate(),
+                                  body_getter);
 }
 
 SimpleURLLoaderWrapper::~SimpleURLLoaderWrapper() {

+ 7 - 5
shell/browser/native_window_mac.mm

@@ -22,6 +22,7 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/desktop_media_id.h"
+#include "shell/browser/javascript_environment.h"
 #include "shell/browser/native_browser_view_mac.h"
 #include "shell/browser/ui/cocoa/electron_native_widget_mac.h"
 #include "shell/browser/ui/cocoa/electron_ns_window.h"
@@ -278,10 +279,11 @@ NativeWindowMac::NativeWindowMac(const gin_helper::Dictionary& options,
   options.Get(options::kVisualEffectState, &visual_effect_state_);
 
   if (options.Has(options::kFullscreenWindowTitle)) {
-    EmitWarning(node::Environment::GetCurrent(v8::Isolate::GetCurrent()),
-                "\"fullscreenWindowTitle\" option has been deprecated and is "
-                "no-op now.",
-                "electron");
+    EmitWarning(
+        node::Environment::GetCurrent(JavascriptEnvironment::GetIsolate()),
+        "\"fullscreenWindowTitle\" option has been deprecated and is "
+        "no-op now.",
+        "electron");
   }
 
   bool minimizable = true;
@@ -1313,7 +1315,7 @@ void NativeWindowMac::SetVibrancy(const std::string& type) {
 
   std::string dep_warn = " has been deprecated and removed as of macOS 10.15.";
   node::Environment* env =
-      node::Environment::GetCurrent(v8::Isolate::GetCurrent());
+      node::Environment::GetCurrent(JavascriptEnvironment::GetIsolate());
 
   NSVisualEffectMaterial vibrancyType;
 

+ 1 - 1
shell/renderer/api/electron_api_web_frame.cc

@@ -205,7 +205,7 @@ class ScriptExecutionCallback : public blink::WebScriptExecutionCallback {
 
   void Completed(
       const blink::WebVector<v8::Local<v8::Value>>& result) override {
-    v8::Isolate* isolate = v8::Isolate::GetCurrent();
+    v8::Isolate* isolate = promise_.isolate();
     if (!result.empty()) {
       if (!result[0].IsEmpty()) {
         v8::Local<v8::Value> value = result[0];