Repository: electron/electron
Pull request: perf: avoid duplicate calculations in gin_helper::Dictionary getters

#### Description of Change

- In getters, hold the result of `GetHandle()` in a temp variable instead of computing it twice
- Prefer [v8::Maybe&lt;bool&gt;::FromMaybe(false)](https://source.chromium.org/chromium/_/chromium/v8/v8/+/47f420e89ec1b33dacc048d93e0317ab7fec43dd:include/v8-maybe.h;l=81) over  [IsJust()](https://source.chromium.org/chromium/_/chromium/v8/v8/+/47f420e89ec1b33dacc048d93e0317ab7fec43dd:include/v8-maybe.h;l=35) && [FromJust()](https://source.chromium.org/chromium/_/chromium/v8/v8/+/47f420e89ec1b33dacc048d93e0317ab7fec43dd:include/v8-maybe.h;l=63) because the inlined code is simpler and safer
- Extract duplicate code into new private helper functions `MakeKey()` and `MakeHiddenKey()`

#### Checklist

- [x] PR description included and stakeholders cc'd
- [x] `npm test` passes
- [x] [PR release notes](https://github.com/electron/clerk/blob/main/README.md) describe the change in a way relevant to app developers, and are [capitalized, punctuated, and past tense](https://github.com/electron/clerk/blob/main/README.md#examples).

#### Release Notes

Notes: none.

Changed files:
filename: shell/common/gin_helper/dictionary.h
patch: @@ -42,16 +42,16 @@ class Dictionary : public gin::Dictionary {
   // 3. It accepts arbitrary type of key.
   template <typename K, typename V>
   bool Get(const K& key, V* out) const {
+    v8::Isolate* const iso = isolate();
+    v8::Local<v8::Object> handle = GetHandle();
+    v8::Local<v8::Context> context = iso->GetCurrentContext();
+    v8::Local<v8::Value> v8_key = gin::ConvertToV8(iso, key);
+    v8::Local<v8::Value> value;
     // Check for existence before getting, otherwise this method will always
     // returns true when T == v8::Local<v8::Value>.
-    v8::Local<v8::Context> context = isolate()->GetCurrentContext();
-    v8::Local<v8::Value> v8_key = gin::ConvertToV8(isolate(), key);
-    v8::Local<v8::Value> value;
-    v8::Maybe<bool> result = GetHandle()->Has(context, v8_key);
-    if (result.IsJust() && result.FromJust() &&
-        GetHandle()->Get(context, v8_key).ToLocal(&value))
-      return gin::ConvertFromV8(isolate(), value, out);
-    return false;
+    return handle->Has(context, v8_key).FromMaybe(false) &&
+           handle->Get(context, v8_key).ToLocal(&value) &&
+           gin::ConvertFromV8(iso, value, out);
   }
 
   // Differences from the Set method in gin::Dictionary:
@@ -64,7 +64,7 @@ class Dictionary : public gin::Dictionary {
     v8::Maybe<bool> result =
         GetHandle()->Set(isolate()->GetCurrentContext(),
                          gin::ConvertToV8(isolate(), key), v8_value);
-    return !result.IsNothing() && result.FromJust();
+    return result.FromMaybe(false);
   }
 
   // Like normal Get but put result in an std::optional.
@@ -81,36 +81,35 @@ class Dictionary : public gin::Dictionary {
 
   template <typename T>
   bool GetHidden(std::string_view key, T* out) const {
-    v8::Local<v8::Context> context = isolate()->GetCurrentContext();
-    v8::Local<v8::Private> privateKey =
-        v8::Private::ForApi(isolate(), gin::StringToV8(isolate(), key));
+    v8::Isolate* const iso = isolate();
+    v8::Local<v8::Object> handle = GetHandle();
+    v8::Local<v8::Context> context = iso->GetCurrentContext();
+    v8::Local<v8::Private> privateKey = MakeHiddenKey(key);
     v8::Local<v8::Value> value;
-    v8::Maybe<bool> result = GetHandle()->HasPrivate(context, privateKey);
-    if (result.IsJust() && result.FromJust() &&
-        GetHandle()->GetPrivate(context, privateKey).ToLocal(&value))
-      return gin::ConvertFromV8(isolate(), value, out);
-    return false;
+    return handle->HasPrivate(context, privateKey).FromMaybe(false) &&
+           handle->GetPrivate(context, privateKey).ToLocal(&value) &&
+           gin::ConvertFromV8(iso, value, out);
   }
 
   template <typename T>
   bool SetHidden(std::string_view key, T val) {
+    v8::Isolate* const iso = isolate();
     v8::Local<v8::Value> v8_value;
-    if (!gin::TryConvertToV8(isolate(), val, &v8_value))
+    if (!gin::TryConvertToV8(iso, val, &v8_value))
       return false;
-    v8::Local<v8::Context> context = isolate()->GetCurrentContext();
-    v8::Local<v8::Private> privateKey =
-        v8::Private::ForApi(isolate(), gin::StringToV8(isolate(), key));
+    v8::Local<v8::Context> context = iso->GetCurrentContext();
+    v8::Local<v8::Private> privateKey = MakeHiddenKey(key);
     v8::Maybe<bool> result =
         GetHandle()->SetPrivate(context, privateKey, v8_value);
-    return !result.IsNothing() && result.FromJust();
+    return result.FromMaybe(false);
   }
 
   template <typename T>
   bool SetMethod(std::string_view key, const T& callback) {
     auto context = isolate()->GetCurrentContext();
     auto templ = CallbackTraits<T>::CreateTemplate(isolate(), callback);
     return GetHandle()
-        ->Set(context, gin::StringToV8(isolate(), key),
+        ->Set(context, MakeKey(key),
               templ->GetFunction(context).ToLocalChecked())
         .ToChecked();
   }
@@ -130,7 +129,7 @@ class Dictionary : public gin::Dictionary {
 
     return GetHandle()
         ->SetNativeDataProperty(
-            context, gin::StringToV8(isolate(), key),
+            context, MakeKey(key),
             [](v8::Local<v8::Name> property_name,
                const v8::PropertyCallbackInfo<v8::Value>& info) {
               AccessorValue<V> acc_value;
@@ -153,9 +152,8 @@ class Dictionary : public gin::Dictionary {
     if (!gin::TryConvertToV8(isolate(), val, &v8_value))
       return false;
     v8::Maybe<bool> result = GetHandle()->DefineOwnProperty(
-        isolate()->GetCurrentContext(), gin::StringToV8(isolate(), key),
-        v8_value, v8::ReadOnly);
-    return !result.IsNothing() && result.FromJust();
+        isolate()->GetCurrentContext(), MakeKey(key), v8_value, v8::ReadOnly);
+    return result.FromMaybe(false);
   }
 
   // Note: If we plan to add more Set methods, consider adding an option instead
@@ -166,22 +164,21 @@ class Dictionary : public gin::Dictionary {
     if (!gin::TryConvertToV8(isolate(), val, &v8_value))
       return false;
     v8::Maybe<bool> result = GetHandle()->DefineOwnProperty(
-        isolate()->GetCurrentContext(), gin::StringToV8(isolate(), key),
-        v8_value,
+        isolate()->GetCurrentContext(), MakeKey(key), v8_value,
         static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontDelete));
-    return !result.IsNothing() && result.FromJust();
+    return result.FromMaybe(false);
   }
 
   bool Has(std::string_view key) const {
-    v8::Maybe<bool> result = GetHandle()->Has(isolate()->GetCurrentContext(),
-                                              gin::StringToV8(isolate(), key));
-    return !result.IsNothing() && result.FromJust();
+    v8::Maybe<bool> result =
+        GetHandle()->Has(isolate()->GetCurrentContext(), MakeKey(key));
+    return result.FromMaybe(false);
   }
 
   bool Delete(std::string_view key) {
-    v8::Maybe<bool> result = GetHandle()->Delete(
-        isolate()->GetCurrentContext(), gin::StringToV8(isolate(), key));
-    return !result.IsNothing() && result.FromJust();
+    v8::Maybe<bool> result =
+        GetHandle()->Delete(isolate()->GetCurrentContext(), MakeKey(key));
+    return result.FromMaybe(false);
   }
 
   bool IsEmpty() const { return isolate() == nullptr || GetHandle().IsEmpty(); }
@@ -194,6 +191,16 @@ class Dictionary : public gin::Dictionary {
 
  private:
   // DO NOT ADD ANY DATA MEMBER.
+
+  [[nodiscard]] v8::Local<v8::String> MakeKey(
+      const std::string_view key) const {
+    return gin::StringToV8(isolate(), key);
+  }
+
+  [[nodiscard]] v8::Local<v8::Private> MakeHiddenKey(
+      const std::string_view key) const {
+    return v8::Private::ForApi(isolate(), MakeKey(key));
+  }
 };
 
 }  // namespace gin_helper

filename: shell/common/gin_helper/persistent_dictionary.h
patch: @@ -32,14 +32,13 @@ class PersistentDictionary {
 
   template <typename K, typename V>
   bool Get(const K& key, V* out) const {
+    const auto handle = GetHandle();
     v8::Local<v8::Context> context = isolate_->GetCurrentContext();
     v8::Local<v8::Value> v8_key = gin::ConvertToV8(isolate_, key);
     v8::Local<v8::Value> value;
-    v8::Maybe<bool> result = GetHandle()->Has(context, v8_key);
-    if (result.IsJust() && result.FromJust() &&
-        GetHandle()->Get(context, v8_key).ToLocal(&value))
-      return gin::ConvertFromV8(isolate_, value, out);
-    return false;
+    return handle->Has(context, v8_key).FromMaybe(false) &&
+           handle->Get(context, v8_key).ToLocal(&value) &&
+           gin::ConvertFromV8(isolate_, value, out);
   }
 
  private: