From 241cceb2c9ad865481c5739254d387645cd4c313 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Mon, 24 May 2021 04:32:55 +0200 Subject: [PATCH] refactor: eliminate duplicate code (#29174) --- .../electron_api_system_preferences_mac.mm | 45 ++++------- .../browser/api/electron_api_web_contents.cc | 12 +-- shell/common/api/electron_api_native_image.cc | 24 +++--- shell/common/api/electron_api_native_image.h | 2 + .../api/electron_api_context_bridge.cc | 12 +-- shell/renderer/api/electron_api_web_frame.cc | 81 ++++++------------- .../electron_sandboxed_renderer_client.cc | 15 ++-- .../electron_sandboxed_renderer_client.h | 1 - shell/renderer/renderer_client_base.cc | 22 +---- shell/renderer/renderer_client_base.h | 3 - 10 files changed, 66 insertions(+), 151 deletions(-) diff --git a/shell/browser/api/electron_api_system_preferences_mac.mm b/shell/browser/api/electron_api_system_preferences_mac.mm index 2595a1c7ee..9281837fc0 100644 --- a/shell/browser/api/electron_api_system_preferences_mac.mm +++ b/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); } } diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index c7537a4f8f..922fde26ed 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/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( - frame_host->GetView()->GetRenderWidgetHost()) - : nullptr; + auto* widget_host_impl = static_cast( + 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( - frame_host->GetView()->GetRenderWidgetHost()) - : nullptr; + auto* widget_host_impl = static_cast( + frame_host->GetView()->GetRenderWidgetHost()); if (widget_host_impl) { auto& frame_widget = widget_host_impl->GetAssociatedFrameWidget(); frame_widget->DisableDeviceEmulation(); diff --git a/shell/common/api/electron_api_native_image.cc b/shell/common/api/electron_api_native_image.cc index cae7d407a7..53dc544504 100644 --- a/shell/common/api/electron_api_native_image.cc +++ b/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(image_skia->bitmap()->computeByteSize())); + int64_t size = image_skia->bitmap()->computeByteSize(); + isolate_->AdjustAmountOfExternalAllocatedMemory(add ? size : -size); } } } diff --git a/shell/common/api/electron_api_native_image.h b/shell/common/api/electron_api_native_image.h index 182c47b9a3..d98763c946 100644 --- a/shell/common/api/electron_api_native_image.h +++ b/shell/common/api/electron_api_native_image.h @@ -118,6 +118,8 @@ class NativeImage : public gin::Wrappable { float GetAspectRatio(const base::Optional 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. diff --git a/shell/renderer/api/electron_api_context_bridge.cc b/shell/renderer/api/electron_api_context_bridge.cc index d6e93ffb49..4cc4f0705e 100644 --- a/shell/renderer/api/electron_api_context_bridge.cc +++ b/shell/renderer/api/electron_api_context_bridge.cc @@ -35,6 +35,8 @@ const base::Feature kContextBridgeMutability{"ContextBridgeMutability", namespace electron { +content::RenderFrame* GetRenderFrame(v8::Local value); + namespace api { namespace context_bridge { @@ -56,16 +58,6 @@ inline bool IsTrue(v8::Maybe maybe) { return maybe.IsJust() && maybe.FromJust(); } -content::RenderFrame* GetRenderFrame(const v8::Local& value) { - v8::Local 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& object, diff --git a/shell/renderer/api/electron_api_web_frame.cc b/shell/renderer/api/electron_api_web_frame.cc index 936ba140dc..7362cf3941 100644 --- a/shell/renderer/api/electron_api_web_frame.cc +++ b/shell/renderer/api/electron_api_web_frame.cc @@ -98,10 +98,6 @@ struct Converter { namespace electron { -namespace api { - -namespace { - content::RenderFrame* GetRenderFrame(v8::Local value) { v8::Local context = value->CreationContext(); if (context.IsEmpty()) @@ -112,6 +108,10 @@ content::RenderFrame* GetRenderFrame(v8::Local 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, return true; } + static v8::Local 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, 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, 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 GetTop(v8::Isolate* isolate) { @@ -831,13 +830,7 @@ class WebFrameRenderer : public gin::Wrappable, 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 GetFirstChild(v8::Isolate* isolate) { @@ -846,13 +839,7 @@ class WebFrameRenderer : public gin::Wrappable, 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 GetNextSibling(v8::Isolate* isolate) { @@ -861,13 +848,7 @@ class WebFrameRenderer : public gin::Wrappable, 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 GetFrameForSelector(v8::Isolate* isolate, @@ -883,30 +864,18 @@ class WebFrameRenderer : public gin::Wrappable, 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 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 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 diff --git a/shell/renderer/electron_sandboxed_renderer_client.cc b/shell/renderer/electron_sandboxed_renderer_client.cc index 8eb90becef..9f27320dc3 100644 --- a/shell/renderer/electron_sandboxed_renderer_client.cc +++ b/shell/renderer/electron_sandboxed_renderer_client.cc @@ -86,9 +86,13 @@ v8::Local GetBinding(v8::Isolate* isolate, } v8::Local CreatePreloadScript(v8::Isolate* isolate, - v8::Local preloadSrc) { - return RendererClientBase::RunScript(isolate->GetCurrentContext(), - preloadSrc); + v8::Local source) { + auto context = isolate->GetCurrentContext(); + auto maybe_script = v8::Script::Compile(context, source); + v8::Local script; + if (!maybe_script.ToLocal(&script)) + return v8::Local(); + 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); diff --git a/shell/renderer/electron_sandboxed_renderer_client.h b/shell/renderer/electron_sandboxed_renderer_client.h index 67c9002ec6..a3ccf2c6eb 100644 --- a/shell/renderer/electron_sandboxed_renderer_client.h +++ b/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, diff --git a/shell/renderer/renderer_client_base.cc b/shell/renderer/renderer_client_base.cc index fd6a7685dc..0ead7d8189 100644 --- a/shell/renderer/renderer_client_base.cc +++ b/shell/renderer/renderer_client_base.cc @@ -88,17 +88,9 @@ namespace electron { -namespace { +content::RenderFrame* GetRenderFrame(v8::Local value); -content::RenderFrame* GetRenderFrame(v8::Local value) { - v8::Local 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 object) { gin_helper::Dictionary dict(isolate, object); @@ -484,16 +476,6 @@ v8::Local RendererClientBase::GetContext( return frame->MainWorldScriptContext(); } -v8::Local RendererClientBase::RunScript( - v8::Local context, - v8::Local source) { - auto maybe_script = v8::Script::Compile(context, source); - v8::Local script; - if (!maybe_script.ToLocal(&script)) - return v8::Local(); - return script->Run(context).ToLocalChecked(); -} - #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) extensions::ExtensionsClient* RendererClientBase::CreateExtensionsClient() { return new ElectronExtensionsClient; diff --git a/shell/renderer/renderer_client_base.h b/shell/renderer/renderer_client_base.h index 6c75a706da..49a26952cd 100644 --- a/shell/renderer/renderer_client_base.h +++ b/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 GetContext(blink::WebLocalFrame* frame, v8::Isolate* isolate) const; - // Executes a given v8 Script - static v8::Local RunScript(v8::Local context, - v8::Local source); static void AllowGuestViewElementDefinition( v8::Isolate* isolate,