refactor: eliminate duplicate code (#29174)

shell/browser/api/electron_api_system_preferences_mac.mm

@@ -123,6 +123,19 @@ std::string ConvertSystemPermission(
   }
 }
 
+NSNotificationCenter* GetNotificationCenter(NotificationCenterKind kind) {
+  switch (kind) {
+    case NotificationCenterKind::kNSDistributedNotificationCenter:
+      return [NSDistributedNotificationCenter defaultCenter];
+    case NotificationCenterKind::kNSNotificationCenter:
+      return [NSNotificationCenter defaultCenter];
+    case NotificationCenterKind::kNSWorkspaceNotificationCenter:
+      return [[NSWorkspace sharedWorkspace] notificationCenter];
+    default:
+      return nil;
+  }
+}
+
 }  // namespace
 
 void SystemPreferences::PostNotification(const std::string& name,
@@ -199,22 +212,8 @@ int SystemPreferences::DoSubscribeNotification(
     NotificationCenterKind kind) {
   int request_id = g_next_id++;
   __block NotificationCallback copied_callback = callback;
-  NSNotificationCenter* center;
-  switch (kind) {
-    case NotificationCenterKind::kNSDistributedNotificationCenter:
-      center = [NSDistributedNotificationCenter defaultCenter];
-      break;
-    case NotificationCenterKind::kNSNotificationCenter:
-      center = [NSNotificationCenter defaultCenter];
-      break;
-    case NotificationCenterKind::kNSWorkspaceNotificationCenter:
-      center = [[NSWorkspace sharedWorkspace] notificationCenter];
-      break;
-    default:
-      break;
-  }
 
-  g_id_map[request_id] = [center
+  g_id_map[request_id] = [GetNotificationCenter(kind)
       addObserverForName:base::SysUTF8ToNSString(name)
                   object:nil
                    queue:nil
@@ -243,21 +242,7 @@ void SystemPreferences::DoUnsubscribeNotification(int request_id,
   auto iter = g_id_map.find(request_id);
   if (iter != g_id_map.end()) {
     id observer = iter->second;
-    NSNotificationCenter* center;
-    switch (kind) {
-      case NotificationCenterKind::kNSDistributedNotificationCenter:
-        center = [NSDistributedNotificationCenter defaultCenter];
-        break;
-      case NotificationCenterKind::kNSNotificationCenter:
-        center = [NSNotificationCenter defaultCenter];
-        break;
-      case NotificationCenterKind::kNSWorkspaceNotificationCenter:
-        center = [[NSWorkspace sharedWorkspace] notificationCenter];
-        break;
-      default:
-        break;
-    }
-    [center removeObserver:observer];
+    [GetNotificationCenter(kind) removeObserver:observer];
     g_id_map.erase(iter);
   }
 }

shell/browser/api/electron_api_web_contents.cc

@@ -2217,10 +2217,8 @@ void WebContents::EnableDeviceEmulation(
   DCHECK(web_contents());
   auto* frame_host = web_contents()->GetMainFrame();
   if (frame_host) {
-    auto* widget_host_impl =
-        frame_host ? static_cast<content::RenderWidgetHostImpl*>(
-                         frame_host->GetView()->GetRenderWidgetHost())
-                   : nullptr;
+    auto* widget_host_impl = static_cast<content::RenderWidgetHostImpl*>(
+        frame_host->GetView()->GetRenderWidgetHost());
     if (widget_host_impl) {
       auto& frame_widget = widget_host_impl->GetAssociatedFrameWidget();
       frame_widget->EnableDeviceEmulation(params);
@@ -2235,10 +2233,8 @@ void WebContents::DisableDeviceEmulation() {
   DCHECK(web_contents());
   auto* frame_host = web_contents()->GetMainFrame();
   if (frame_host) {
-    auto* widget_host_impl =
-        frame_host ? static_cast<content::RenderWidgetHostImpl*>(
-                         frame_host->GetView()->GetRenderWidgetHost())
-                   : nullptr;
+    auto* widget_host_impl = static_cast<content::RenderWidgetHostImpl*>(
+        frame_host->GetView()->GetRenderWidgetHost());
     if (widget_host_impl) {
       auto& frame_widget = widget_host_impl->GetAssociatedFrameWidget();
       frame_widget->DisableDeviceEmulation();

shell/common/api/electron_api_native_image.cc

@@ -109,13 +109,7 @@ base::win::ScopedHICON ReadICOFromPath(int size, const base::FilePath& path) {
 
 NativeImage::NativeImage(v8::Isolate* isolate, const gfx::Image& image)
     : image_(image), isolate_(isolate) {
-  if (image_.HasRepresentation(gfx::Image::kImageRepSkia)) {
-    auto* const image_skia = image_.ToImageSkia();
-    if (!image_skia->isNull()) {
-      isolate_->AdjustAmountOfExternalAllocatedMemory(
-          image_skia->bitmap()->computeByteSize());
-    }
-  }
+  AdjustAmountOfExternalAllocatedMemory(true);
 }
 
 #if defined(OS_WIN)
@@ -125,21 +119,21 @@ NativeImage::NativeImage(v8::Isolate* isolate, const base::FilePath& hicon_path)
   gfx::ImageSkia image_skia;
   electron::util::ReadImageSkiaFromICO(&image_skia, GetHICON(256));
   image_ = gfx::Image(image_skia);
-  if (image_.HasRepresentation(gfx::Image::kImageRepSkia)) {
-    if (!image_skia.isNull()) {
-      isolate_->AdjustAmountOfExternalAllocatedMemory(
-          image_.ToImageSkia()->bitmap()->computeByteSize());
-    }
-  }
+
+  AdjustAmountOfExternalAllocatedMemory(true);
 }
 #endif
 
 NativeImage::~NativeImage() {
+  AdjustAmountOfExternalAllocatedMemory(false);
+}
+
+void NativeImage::AdjustAmountOfExternalAllocatedMemory(bool add) {
   if (image_.HasRepresentation(gfx::Image::kImageRepSkia)) {
     auto* const image_skia = image_.ToImageSkia();
     if (!image_skia->isNull()) {
-      isolate_->AdjustAmountOfExternalAllocatedMemory(
-          -static_cast<int64_t>(image_skia->bitmap()->computeByteSize()));
+      int64_t size = image_skia->bitmap()->computeByteSize();
+      isolate_->AdjustAmountOfExternalAllocatedMemory(add ? size : -size);
     }
   }
 }

shell/common/api/electron_api_native_image.h

@@ -118,6 +118,8 @@ class NativeImage : public gin::Wrappable<NativeImage> {
   float GetAspectRatio(const base::Optional<float> scale_factor);
   void AddRepresentation(const gin_helper::Dictionary& options);
 
+  void AdjustAmountOfExternalAllocatedMemory(bool add);
+
   // Mark the image as template image.
   void SetTemplateImage(bool setAsTemplate);
   // Determine if the image is a template image.

shell/renderer/api/electron_api_context_bridge.cc

@@ -35,6 +35,8 @@ const base::Feature kContextBridgeMutability{"ContextBridgeMutability",
 
 namespace electron {
 
+content::RenderFrame* GetRenderFrame(v8::Local<v8::Object> value);
+
 namespace api {
 
 namespace context_bridge {
@@ -56,16 +58,6 @@ inline bool IsTrue(v8::Maybe<bool> maybe) {
   return maybe.IsJust() && maybe.FromJust();
 }
 
-content::RenderFrame* GetRenderFrame(const v8::Local<v8::Object>& value) {
-  v8::Local<v8::Context> context = value->CreationContext();
-  if (context.IsEmpty())
-    return nullptr;
-  blink::WebLocalFrame* frame = blink::WebLocalFrame::FrameForContext(context);
-  if (!frame)
-    return nullptr;
-  return content::RenderFrame::FromWebFrame(frame);
-}
-
 // Sourced from "extensions/renderer/v8_schema_registry.cc"
 // Recursively freezes every v8 object on |object|.
 bool DeepFreeze(const v8::Local<v8::Object>& object,

shell/renderer/api/electron_api_web_frame.cc

@@ -98,10 +98,6 @@ struct Converter<blink::WebDocument::CSSOrigin> {
 
 namespace electron {
 
-namespace api {
-
-namespace {
-
 content::RenderFrame* GetRenderFrame(v8::Local<v8::Object> value) {
   v8::Local<v8::Context> context = value->CreationContext();
   if (context.IsEmpty())
@@ -112,6 +108,10 @@ content::RenderFrame* GetRenderFrame(v8::Local<v8::Object> value) {
   return content::RenderFrame::FromWebFrame(frame);
 }
 
+namespace api {
+
+namespace {
+
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
 
 bool SpellCheckWord(content::RenderFrame* render_frame,
@@ -422,6 +422,17 @@ class WebFrameRenderer : public gin::Wrappable<WebFrameRenderer>,
     return true;
   }
 
+  static v8::Local<v8::Value> CreateWebFrameRenderer(v8::Isolate* isolate,
+                                                     blink::WebFrame* frame) {
+    if (frame && frame->IsWebLocalFrame()) {
+      auto* render_frame =
+          content::RenderFrame::FromWebFrame(frame->ToWebLocalFrame());
+      return WebFrameRenderer::Create(isolate, render_frame).ToV8();
+    } else {
+      return v8::Null(isolate);
+    }
+  }
+
   void SetName(v8::Isolate* isolate, const std::string& name) {
     content::RenderFrame* render_frame;
     if (!MaybeGetRenderFrame(isolate, "setName", &render_frame))
@@ -800,13 +811,7 @@ class WebFrameRenderer : public gin::Wrappable<WebFrameRenderer>,
       return v8::Null(isolate);
 
     blink::WebFrame* frame = render_frame->GetWebFrame()->Opener();
-    if (frame && frame->IsWebLocalFrame())
-      return WebFrameRenderer::Create(
-                 isolate,
-                 content::RenderFrame::FromWebFrame(frame->ToWebLocalFrame()))
-          .ToV8();
-    else
-      return v8::Null(isolate);
+    return CreateWebFrameRenderer(isolate, frame);
   }
 
   // Don't name it as GetParent, Windows has API with same name.
@@ -816,13 +821,7 @@ class WebFrameRenderer : public gin::Wrappable<WebFrameRenderer>,
       return v8::Null(isolate);
 
     blink::WebFrame* frame = render_frame->GetWebFrame()->Parent();
-    if (frame && frame->IsWebLocalFrame())
-      return WebFrameRenderer::Create(
-                 isolate,
-                 content::RenderFrame::FromWebFrame(frame->ToWebLocalFrame()))
-          .ToV8();
-    else
-      return v8::Null(isolate);
+    return CreateWebFrameRenderer(isolate, frame);
   }
 
   v8::Local<v8::Value> GetTop(v8::Isolate* isolate) {
@@ -831,13 +830,7 @@ class WebFrameRenderer : public gin::Wrappable<WebFrameRenderer>,
       return v8::Null(isolate);
 
     blink::WebFrame* frame = render_frame->GetWebFrame()->Top();
-    if (frame && frame->IsWebLocalFrame())
-      return WebFrameRenderer::Create(
-                 isolate,
-                 content::RenderFrame::FromWebFrame(frame->ToWebLocalFrame()))
-          .ToV8();
-    else
-      return v8::Null(isolate);
+    return CreateWebFrameRenderer(isolate, frame);
   }
 
   v8::Local<v8::Value> GetFirstChild(v8::Isolate* isolate) {
@@ -846,13 +839,7 @@ class WebFrameRenderer : public gin::Wrappable<WebFrameRenderer>,
       return v8::Null(isolate);
 
     blink::WebFrame* frame = render_frame->GetWebFrame()->FirstChild();
-    if (frame && frame->IsWebLocalFrame())
-      return WebFrameRenderer::Create(
-                 isolate,
-                 content::RenderFrame::FromWebFrame(frame->ToWebLocalFrame()))
-          .ToV8();
-    else
-      return v8::Null(isolate);
+    return CreateWebFrameRenderer(isolate, frame);
   }
 
   v8::Local<v8::Value> GetNextSibling(v8::Isolate* isolate) {
@@ -861,13 +848,7 @@ class WebFrameRenderer : public gin::Wrappable<WebFrameRenderer>,
       return v8::Null(isolate);
 
     blink::WebFrame* frame = render_frame->GetWebFrame()->NextSibling();
-    if (frame && frame->IsWebLocalFrame())
-      return WebFrameRenderer::Create(
-                 isolate,
-                 content::RenderFrame::FromWebFrame(frame->ToWebLocalFrame()))
-          .ToV8();
-    else
-      return v8::Null(isolate);
+    return CreateWebFrameRenderer(isolate, frame);
   }
 
   v8::Local<v8::Value> GetFrameForSelector(v8::Isolate* isolate,
@@ -883,30 +864,18 @@ class WebFrameRenderer : public gin::Wrappable<WebFrameRenderer>,
       return v8::Null(isolate);
 
     blink::WebFrame* frame = blink::WebFrame::FromFrameOwnerElement(element);
-    if (frame && frame->IsWebLocalFrame())
-      return WebFrameRenderer::Create(
-                 isolate,
-                 content::RenderFrame::FromWebFrame(frame->ToWebLocalFrame()))
-          .ToV8();
-    else
-      return v8::Null(isolate);
+    return CreateWebFrameRenderer(isolate, frame);
   }
 
   v8::Local<v8::Value> FindFrameByName(v8::Isolate* isolate,
                                        const std::string& name) {
     content::RenderFrame* render_frame;
-    if (!MaybeGetRenderFrame(isolate, "getFrameForSelector", &render_frame))
+    if (!MaybeGetRenderFrame(isolate, "findFrameByName", &render_frame))
       return v8::Null(isolate);
 
     blink::WebFrame* frame = render_frame->GetWebFrame()->FindFrameByName(
         blink::WebString::FromUTF8(name));
-    if (frame && frame->IsWebLocalFrame())
-      return WebFrameRenderer::Create(
-                 isolate,
-                 content::RenderFrame::FromWebFrame(frame->ToWebLocalFrame()))
-          .ToV8();
-    else
-      return v8::Null(isolate);
+    return CreateWebFrameRenderer(isolate, frame);
   }
 
   int GetRoutingId(v8::Isolate* isolate) {
@@ -937,8 +906,8 @@ void Initialize(v8::Local<v8::Object> exports,
 
   v8::Isolate* isolate = context->GetIsolate();
   gin_helper::Dictionary dict(isolate, exports);
-  dict.Set("mainFrame",
-           WebFrameRenderer::Create(isolate, GetRenderFrame(exports)));
+  dict.Set("mainFrame", WebFrameRenderer::Create(
+                            isolate, electron::GetRenderFrame(exports)));
 }
 
 }  // namespace

shell/renderer/electron_sandboxed_renderer_client.cc

@@ -86,9 +86,13 @@ v8::Local<v8::Value> GetBinding(v8::Isolate* isolate,
 }
 
 v8::Local<v8::Value> CreatePreloadScript(v8::Isolate* isolate,
-                                         v8::Local<v8::String> preloadSrc) {
-  return RendererClientBase::RunScript(isolate->GetCurrentContext(),
-                                       preloadSrc);
+                                         v8::Local<v8::String> source) {
+  auto context = isolate->GetCurrentContext();
+  auto maybe_script = v8::Script::Compile(context, source);
+  v8::Local<v8::Script> script;
+  if (!maybe_script.ToLocal(&script))
+    return v8::Local<v8::Value>();
+  return script->Run(context).ToLocalChecked();
 }
 
 double Uptime() {
@@ -157,11 +161,6 @@ void ElectronSandboxedRendererClient::RenderFrameCreated(
   RendererClientBase::RenderFrameCreated(render_frame);
 }
 
-void ElectronSandboxedRendererClient::RenderViewCreated(
-    content::RenderView* render_view) {
-  RendererClientBase::RenderViewCreated(render_view);
-}
-
 void ElectronSandboxedRendererClient::RunScriptsAtDocumentStart(
     content::RenderFrame* render_frame) {
   RendererClientBase::RunScriptsAtDocumentStart(render_frame);

shell/renderer/electron_sandboxed_renderer_client.h

@@ -29,7 +29,6 @@ class ElectronSandboxedRendererClient : public RendererClientBase {
                                 content::RenderFrame* render_frame) override;
   // content::ContentRendererClient:
   void RenderFrameCreated(content::RenderFrame*) override;
-  void RenderViewCreated(content::RenderView*) override;
   void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override;
   void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame) override;
   bool ShouldFork(blink::WebLocalFrame* frame,

shell/renderer/renderer_client_base.cc

@@ -88,17 +88,9 @@
 
 namespace electron {
 
-namespace {
+content::RenderFrame* GetRenderFrame(v8::Local<v8::Object> value);
 
-content::RenderFrame* GetRenderFrame(v8::Local<v8::Object> value) {
-  v8::Local<v8::Context> context = value->CreationContext();
-  if (context.IsEmpty())
-    return nullptr;
-  blink::WebLocalFrame* frame = blink::WebLocalFrame::FrameForContext(context);
-  if (!frame)
-    return nullptr;
-  return content::RenderFrame::FromWebFrame(frame);
-}
+namespace {
 
 void SetIsWebView(v8::Isolate* isolate, v8::Local<v8::Object> object) {
   gin_helper::Dictionary dict(isolate, object);
@@ -484,16 +476,6 @@ v8::Local<v8::Context> RendererClientBase::GetContext(
     return frame->MainWorldScriptContext();
 }
 
-v8::Local<v8::Value> RendererClientBase::RunScript(
-    v8::Local<v8::Context> context,
-    v8::Local<v8::String> source) {
-  auto maybe_script = v8::Script::Compile(context, source);
-  v8::Local<v8::Script> script;
-  if (!maybe_script.ToLocal(&script))
-    return v8::Local<v8::Value>();
-  return script->Run(context).ToLocalChecked();
-}
-
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
 extensions::ExtensionsClient* RendererClientBase::CreateExtensionsClient() {
   return new ElectronExtensionsClient;

shell/renderer/renderer_client_base.h

@@ -82,9 +82,6 @@ class RendererClientBase : public content::ContentRendererClient
   // Get the context that the Electron API is running in.
   v8::Local<v8::Context> GetContext(blink::WebLocalFrame* frame,
                                     v8::Isolate* isolate) const;
-  // Executes a given v8 Script
-  static v8::Local<v8::Value> RunScript(v8::Local<v8::Context> context,
-                                        v8::Local<v8::String> source);
 
   static void AllowGuestViewElementDefinition(
       v8::Isolate* isolate,