perf: omit unnecessary work from ElectronRenderFrameObserver::ShouldNotifyClient() (#41347)

perf: omit unnecessary work from ElectronRenderFrameObserver::ShouldNotifyClient()

- (perf) GetBlinkPreferences() returns a const&, so we can use that
  reference instead of making a temporary copy

- (perf) Don't create url object unless it's needed.

- (refactor) Move is_main_world() and is_isolated_world() from the
  header into an anonymous namespace in the .cc file so they can
  be inlined and made constexpr
This commit is contained in:
Charles Kerr 2024-02-20 06:29:11 -06:00 committed by GitHub
parent 995f3dc120
commit 7cd23a4900
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 20 additions and 21 deletions

View file

@ -47,6 +47,14 @@ scoped_refptr<base::RefCountedMemory> NetResourceProvider(int key) {
return nullptr; return nullptr;
} }
[[nodiscard]] constexpr bool is_main_world(int world_id) {
return world_id == WorldIDs::MAIN_WORLD_ID;
}
[[nodiscard]] constexpr bool is_isolated_world(int world_id) {
return world_id == WorldIDs::ISOLATED_WORLD_ID;
}
} // namespace } // namespace
ElectronRenderFrameObserver::ElectronRenderFrameObserver( ElectronRenderFrameObserver::ElectronRenderFrameObserver(
@ -130,7 +138,7 @@ void ElectronRenderFrameObserver::DidInstallConditionalFeatures(
// This logic matches the EXPLAINED logic in electron_renderer_client.cc // This logic matches the EXPLAINED logic in electron_renderer_client.cc
// to avoid explaining it twice go check that implementation in // to avoid explaining it twice go check that implementation in
// DidCreateScriptContext(); // DidCreateScriptContext();
bool is_main_world = IsMainWorld(world_id); bool is_main_world = electron::is_main_world(world_id);
bool is_main_frame = render_frame_->IsMainFrame(); bool is_main_frame = render_frame_->IsMainFrame();
bool allow_node_in_sub_frames = prefs.node_integration_in_sub_frames; bool allow_node_in_sub_frames = prefs.node_integration_in_sub_frames;
@ -204,16 +212,8 @@ void ElectronRenderFrameObserver::CreateIsolatedWorldContext() {
blink::BackForwardCacheAware::kPossiblyDisallow); blink::BackForwardCacheAware::kPossiblyDisallow);
} }
bool ElectronRenderFrameObserver::IsMainWorld(int world_id) { bool ElectronRenderFrameObserver::ShouldNotifyClient(int world_id) const {
return world_id == WorldIDs::MAIN_WORLD_ID; const auto& prefs = render_frame_->GetBlinkPreferences();
}
bool ElectronRenderFrameObserver::IsIsolatedWorld(int world_id) {
return world_id == WorldIDs::ISOLATED_WORLD_ID;
}
bool ElectronRenderFrameObserver::ShouldNotifyClient(int world_id) {
auto prefs = render_frame_->GetBlinkPreferences();
// This is necessary because if an iframe is created and a source is not // This is necessary because if an iframe is created and a source is not
// set, the iframe loads about:blank and creates a script context for the // set, the iframe loads about:blank and creates a script context for the
@ -221,17 +221,17 @@ bool ElectronRenderFrameObserver::ShouldNotifyClient(int world_id) {
// is later set, the JS necessary to do that triggers illegal access errors // is later set, the JS necessary to do that triggers illegal access errors
// when the initial about:blank Node.js environment is cleaned up. See: // when the initial about:blank Node.js environment is cleaned up. See:
// https://source.chromium.org/chromium/chromium/src/+/main:content/renderer/render_frame_impl.h;l=870-892;drc=4b6001440a18740b76a1c63fa2a002cc941db394 // https://source.chromium.org/chromium/chromium/src/+/main:content/renderer/render_frame_impl.h;l=870-892;drc=4b6001440a18740b76a1c63fa2a002cc941db394
GURL url = render_frame_->GetWebFrame()->GetDocument().Url(); const bool allow_node_in_sub_frames = prefs.node_integration_in_sub_frames;
bool allow_node_in_sub_frames = prefs.node_integration_in_sub_frames; if (allow_node_in_sub_frames && !render_frame_->IsMainFrame()) {
if (allow_node_in_sub_frames && url.IsAboutBlank() && if (GURL{render_frame_->GetWebFrame()->GetDocument().Url()}.IsAboutBlank())
!render_frame_->IsMainFrame()) return false;
return false; }
if (prefs.context_isolation && if (prefs.context_isolation &&
(render_frame_->IsMainFrame() || allow_node_in_sub_frames)) (render_frame_->IsMainFrame() || allow_node_in_sub_frames))
return IsIsolatedWorld(world_id); return is_isolated_world(world_id);
return IsMainWorld(world_id); return is_main_world(world_id);
} }
} // namespace electron } // namespace electron

View file

@ -37,10 +37,9 @@ class ElectronRenderFrameObserver : public content::RenderFrameObserver {
void DidMeaningfulLayout(blink::WebMeaningfulLayout layout_type) override; void DidMeaningfulLayout(blink::WebMeaningfulLayout layout_type) override;
private: private:
bool ShouldNotifyClient(int world_id); [[nodiscard]] bool ShouldNotifyClient(int world_id) const;
void CreateIsolatedWorldContext(); void CreateIsolatedWorldContext();
bool IsMainWorld(int world_id);
bool IsIsolatedWorld(int world_id);
void OnTakeHeapSnapshot(IPC::PlatformFileForTransit file_handle, void OnTakeHeapSnapshot(IPC::PlatformFileForTransit file_handle,
const std::string& channel); const std::string& channel);